Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Math.random to use xorshift128+ #145

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Update Math.random to use xorshift128+ #145

merged 1 commit into from
Jan 21, 2016

Conversation

suwc
Copy link

@suwc suwc commented Jan 19, 2016

Addressing #31

Update the underlying pseudo-random number generator (PRNG) used by Math.random() from
linear congruential generator (LCG) to xorshift128+ (http://vigna.di.unimi.it/ftp/papers/xorshiftplus.pdf)
to be interoperable with other browsers:

http://jandemooij.nl/blog/2015/11/30/testing-math-random-crushing-the-browser/
https://bugzilla.mozilla.org/show_bug.cgi?id=322529
http://v8project.blogspot.com/2015/12/theres-mathrandom-and-then-theres.html

Also included:

  • inlining of Math.random() for X64
  • override switches for PRNG seeds

This change passed all 96 TestU01 Crush tests.

@msftclas
Copy link

Hi @suwc, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Suwei Chen). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@suwc
Copy link
Author

suwc commented Jan 19, 2016

Reviewers: @LouisLaf @ianwjhalliday @abchatra

@abchatra
Copy link
Contributor

👏

Change looks good to me.
Before merging, please wait for someone more knowledgeable in the community for the feedback as well.

@LouisLaf
Copy link
Collaborator

Looks good to me as well.

@ianwjhalliday
Copy link
Collaborator

Yep, LGTM too

seed = s1.QuadPart;

ThreadContext *threadContext = scriptContext->GetThreadContext();
threadContext->GetEntropy().AddThreadCycleTime();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like entropy class isn't used anymore, so imo we should get rid of it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entropy class is used for telemetry and cannot be removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that rand_s() calls may fail, entropy class call is added back in as a backup.

…ath.random() from

linear congruential generator (LCG) to xorshift128+ (http://vigna.di.unimi.it/ftp/papers/xorshiftplus.pdf)
to be interoperable with other browsers:

http://jandemooij.nl/blog/2015/11/30/testing-math-random-crushing-the-browser/
https://bugzilla.mozilla.org/show_bug.cgi?id=322529
http://v8project.blogspot.com/2015/12/theres-mathrandom-and-then-theres.html

Also included:
- inlining of Math.random() for X64
- override switches for PRNG seeds

This change pass all 96 TestU01 Crush tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants