> but I don't think rng_rand() should support calling without arguments.
This is a backwards-compatibility leftover in mt_rand() and we should not
carry it over into a new API.
While this is true, the current implementation of MT in PHP relies on
global state, and I believe that having a compatible implementation that
can eliminate this is a good option.
If PHP supports some parallel implementation in the future, and
`mt_srand()` and `mt_rand()` are deprecated, it may be useful as an
alternative method.
> You should probably mention that the classes can be serialized in the
RFC.
I had forgotten about this one. I've added it. Thanks.
> Naming
I think this needs to be discussed further.
The name RNG\RNGInterface is a compromise at this point, and is mainly
based on the PSR conventions. However, PSR is only a userland convention
and I am not sure if the core should take this into account.
However, we also believe that the name is easy to understand, although it
is somewhat redundant.
> 64-bit
I've been thinking for a while about what to do since then, and I think
your suggestion is probably the most appropriate.
That is, remove the `RNG64Interface` and the `next64()` method, and use the
`rng_range()` function to generate random numbers beyond 32 bits.
However, there is a concern. RNG implementations will no longer be able to
return native 64-bit random numbers for seed values.
Given the class name `XorShift128Plus` , users might expect to return the
number `6233086606872742541` when `12345` is the seed value.
On the other hand, it is possible to pseudo-generate 64-bit random numbers
by bit-shifting even for 32-bit RNGs.
I thought of using this to include `next64()` in the `RNGInterface` itself,
but then the problem of userland implementation comes into play.
Originally, the `next64()` method should always throw an exception if
`PHP_INT_SIZE >= 8`, but if you allow userland implementation, it is
implementation dependent.
To solve these problems, how about adding a function like the following? It
doesn't seem like a very good idea, but...
`rng_next(RNG\RNGInterface $rng, bool $unsigned = true): int`
`rng_next64(RNG\RNGInterface $rng, bool $unsigned = true): int`
These will do a bit shift and return an unsigned integer if `$unsigned` is
true. Otherwise, they return the value as generated by the RNG.
Of course, if `rng_next64()` is called in a 32bit environment, it will
throw a `ValueError` exception.
In this case, however, the interface is no longer an interface. Perhaps it
should be an abstract class, like this
```php
abstract class AbstractRNG
{
abstract protected function next(): int;
abstract protected function next64(): int;
}
```
It may also be useful to have a trait like this
```php
trait RNG64Support
{
abstract function next(): int;
protected function next64(): int
{
$ret = $this->next();
return $ret << 32 | $this->ret();
}
}
```
It's pretty "exotic" for a core implementation. :o
Does anyone have any good ideas?
After all this time, I'm not good at English, and I apologize if this is
inappropriate.
Regards,
Go Kudo
2021年1月20日(水) 1:19 Nikita Popov <nikita....@gmail.com>:
> On Mon, Jan 18, 2021 at 5:29 PM Go Kudo <zeriyo...@gmail.com> wrote:
>
>> RFC and implementation updated.
>>
>> https://wiki.php.net/rfc/object_scope_prng
>> https://github.com/php/php-src/pull/6568
>>
>> > MT19937 returns a signed 32bit number as a result of the `next()`
>> method;
>> the mt_rand() function returns a bit-shifted unsigned 31bit number, so the
>> results are not identical.
>>
>> The `rng_range()` function has been renamed to `rng_rand()` and returns an
>> unsigned 31-bit integer like the `mt_rand()` function if only the $rng
>> argument is received.
>>
>> ```php
>> mt_srand(1234);
>> mt_rand() === rng_rand(new \RNG\MT19937(1234)); // true
>> ```
>>
>> This allows to completely replace the `mt_rand()` function with the
>> RNG\MT19937.
>>
>> I said I wanted to initiate a vote, but there seems to be too much
>> discussion missing for that.
>> I'd like to wait and see for a while.
>>
>> Regards,
>> Go Kudo
>>
>> 2021年1月17日(日) 20:02 Go Kudo <zeriyo...@gmail.com>:
>>
>> > Updated the RFC and fixed the implementation.
>> > Also made some additions to the RFC about when this feature might be
>> > useful.
>> >
>> > RFC: https://wiki.php.net/rfc/object_scope_prng
>> > Implementation PR: https://github.com/php/php-src/pull/6568 (All CI
>> > passed)
>> >
>> > The main points are as follows:
>> >
>> > - The implementation now meets all requirements. (maybe)
>> > - Implemented MT19937, which is compatible with PHP standard MT. The
>> > consistency of these is ensured by tests.
>> > - The implementation is now what we used to call TypeII.
>> > - All RNG implementations can now be inherited like regular classes. Use
>> > faster calls only if the class is an internal class.
>> > - The implementation has been heavily modified and the quality of the
>> code
>> > has been improved.
>> >
>> > Currently, there are a few concerns:
>> >
>> > - MT19937 returns a signed 32bit number as a result of the `next()`
>> > method; the mt_rand() function returns a bit-shifted unsigned 31bit
>> number,
>> > so the results are not identical.
>> > - You can get the equivalent result with `$rng->next() << 1 &
>> > PHP_INT_MAX`, but maybe you should have a `nextUnsigned31()` method.
>> > - Currently, the test uses `$rng->next() << 1 & PHP_INT_MAX`.
>> > - The implementation of MT19937 is redundant. This can be merged.
>> >
>> > Anyway, I think the code is finally ready for review.
>> > Once that is done, we will start voting on RFCs.
>> >
>> > Regards,
>> > Go Kudo
>>
>
> Overall I like the proposal. Some notes:
>
> * I know you only just changed it, but I don't think rng_rand() should
> support calling without arguments. This is a backwards-compatibility
> leftover in mt_rand() and we should not carry it over into a new API.
>
> * You should probably mention that the classes can be serialized in the
> RFC. Maybe you want to write out all the methods so it's also visible how
> the constructors for the classes look like.
>
> * Naming: RNG\RNGInterface seems a bit redundant. Of course, we can't make
> it RNG\Interface. RNG\Generator would be an option. I'm not sure about
> this. Maybe RNG\RNGInterface is fine.
>
> * 64-bit: I looked over your implementation, and I think your approach to
> handling 64-bits is correct. You only call next64() if the requested range
> is larger than 32-bit, and that can only happen on 64-bit systems, thus
> guaranteeing consistency of results (where they can be consistent at all).
> What I'm unsure about is the way this gets exposed to userland via next()
> and next64() methods. I think fetching of 64-bit values is mainly
> interesting as an internal optimization, and not so much important to be
> part of the RNGInterface API. The user is intended to get numbers via
> rng_rand(), not by directly calling next(). A possibility here would be to
> drop the RNG64Interface and next64() method, have next() always return
> 32-bit, but retain the internal next64 API. For this to make sense, we'd
> need two subsequent internal next() calls to return the same data as a
> single next64() call (i.e. store the result and first return one half then
> the other). Would this make sense to you?
>
> Regards,
> Nikita
>
