Bug 53506 - Modify RandomNumberSeed.h to use USE(MERSENNE_TWISTER_19937)
Summary: Modify RandomNumberSeed.h to use USE(MERSENNE_TWISTER_19937)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on: 53253
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-01 09:54 PST by Daniel Bates
Modified: 2011-02-01 11:32 PST (History)
6 users (show)

See Also:


Attachments
Patch (3.19 KB, patch)
2011-02-01 10:12 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (2.59 KB, patch)
2011-02-01 10:20 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (2.45 KB, patch)
2011-02-01 10:32 PST, Daniel Bates
tonikitoo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2011-02-01 09:54:03 PST
Following up from bug #53253, we should guard code in RandomNumberSeed.h that uses the Mersenne Twister pseudorandom number generator with USE(MERSENNE_TWISTER_19937).
Comment 1 Daniel Bates 2011-02-01 10:12:03 PST
Created attachment 80780 [details]
Patch

I filed bug #53508 to add support for the Mersenne Twister PRNG for all ports that don't use srand(3) and rand(3).

I am open to suggestions on how best to write this patch.
Comment 2 WebKit Review Bot 2011-02-01 10:15:13 PST
Attachment 80780 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7680840
Comment 3 Patrick R. Gansterer 2011-02-01 10:18:21 PST
I don't like the return. What about the following?

#if OS(DARWIN)
// On Darwin we use arc4random which initialises itself.
#elif OS(WINCE)
// initialize rand()
srand(GetTickCount());
#elif COMPILER(MSVC) && defined(_CRT_RAND_S)
// On Windows we use rand_s which initialises itself
#elif PLATFORM(BREWMP)
// On Brew MP we use AEECLSID_RANDOM which initialises itself
#elif OS(UNIX)
// srandomdev is not guaranteed to exist on linux so we use this poor seed, this should be improved
timeval time;
gettimeofday(&time, 0);
srandom(static_cast<unsigned>(time.tv_usec * getpid()));
#else
srand(static_cast<unsigned>(time(0)));
#endif

#if USE(MERSENNE_TWISTER_19937)
// use rand() to initialize the Mersenne Twister random number generator.
unsigned long initializationBuffer[4];
initializationBuffer[0] = (rand() << 16) | rand();
initializationBuffer[1] = (rand() << 16) | rand();
initializationBuffer[2] = (rand() << 16) | rand();
initializationBuffer[3] = (rand() << 16) | rand();
init_by_array(initializationBuffer, 4);
#endif
Comment 4 Daniel Bates 2011-02-01 10:20:58 PST
Created attachment 80781 [details]
Patch

Updated patch.
Comment 5 Patrick R. Gansterer 2011-02-01 10:26:23 PST
Comment on attachment 80781 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=80781&action=review

> Source/JavaScriptCore/wtf/RandomNumberSeed.h:71
> +// FIXME: Add Mersenne Twister support for platforms that don't use srand(3) and rand(3).
> +// See <https://bugs.webkit.org/show_bug.cgi?id=53508>.

Is there any value in this comments? Other ports don't use rand and _don't_ need Mersenne Twister. (See comments above.)
Comment 6 Daniel Bates 2011-02-01 10:28:15 PST
(In reply to comment #5)
> (From update of attachment 80781 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80781&action=review
> 
> > Source/JavaScriptCore/wtf/RandomNumberSeed.h:71
> > +// FIXME: Add Mersenne Twister support for platforms that don't use srand(3) and rand(3).
> > +// See <https://bugs.webkit.org/show_bug.cgi?id=53508>.
> 
> Is there any value in this comments? Other ports don't use rand and _don't_ need Mersenne Twister. (See comments above.)

Will remove it.
Comment 7 Daniel Bates 2011-02-01 10:32:13 PST
Created attachment 80783 [details]
Patch

Updated patch based on Patrick Gansterer's comment and marked bug #53508 as invalid.
Comment 8 Daniel Bates 2011-02-01 10:50:43 PST
Committed r77260: <http://trac.webkit.org/changeset/77260>
Comment 9 Build Bot 2011-02-01 10:51:04 PST
Attachment 80780 [details] did not build on win:
Build output: http://queues.webkit.org/results/7680852
Comment 10 WebKit Review Bot 2011-02-01 11:32:42 PST
http://trac.webkit.org/changeset/77260 might have broken SnowLeopard Intel Release (Tests)