Bug 53506

Summary: Modify RandomNumberSeed.h to use USE(MERSENNE_TWISTER_19937)
Product: WebKit Reporter: Daniel Bates <dbates>
Component: Web Template FrameworkAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, buildbot, dglazkov, eric, paroga, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Other   
Bug Depends on: 53253    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch tonikitoo: review+

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)