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