Modernize the PRNG used for rand() by using a Configure option - #24105
Modernize the PRNG used for rand() by using a Configure option#24105scottchiefbaker wants to merge 16 commits into
rand() by using a Configure option#24105Conversation
9c6eb8c to
8b56090
Compare
8b56090 to
bb9d9b5
Compare
40d4a6d to
1b78937
Compare
9002fb7 to
b0c2c62
Compare
rand() by add a Configure optionrand() by adding a Configure option
rand() by adding a Configure optionrand() by using a Configure option
b0c2c62 to
de7f507
Compare
| const uint64_t word = ((prng.state >> ((prng.state >> 59) + 5)) ^ prng.state) * 12605985483714917081ull; | ||
| prng.state = prng.state * 6364136223846793005ull + prng.inc; |
There was a problem hiding this comment.
Are these fairly arbitrary numbers, or did they get created by some specific process? Either way, I'd like to see some sort of comment explaining why these numbers in particular.
There was a problem hiding this comment.
That algorithm was designed by people WAY smarter than me. Per a previous iteration of this PR I added notes in prng.h with links to various documents talking about PCG64 and how to choose a good PRNG:
// Notes:
// https://zephyrtronium.github.io/articles/randomness.html
// https://docs.oracle.com/en/java/javase/21/core/choosing-prng-algorithm.html
// https://github.com/alvoskov/SmokeRand
TLDR; PCG64 has been tested and vetted by people smarter than me.
There was a problem hiding this comment.
I think it could use a reference, maybe https://en.wikipedia.org/wiki/Permuted_congruential_generator
I looked for more "original" references, but the original paper doesn't have the constants that I saw.
| // Splitmix64 is defined in util.c | ||
| static U64 splitmix64(U64 *state); | ||
|
|
||
| typedef struct { uint64_t state; uint64_t inc; } pcg64_random_t; |
There was a problem hiding this comment.
I suspect this should be U64 instead of uint64_t
There was a problem hiding this comment.
I think it could be more consistent, some places use U64, some uint64_t
We do use uint64_t in a few other places.
|
As we're now in the pre-release change freeze ahead of 5.44.0, this is no longer a suitable candidate for merge. But that means we can spend some time getting it nicely lined up ready for the 5.45.x development series. |
|
I am be interested in helping get this landed, but I lack the requisite core Perl knowledge to get it done. I'm asking for someone more knowledgeable about the Perl internals to adopt this and get it committed. I will be happy to help test and offer implementation advice. |
|
PR #24294 adds threading support, amongst other changes, but hasn't received any comments. |
b786199 to
2b7518b
Compare
|
I pulled in the changes from @tonycoz's branch and made a couple of code clean-ups. Overall it looks good and does what we need it to. This branch should be mergeable whenever we open things up for v5.45. |
|
This script outputs the average number of bits of randomness seen in use strict;
use warnings;
use v5.16;
use List::Util qw( sum );
sub avg { sum(@_)/@_ }
###############################################################################
my @data;
for (1 .. 1000) {
my $str = sprintf("%064b", int(rand() * 2**64 - 1));
my ($zeros) = $str =~ m/(0*)$/;
push(@data, 64 - length($zeros) + 1);
}
printf("~%0.2f bits\n", avg(@data));Prior to this patch, I consistently get ~48 bits of randomness. After landing this we get the full 53 bits of entropy a double can hold. Getting more bits of entropy is nice, but we also get much better statistical properties from a new PRNG as well. |
|
Using Smokerand we can see that PCG64 is significantly faster than drand48. This change should give us a boost in |
I'm surprised by that - perl's implementation is just a multiplication and an add on 64-bit systems, while PCG64 is 2 multiplies and several other ops. Did you test perl's drand48() or the system drand48()? |
|
The speed comparison results are direct from Smokerand comparing the two algorithms. I was surprised also. vs |
237fc33 to
c145c04
Compare
c145c04 to
9f10ad7
Compare
9f10ad7 to
12c44a4
Compare
since we may not be using drand48, just something that returns double (at least for now)
Some of this will be further changed: - moved the pcg64_random_t type to util.h so it's visible to intrpvar.h - changed the PL_RANDOM_STATE_TYPE to pcg64_random_t - changed the internal RNG to reference the new RNG and changed the macro names to something more generic (without "drand48") - added pcg64_random_t * parameters to the seed and RNG generation functions (and rename with _r) so we can make them thread safe and eliminate the global non-thread-safe random state - Needed to move the functions out of prng.h so porting/args_assert could find the function definitions and eliminate prng.h - re-define the original non-thread-safe functions as macro wrappers that pass in the state to the _r versions of the functions I plan to move all of the random stuff out of util.h/util.c into perlrand.h and random.c.
Perl supports larger floats, the existing code would drop bits of randomness on the floor, avoid that.
Make it easier to find the bits of perl that deal with random number generation and seeding.
This is an old artifact and just needs to be cleaned up
a79b0d0 to
70540e7
Compare
| #include <sys/random.h> | ||
| #endif | ||
|
|
||
| // https://prng.di.unimi.it/#remarks |
There was a problem hiding this comment.
perlhack disallows C++ (//) style comments (I don't know why)
Per past discussion on p5p we know the
drand48we use as a PRNG has limitations. This PR attempts to cleanly upgrade the PRNG used forrand()calls, while leaving the existing drand48 code in place for internal Perl stuff (hash seed, etc). I foundrandfuncandseedfuncoptions in Configure that allow us to point at a new PRNG without affecting the existing code.I created a new
prng.hfile which is largely self-contained except for a couple of additions toembed.fncandConfigure. I have included two new PRNG implementations: PCG64 (my preference)and xoroshiro128** as options.Completed items
srand()functionality works as expectedrand()outputs the full 53 bit state capable from a double (drand48 could only do 48 bits)./perl -I lib -E 'for (1..5) { printf("%064b\n", rand() * 2**64-1); }'TODO
prng.hdoes not seem to be rebuilt consistently after changes. Do I need to add this new file to build system?make regenputs the functions prototypes in a weird location "Used in locale.c and perl.c"rand64()?Alternate options
rand()is "good enough"Random::Simpleis a drop-in replacement forrand()andsrand()already