REGRESSION(r192855): Math.random() always produces the same first 7 decimal points the first two invocations

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
Math.random() always produces the same first 7 decimal points the first two invocations

* STEPS TO REPRODUCE
1. Inspect about:blank
2. Run Math.random() twice
  => first 7 decimals are identical

* EXAMPLES

  > Math.random()
  < 0.39641653952203815
  < 0.39641654154667616

  > Math.random()
  < 0.7208129766006761
  < 0.7208129575360934

* NOTES
Seen in a comment on Hacker News: <https://news.ycombinator.com/item?id=11711912>
> 2) Math.random() returns the same result the first two calls on page load.

[Filip Pizlo]

Wow, that's hilarious.

[Joseph Pecoraro]

Caused by r192855:
<http://trac.webkit.org/changeset/192855>

[Geoffrey Garen]

Probably too many zeroes in the initial seed, since our seed API is 32 bit. You can probably fix this by:

        uint64_t seed64 = seed;
        m_low = (seed64 << 32) | seed64;
        m_high = (seed64 << 32) | seed64;

Or you could update the seed API to be 128bit.

[Radar WebKit Bug Importer]

<rdar://problem/26327851>

[Joseph Pecoraro]

(In reply to comment #3)
> Probably too many zeroes in the initial seed, since our seed API is 32 bit.
> You can probably fix this by:
> 
>         uint64_t seed64 = seed;
>         m_low = (seed64 << 32) | seed64;
>         m_high = (seed64 << 32) | seed64;
> 

After this the first 4 digits stayed nearly identical:

    >>> Math.random()
    0.41647092963078913
    >>> Math.random()
    0.41646612834590113

    >>> Math.random()
    0.47840231815551537
    >>> Math.random()
    0.47839187513838377

How about we just advance once after setting the seed?

    m_low = seed;
    m_high = seed;
    advance();

Do you see any disadvantages to this?

[Geoffrey Garen]

> How about we just advance once after setting the seed?
> 
>     m_low = seed;
>     m_high = seed;
>     advance();
> 
> Do you see any disadvantages to this?

LOL. Brain fart. It's guaranteed that no better solution is possible.

We have 128 bits of state that we want to set based on 32 bits of input. All solutions to this problem include some kind of bias unless they are... random!

[Joseph Pecoraro]

Created attachment 279167 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

An alternative test approach could be TestWebKitAPI creating a WeakRandom, or a jsc API that allows setting the random seed. But this tests Math.random which is web exposed so probably the safest.

[Geoffrey Garen]

Comment on attachment 279167 [details]
[PATCH] Proposed Fix

EWS failure seems spurious.

[WebKit Commit Bot]

Comment on attachment 279167 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 279167

Committed r201053: <http://trac.webkit.org/changeset/201053>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.