[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tests/qht-bench: Adjust rate computation and comparisons
From: |
Richard Henderson |
Subject: |
Re: [PATCH] tests/qht-bench: Adjust rate computation and comparisons |
Date: |
Tue, 23 Jun 2020 15:37:51 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 |
On 6/21/20 2:28 PM, Emilio G. Cota wrote:
>> - if (info->r < resize_threshold) {
>> + if (info->r <= resize_threshold) {
>> size_t size = info->resize_down ? resize_min : resize_max;
>> bool resized;
>
> This works, but only because info->r cannot be 0 since xorshift never
> returns it. (xorshift returns a random number in the range [1, u64max],
> a fact that I missed when I wrote this code.)
> If r were 0, then we would resize even if resize_threshold == 0.0.
>
> I think it will be easier to reason about this if we rename info->r
> to info->seed, and then have a local r = info->seed - 1. Then we can keep
> the "if random < threshold" form (and its negated "if random >= threshold"
> as below), which (at least to me) is intuitive provided that random's range
> is [0, threshold), e.g. [0.0, 1.0) with drand48(3).
Fair enough.
>> static void do_threshold(double rate, uint64_t *threshold)
>> {
>> + /*
>> + * For 0 <= rate <= 1, scale to fit in a uint64_t.
>> + *
>> + * For rate == 1, returning UINT64_MAX means 100% certainty: all
>> + * uint64_t will match using <=. The largest representable value
>> + * for rate less than 1 is 0.999999999999999889; scaling that
>> + * by 2**64 results in 0xfffffffffffff800.
>> + */
>> if (rate == 1.0) {
>> *threshold = UINT64_MAX;
>> } else {
>> - *threshold = (rate * 0xffff000000000000ull)
>> - + (rate * 0x0000ffffffffffffull);
>> + *threshold = rate * 0x1p64;
>
> I'm sorry this caused a breakage for some integration tests; I thought
> this was fixed in May with:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg01477.html
>
> Just for my own education, why isn't nextafter needed here?
I hoped I was being clear in the comment, but re-reading, it doesn't finish the
thought.
We have removed 1.0, so the rate values are between 0 and nextafter(1, 0) =
0x1.fffffffffffff00000p-1 = 0.999999999999999889.
Scaling by 2**64 results in an exact extract of the 53-bit mantessa, evenly
spread across 0 to 0xfffffffffffff800. Plus 1.0 -> UINT64_MAX, which we could
consider off-by-one its "proper" value.
If we scale by nextafter(0x1p64, 0), then the values are spread across 0 to
0xfffffffffffff000. The gap is twice as large between 1.0 and nextafter(1, 0).
r~