qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Emilio G. Cota" <cota@braap.org>
To: Richard Henderson <rth@twiddle.net>
Cc: "QEMU Developers" <qemu-devel@nongnu.org>,
	"MTTCG Devel" <mttcg@greensocs.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Peter Crosthwaite" <crosthwaite.peter@gmail.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Sergey Fedorov" <serge.fdrv@gmail.com>
Subject: Re: [Qemu-devel] [PATCH v3 11/11] translate-all: add tb hash bucket info to 'info jit' dump
Date: Sun, 24 Apr 2016 18:06:51 -0400	[thread overview]
Message-ID: <20160424220651.GB1122@flamenco> (raw)
In-Reply-To: <571D2280.3040908@twiddle.net>

On Sun, Apr 24, 2016 at 12:46:08 -0700, Richard Henderson wrote:
> On 04/22/2016 04:57 PM, Emilio G. Cota wrote:
> >On Fri, Apr 22, 2016 at 12:59:52 -0700, Richard Henderson wrote:
> >>FWIW, so that I could get an idea of how the stats change as we improve the
> >>hashing, I inserted the attachment 1 patch between patches 5 and 6, and with
> >>attachment 2 attempting to fix the accounting for patches 9 and 10.
> >
> >For qht, I dislike the approach of reporting "avg chain" per-element,
> >instead of per-bucket. Performance for a bucket whose entries are
> >all valid is virtually the same as that of a bucket that only
> >has one valid element; thus, with per-bucket reporting, we'd say that
> >the chain lenght is 1 in both cases, i.e. "perfect". With per-element
> >reporting, we'd report 4 (on a 64-bit host, since that's the value of
> >QHT_BUCKET_ENTRIES) when the bucket is full, which IMO gives the
> >wrong idea (users would think they're in trouble, when they're not).
> 
> But otherwise you have no way of knowing how full the buckets are.  The
> bucket size is just something that you have to keep in mind.

I'll make some changes in v4 that I think will address both your and
my concerns:
- Report the number of empty buckets
- Do not count empty buckets when reporting avg bucket chain length
- Report average bucket occupancy (in %, so that QHT_BUCKET_ENTRIES
  does not have to be reported.)

> >If those numbers are off, then either this
> >     assert(hinfo.used_entries ==
> >            tcg_ctx.tb_ctx.nb_tbs - tcg_ctx.tb_ctx.tb_phys_invalidate_count);
> >should trigger, or the accounting isn't right.
> 
> I think I used an NDEBUG build, so these weren't effective.
> 
> >Only the second report ("after 7/11") seems good (taking into account
> >lack of precision of just 3 decimals):
> >   5.26*32582=171381.32 ~= 171367
> >which leads me to believe that you've used the TB and invalidate
> >counts from that test.
> 
> The TB and invalidate numbers are repeatable; the same every time.

Then something else is going on, because both the 1st and 3rd tests are
way off. I'd re-test with assertions enabled.

Thanks,

		Emilio

  reply	other threads:[~2016-04-24 22:06 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-19 23:07 [Qemu-devel] [PATCH v3 00/11] tb hash improvements Emilio G. Cota
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 01/11] compiler.h: add QEMU_ALIGNED() to enforce struct alignment Emilio G. Cota
2016-04-22  9:32   ` Alex Bennée
2016-04-22  9:35   ` Peter Maydell
2016-04-22 15:50     ` Emilio G. Cota
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 02/11] seqlock: remove optional mutex Emilio G. Cota
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 03/11] seqlock: rename write_lock/unlock to write_begin/end Emilio G. Cota
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 04/11] include/processor.h: define cpu_relax() Emilio G. Cota
2016-04-20 12:15   ` KONRAD Frederic
2016-04-20 17:16     ` Emilio G. Cota
2016-04-20 17:18       ` Peter Maydell
2016-04-20 17:32   ` [Qemu-devel] [UPDATED " Emilio G. Cota
2016-04-22  9:35   ` [Qemu-devel] [PATCH " Alex Bennée
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 05/11] qemu-thread: add simple test-and-set spinlock Emilio G. Cota
2016-04-20 15:18   ` Richard Henderson
2016-04-20 17:17     ` Emilio G. Cota
2016-04-20 17:55       ` Richard Henderson
2016-04-20 18:11         ` Emilio G. Cota
2016-04-20 19:39           ` Richard Henderson
2016-04-21 16:24             ` Emilio G. Cota
2016-04-20 17:35   ` [Qemu-devel] [UPDATED " Emilio G. Cota
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 06/11] exec: add tb_hash_func5, derived from xxhash Emilio G. Cota
2016-04-20 15:19   ` Richard Henderson
2016-04-22 12:58   ` Alex Bennée
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 07/11] tb hash: hash phys_pc, pc, and flags with xxhash Emilio G. Cota
2016-04-22 12:58   ` Alex Bennée
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 08/11] qht: QEMU's fast, resizable and scalable Hash Table Emilio G. Cota
2016-04-22 14:04   ` Alex Bennée
2016-04-24 20:01   ` Richard Henderson
2016-04-24 21:58     ` Emilio G. Cota
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 09/11] qht: add test program Emilio G. Cota
2016-04-22 14:35   ` Alex Bennée
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 10/11] tb hash: track translated blocks with qht Emilio G. Cota
2016-04-28 13:27   ` Alex Bennée
2016-04-19 23:07 ` [Qemu-devel] [PATCH v3 11/11] translate-all: add tb hash bucket info to 'info jit' dump Emilio G. Cota
2016-04-20 15:21   ` Richard Henderson
2016-04-22 14:38   ` Alex Bennée
2016-04-22 17:41   ` Richard Henderson
2016-04-22 19:23     ` Emilio G. Cota
2016-04-22 19:59     ` Richard Henderson
2016-04-22 23:57       ` Emilio G. Cota
2016-04-24 19:46         ` Richard Henderson
2016-04-24 22:06           ` Emilio G. Cota [this message]
2016-04-27  2:43             ` Emilio G. Cota
2016-04-28 16:37               ` Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160424220651.GB1122@flamenco \
    --to=cota@braap.org \
    --cc=alex.bennee@linaro.org \
    --cc=crosthwaite.peter@gmail.com \
    --cc=mttcg@greensocs.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=serge.fdrv@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).