[VOTE] Object scoped RNG implementation View as Markdown

Hi Go Kudo,

First, let me say that I believe we need such implementation in PHP and I
would like to see object scoped RNG as part of the standard. However, I
have voted no for a number of reasons. Let me list them from the
perspective of a noob PHP user.

• I really do not understand why we are introducing new functions. Can't
  the classes implement the necessary methods to get integers, doubles, and
  string of bytes? As a new user I would be completely overwhelmed by all
  these functions: rand(), mt_rand(), rng_int(), rng_next(), rng_next64().
  Which one should I use? What is the difference between rng_next()
  and rng_next64?
• As soon as I left the RFC page I forgot what the new classes were called.
  I still can't tell from memory what's their name. I understand what they
  mean, but they are definitely not friendly names.
• What's the difference between MT19937 and XorShift128Plus? They are
  different algorithms but which one should I pick? I tested the
  implementation locally and I see no difference in performance.
• I am not a fan of adding a new optional parameter to shuffle() and
  friends. I'd prefer to have a method in the class that I can pass an array
  to.
• What is the default seed? Do I have to provide a seed each time? Why
  can't the seed be done automatically?
• Signed? Unsigned? As far as I know, PHP doesn't have unsigned integers.
  What's the real-life purpose of this flag?
• I don't see any use in supporting userland implementations. Why can't
  they create separate libraries? I don't know about performance, but if
  someone wants to have custom RNG then I don't think they worry about
  performance.
• When using the functions the performance was 50% worse than when calling
  ->next() directly. Is this right or is the implementation going to be
  optimized further? The fastest way to get a random number seems to be
  mt_rand() based on my tests.

I would rather like to see a single class called RNG/Random that implements
RNG/RandomInterface. The constructor of the class would take 2 arguments.
The first is the algorithm with a default either MT or XORShift. The second
is an optional seed. If no seed is provided then the seed is generated
automatically like in mt_srand(). The class would then implement methods
like: nextInt(), nextDouble(), nextBytes(), arrayShuffle(),
stringShuffle(), randomArrayKeys(). I would keep the standard functions as
they are. Let them use MT by default. We could even deprecate them in
future if this takes off.

This would make it painfully obvious what the class does and how to use it.
No more procedural code. I would also make the class final so that you
can't inherit from it, but that is highly opinion-based.
Now that I have written this, I read previous conversations and it looks to
me like what I would like is what you had previously.

I'm sorry if I complain too much, but I would like to see something like
this implemented, just not like you are proposing right now. It is too
messy for me and I know I wouldn't like it if I had to use it.

Changed my vote to "no" after this in-detail review.

Overall, I was conflicted, and voted "yes" especially because we do really
need a better and reproducible seeding solution for RNG, and the current
API is insufficient.

The API proposed so far is quite terrible though, and if the vote passed,
we'd have to stick with it for an infinity of years.

Hi Go Kudo,

First, let me say that I believe we need such implementation in PHP and I
would like to see object scoped RNG as part of the standard. However, I
have voted no for a number of reasons. Let me list them from the
perspective of a noob PHP user.

• I really do not understand why we are introducing new functions. Can't
  the classes implement the necessary methods to get integers, doubles, and
  string of bytes? As a new user I would be completely overwhelmed by all
  these functions: rand(), mt_rand(), rng_int(), rng_next(), rng_next64().
  Which one should I use? What is the difference between rng_next()
  and rng_next64?
• As soon as I left the RFC page I forgot what the new classes were called.
  I still can't tell from memory what's their name. I understand what they
  mean, but they are definitely not friendly names.
• What's the difference between MT19937 and XorShift128Plus? They are
  different algorithms but which one should I pick? I tested the
  implementation locally and I see no difference in performance.
• I am not a fan of adding a new optional parameter to shuffle() and
  friends. I'd prefer to have a method in the class that I can pass an array
  to.
• What is the default seed? Do I have to provide a seed each time? Why
  can't the seed be done automatically?
• Signed? Unsigned? As far as I know, PHP doesn't have unsigned integers.
  What's the real-life purpose of this flag?
• I don't see any use in supporting userland implementations. Why can't
  they create separate libraries? I don't know about performance, but if
  someone wants to have custom RNG then I don't think they worry about
  performance.
• When using the functions the performance was 50% worse than when calling
  ->next() directly. Is this right or is the implementation going to be
  optimized further? The fastest way to get a random number seems to be
  mt_rand() based on my tests.

I would rather like to see a single class called RNG/Random that implements
RNG/RandomInterface. The constructor of the class would take 2 arguments.
The first is the algorithm with a default either MT or XORShift. The second
is an optional seed. If no seed is provided then the seed is generated
automatically like in mt_srand(). The class would then implement methods
like: nextInt(), nextDouble(), nextBytes(), arrayShuffle(),
stringShuffle(), randomArrayKeys(). I would keep the standard functions as
they are. Let them use MT by default. We could even deprecate them in
future if this takes off.

This would make it painfully obvious what the class does and how to use it.
No more procedural code. I would also make the class final so that you
can't inherit from it, but that is highly opinion-based.
Now that I have written this, I read previous conversations and it looks to
me like what I would like is what you had previously.

I'm sorry if I complain too much, but I would like to see something like
this implemented, just not like you are proposing right now. It is too
messy for me and I know I wouldn't like it if I had to use it.

Regards,
Kamil

I also didn't pay close attention to the previous discussion, but reading the RFC I agree with all of this. The functionality proposed is good and needed, but the API is a mess. What Kamil suggests is far better, and fully commits to being OOPy. Given that the use case is for situations where you need a predictable and repeatable random sequence, such as Fibers or threads or such, going all-in on an object seems like the correct approach.

One thing I'm not 100% clear on from the RFC, does this also deprecate random_int()/random_bytes()? Those are (AFAIK) unseeded, so they seem like they'd continue to serve their current purpose, but it's not clear from the RFC.

Voting no for now, but I would welcome a resubmission with a cleaner API, even in this cycle.

--Larry Garfield

This RFC seems to have a lot of problems.
But I would like to answer the question first.

• I really do not understand why we are introducing new functions. Can't
  the classes implement the necessary methods to get integers, doubles, and
  string of bytes? As a new user I would be completely overwhelmed by all
  these functions: rand(), mt_rand(), rng_int(), rng_

This was done with the intention of lightening the burden of RNG
implementation in PHP.
At the time of the discussion, it was assumed that RNGInterface would also
provide alternative functions such as shuffle() and array_rand().

• As soon as I left the RFC page I forgot what the new classes were
  called. I still can't tell from memory what's their name. I understand what
  they mean, but they are definitely not friendly names.

The class name is based on the random number generation algorithm. As the
namespace is RNG, I think this is straightforward enough.

What's the difference between MT19937 and XorShift128Plus? They are
different algorithms but which one should I pick? I tested the
implementation locally and I see no difference in performance.

Basically, it assumes that XorShift128+ is used.
MT19937 is implemented in case you need to reproduce the behavior of
traditional MT.

I am not a fan of adding a new optional parameter to shuffle() and
friends. I'd prefer to have a method in the class that I can pass an array
to.

As I have said many times before, this was the original implementation plan.
The PECL orng extension provides functionality that is largely based on
this.

Signed? Unsigned? As far as I know, PHP doesn't have unsigned integers.
What's the real-life purpose of this flag?

The idea is to use RNG\MT19937 and rng_next() to make them behave the
same as mt_srand() and mt_rand().
This will allow all mt_srand() to be replaced by the new object-based RNG
implementation.

I don't see any use in supporting userland implementations. Why can't
they create separate libraries? I don't know about performance, but if
someone wants to have custom RNG then I don't think they worry about
performance.

This can be useful when you want to generate results from a pre-determined
random number table. See this test case.
https://github.com/php/php-src/blob/6834426452b151c36e4d1e292ad997d6bcf139ae/ext/standard/tests/rng/function_rng_bytes_string.phpt

When using the functions the performance was 50% worse than when calling
->next() directly. Is this right or is the implementation going to be
optimized further? The fastest way to get a random number seems to be
mt_rand() based on my tests.

This is the cost of increased versatility. I'm not so convinced about this
either.
Calling next64() in a 32bit environment will give incorrect results.
rng_next64() determines the execution environment beforehand, and throws
ValueError if it is not available.
Since it is difficult to expect such "error checking" from a userland
implementation, this is the way it is.

This RFC ballot should be rescinded. There was too little discussion
beforehand.
I will create a new proposal based on the feedback I received. I would
appreciate your active input so that this does not happen during the voting
phase.

2021年4月3日(土) 6:56 Kamil Tekiela tekiela246@gmail.com:

Hi Go Kudo,

First, let me say that I believe we need such implementation in PHP and I
would like to see object scoped RNG as part of the standard. However, I
have voted no for a number of reasons. Let me list them from the
perspective of a noob PHP user.

• I really do not understand why we are introducing new functions. Can't
  the classes implement the necessary methods to get integers, doubles, and
  string of bytes? As a new user I would be completely overwhelmed by all
  these functions: rand(), mt_rand(), rng_int(), rng_next(), rng_next64().
  Which one should I use? What is the difference between rng_next()
  and rng_next64?
• As soon as I left the RFC page I forgot what the new classes were
  called. I still can't tell from memory what's their name. I understand what
  they mean, but they are definitely not friendly names.
• What's the difference between MT19937 and XorShift128Plus? They are
  different algorithms but which one should I pick? I tested the
  implementation locally and I see no difference in performance.
• I am not a fan of adding a new optional parameter to shuffle() and
  friends. I'd prefer to have a method in the class that I can pass an array
  to.
• What is the default seed? Do I have to provide a seed each time? Why
  can't the seed be done automatically?
• Signed? Unsigned? As far as I know, PHP doesn't have unsigned integers.
  What's the real-life purpose of this flag?
• I don't see any use in supporting userland implementations. Why can't
  they create separate libraries? I don't know about performance, but if
  someone wants to have custom RNG then I don't think they worry about
  performance.
• When using the functions the performance was 50% worse than when calling
  ->next() directly. Is this right or is the implementation going to be
  optimized further? The fastest way to get a random number seems to be
  mt_rand() based on my tests.

I would rather like to see a single class called RNG/Random that
implements RNG/RandomInterface. The constructor of the class would take 2
arguments. The first is the algorithm with a default either MT or XORShift.
The second is an optional seed. If no seed is provided then the seed is
generated automatically like in mt_srand(). The class would then implement
methods like: nextInt(), nextDouble(), nextBytes(), arrayShuffle(),
stringShuffle(), randomArrayKeys(). I would keep the standard functions as
they are. Let them use MT by default. We could even deprecate them in
future if this takes off.

This would make it painfully obvious what the class does and how to use
it. No more procedural code. I would also make the class final so that you
can't inherit from it, but that is highly opinion-based.
Now that I have written this, I read previous conversations and it looks
to me like what I would like is what you had previously.

I'm sorry if I complain too much, but I would like to see something like
this implemented, just not like you are proposing right now. It is too
messy for me and I know I wouldn't like it if I had to use it.

Regards,
Kamil