[RFC] Add Random Extension (before: Add Random class)

Hi Go Kudo,

I find this iteration acceptable, but I have one last complaint. Why the
double namespace? Can it not be simply Random/RandomNumberGenerator?

Can you also clarify what happens when serialization or cloning fails?

Regards,
Kamil

Hello Internals.

RFC has been reorganized for finalization.

https://wiki.php.net/rfc/rng_extension

The changes from the previous version are as follows:

• Changed again to a class-based approach. The argument can be omitted, in
  which case an instance of XorShift128Plus will be created automatically.
• Future scope was specified in the RFC and the functionality was separated
  as a Random extension.
• Changed to separate it as a Random extension and use the appropriate
  namespace.
• In order to extend the versatility of the final class, Random, a
  RandomInterface has been added, similar in approach to the
  DateTimeInterface.

I've done a tidy implementation to make this final, but I'm currently
suffering from error detection by Valgrind for unknown reasons.

Implementation is here: https://github.com/php/php-src/pull/7079

This can be reproduced with the following code.

# Success
$ valgrind ./sapi/cli/php -r '$random = new Random(); $random->nextInt();'
==95522== Memcheck, a memory error detector
==95522== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==95522== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==95522== Command: ./sapi/cli/php -r $random\ =\ new\ Random();\
$random-\>nextInt();
==95522==
==95522==
==95522== HEAP SUMMARY:
==95522==     in use at exit: 1,286 bytes in 32 blocks
==95522==   total heap usage: 28,445 allocs, 28,413 frees, 4,333,047 bytes
allocated
==95522==
==95522== LEAK SUMMARY:
==95522==    definitely lost: 0 bytes in 0 blocks
==95522==    indirectly lost: 0 bytes in 0 blocks
==95522==      possibly lost: 0 bytes in 0 blocks
==95522==    still reachable: 1,286 bytes in 32 blocks
==95522==         suppressed: 0 bytes in 0 blocks
==95522== Rerun with --leak-check=full to see details of leaked memory
==95522==
==95522== For counts of detected and suppressed errors, rerun with: -v
==95522== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

# Fail
$ valgrind ./sapi/cli/php -r '$random = new Random(); $random->nextInt()
=== $random->nextInt();'
==95395== Memcheck, a memory error detector
==95395== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==95395== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==95395== Command: ./sapi/cli/php -r $random\ =\ new\ Random();\
$random-\>nextInt()\ ===\ $random-\>nextInt();
==95395==
==95395== Conditional jump or move depends on uninitialised value(s)
==95395==    at 0x966925: ZEND_IS_IDENTICAL_SPEC_VAR_VAR_HANDLER
(zend_vm_execute.h:27024)
==95395==    by 0x99AC27: execute_ex (zend_vm_execute.h:57236)
==95395==    by 0x99C902: zend_execute (zend_vm_execute.h:59026)
==95395==    by 0x8DB6B4: zend_eval_stringl (zend_execute_API.c:1191)
==95395==    by 0x8DB861: zend_eval_stringl_ex (zend_execute_API.c:1233)
==95395==    by 0x8DB8D6: zend_eval_string_ex (zend_execute_API.c:1243)
==95395==    by 0xA4DAE4: do_cli (php_cli.c:995)
==95395==    by 0xA4E8E2: main (php_cli.c:1366)
==95395==
==95395==
==95395== HEAP SUMMARY:
==95395==     in use at exit: 1,286 bytes in 32 blocks
==95395==   total heap usage: 28,445 allocs, 28,413 frees, 4,333,070 bytes
allocated
==95395==
==95395== LEAK SUMMARY:
==95395==    definitely lost: 0 bytes in 0 blocks
==95395==    indirectly lost: 0 bytes in 0 blocks
==95395==      possibly lost: 0 bytes in 0 blocks
==95395==    still reachable: 1,286 bytes in 32 blocks
==95395==         suppressed: 0 bytes in 0 blocks
==95395== Rerun with --leak-check=full to see details of leaked memory
==95395==
==95395== For counts of detected and suppressed errors, rerun with: -v
==95395== Use --track-origins=yes to see where uninitialised values come
from
==95395== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

However, the detection is internal to the Zend VM and the cause has not
been identified. From the code, it looks like memory management is being
done properly.

I have a somewhat tricky way of allocating memory to make the process
common, do I need to give some hints to Valgrind?

If you know, I would appreciate your advice on this issue.

Regards,
Go Kudo

Hello Internals.

RFC has been reorganized for finalization.

https://wiki.php.net/rfc/rng_extension
I think this is a good reorganization.
I would like to point out that this does not quite follow the Namespace Policy
(https://wiki.php.net/rfc/namespaces_in_bundled_extensions):
Notably it says:
All symbols defined in the extension should be part of the extension's top-level
namespace or a sub-namespace.
This means that the Random class and RandomInterface interface should be
Random\Random and Random\RandomInterface respectively.
The symbols in the Random\NumberGenerator sub-namespace are okay,
as per the policy.
Note, however, that the policy says "should", and if there is a good argument
made, the guidelines must not be followed.

Hello Internals.

RFC has been reorganized for finalization.

https://wiki.php.net/rfc/rng_extension

The changes from the previous version are as follows:

• Changed again to a class-based approach. The argument can be omitted, in
  which case an instance of XorShift128Plus will be created automatically.
• Future scope was specified in the RFC and the functionality was separated
  as a Random extension.
• Changed to separate it as a Random extension and use the appropriate
  namespace.
• In order to extend the versatility of the final class, Random, a
  RandomInterface has been added, similar in approach to the
  DateTimeInterface.

I've done a tidy implementation to make this final, but I'm currently
suffering from error detection by Valgrind for unknown reasons.

Implementation is here: https://github.com/php/php-src/pull/7079

This can be reproduced with the following code.

This looks a lot nicer!

Question: Why interface RandomInterface but interface RandomNumberGenerator? I would expect either both to have an Interface suffix or neither to. (Preferably neither.)

For that matter, I don't get why RandomInterface exists, if you'll just always use Random all the time, and it's final.

--Larry Garfield

Hello Internals.

RFC has been reorganized for finalization.

https://wiki.php.net/rfc/rng_extension

The changes from the previous version are as follows:

• Changed again to a class-based approach. The argument can be omitted, in
  which case an instance of XorShift128Plus will be created automatically.
• Future scope was specified in the RFC and the functionality was separated
  as a Random extension.
• Changed to separate it as a Random extension and use the appropriate
  namespace.
• In order to extend the versatility of the final class, Random, a
  RandomInterface has been added, similar in approach to the
  DateTimeInterface.

The updated proposal looks quite nice :) I think this is close to done.
Some small bits of feedback:

• The first bit is just clarification. After a cursory look at the
  implementation, my understanding is that the getInt(), shuffleArray() and
  shuffleString() APIs will always produce consistent results on 32-bit and
  64-bit, as long as your inputs are 32-bit as well (i.e., min/max are 32-bit
  and string is smaller than 4G). Is that correct? The only APIs that would
  exhibit different behavior are nextInt() and getBytes(), right?
• Looking at the implementation, nextInt() performs a >> 1 operation on
  the RNG result. I assume the motivation is to get back a non-negative
  number. But why do we want that? The "nextInt()" name doesn't really
  indicate that it's a positive number. I think more generally, my question
  here may be "Why does this method exist at all? When would you use it
  instead of getInt()?"
• Another bit of clarification: For the user-defined RNG, which range is
  generate() expected to return? I assume that it must return the native
  integer size, i.e. 32-bit on 32-bit and 64-bit on 64-bit?
• I don't really get why we need RandomInterface. I think if the choice is
  between "final + interface" and "non-final without interface", I'd prefer
  the latter (though I'm also happy with "final without interface").
• I'm not entirely happy with the naming. Unfortunately, I don't have
  great suggestions either. I think in your current hierarchy, I would make
  the interface Random\NumberGenerator (with implementation in the
  sub-namespace), rather than Random\NumberGenerator\RandomNumberGenerator.

Regards,
Nikita

I've done a tidy implementation to make this final, but I'm currently

suffering from error detection by Valgrind for unknown reasons.

Implementation is here: https://github.com/php/php-src/pull/7079

This can be reproduced with the following code.

# Success
$ valgrind ./sapi/cli/php -r '$random = new Random(); $random->nextInt();'
==95522== Memcheck, a memory error detector
==95522== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==95522== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright
info
==95522== Command: ./sapi/cli/php -r $random\ =\ new\ Random();\
$random-\>nextInt();
==95522==
==95522==
==95522== HEAP SUMMARY:
==95522==     in use at exit: 1,286 bytes in 32 blocks
==95522==   total heap usage: 28,445 allocs, 28,413 frees, 4,333,047 bytes
allocated
==95522==
==95522== LEAK SUMMARY:
==95522==    definitely lost: 0 bytes in 0 blocks
==95522==    indirectly lost: 0 bytes in 0 blocks
==95522==      possibly lost: 0 bytes in 0 blocks
==95522==    still reachable: 1,286 bytes in 32 blocks
==95522==         suppressed: 0 bytes in 0 blocks
==95522== Rerun with --leak-check=full to see details of leaked memory
==95522==
==95522== For counts of detected and suppressed errors, rerun with: -v
==95522== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

# Fail
$ valgrind ./sapi/cli/php -r '$random = new Random(); $random->nextInt()
=== $random->nextInt();'
==95395== Memcheck, a memory error detector
==95395== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==95395== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright
info
==95395== Command: ./sapi/cli/php -r $random\ =\ new\ Random();\
$random-\>nextInt()\ ===\ $random-\>nextInt();
==95395==
==95395== Conditional jump or move depends on uninitialised value(s)
==95395==    at 0x966925: ZEND_IS_IDENTICAL_SPEC_VAR_VAR_HANDLER
(zend_vm_execute.h:27024)
==95395==    by 0x99AC27: execute_ex (zend_vm_execute.h:57236)
==95395==    by 0x99C902: zend_execute (zend_vm_execute.h:59026)
==95395==    by 0x8DB6B4: zend_eval_stringl (zend_execute_API.c:1191)
==95395==    by 0x8DB861: zend_eval_stringl_ex (zend_execute_API.c:1233)
==95395==    by 0x8DB8D6: zend_eval_string_ex (zend_execute_API.c:1243)
==95395==    by 0xA4DAE4: do_cli (php_cli.c:995)
==95395==    by 0xA4E8E2: main (php_cli.c:1366)
==95395==
==95395==
==95395== HEAP SUMMARY:
==95395==     in use at exit: 1,286 bytes in 32 blocks
==95395==   total heap usage: 28,445 allocs, 28,413 frees, 4,333,070 bytes
allocated
==95395==
==95395== LEAK SUMMARY:
==95395==    definitely lost: 0 bytes in 0 blocks
==95395==    indirectly lost: 0 bytes in 0 blocks
==95395==      possibly lost: 0 bytes in 0 blocks
==95395==    still reachable: 1,286 bytes in 32 blocks
==95395==         suppressed: 0 bytes in 0 blocks
==95395== Rerun with --leak-check=full to see details of leaked memory
==95395==
==95395== For counts of detected and suppressed errors, rerun with: -v
==95395== Use --track-origins=yes to see where uninitialised values come
from
==95395== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

However, the detection is internal to the Zend VM and the cause has not
been identified. From the code, it looks like memory management is being
done properly.

I have a somewhat tricky way of allocating memory to make the process
common, do I need to give some hints to Valgrind?

If you know, I would appreciate your advice on this issue.

Regards,
Go Kudo

Le 26/06/2021 à 02:39, Go Kudo a écrit :

Hello Internals.

RFC has been reorganized for finalization.

https://wiki.php.net/rfc/rng_extension

1st I dislike the name "random_ext", why this "_ext" part ?

2nd why not following the standard process ?

1/ publish on pecl
2/ merge in php-src if enough success and good feedback

Remi

Hello Internals.

I mistakenly thought there was still time, but voting for a new RFC for PHP
8.1 has already closed.

I believe that an object-scoped random number generator is a necessity for
PHP and I will continue to work on it. We will continue to improve the RFC.

I will also work on getting it published in PECL so that features are
available quickly.

Regards,
Go Kudo

2021年6月26日(土) 9:39 Go Kudo zeriyoshi@gmail.com:

Hello Internals.

RFC has been reorganized for finalization.

https://wiki.php.net/rfc/rng_extension

The changes from the previous version are as follows:

• Changed again to a class-based approach. The argument can be omitted, in
  which case an instance of XorShift128Plus will be created automatically.
• Future scope was specified in the RFC and the functionality was
  separated as a Random extension.
• Changed to separate it as a Random extension and use the appropriate
  namespace.
• In order to extend the versatility of the final class, Random, a
  RandomInterface has been added, similar in approach to the
  DateTimeInterface.

I've done a tidy implementation to make this final, but I'm currently
suffering from error detection by Valgrind for unknown reasons.

Implementation is here: https://github.com/php/php-src/pull/7079

This can be reproduced with the following code.

# Success
$ valgrind ./sapi/cli/php -r '$random = new Random(); $random->nextInt();'
==95522== Memcheck, a memory error detector
==95522== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==95522== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright
info
==95522== Command: ./sapi/cli/php -r $random\ =\ new\ Random();\
$random-\>nextInt();
==95522==
==95522==
==95522== HEAP SUMMARY:
==95522==     in use at exit: 1,286 bytes in 32 blocks
==95522==   total heap usage: 28,445 allocs, 28,413 frees, 4,333,047 bytes
allocated
==95522==
==95522== LEAK SUMMARY:
==95522==    definitely lost: 0 bytes in 0 blocks
==95522==    indirectly lost: 0 bytes in 0 blocks
==95522==      possibly lost: 0 bytes in 0 blocks
==95522==    still reachable: 1,286 bytes in 32 blocks
==95522==         suppressed: 0 bytes in 0 blocks
==95522== Rerun with --leak-check=full to see details of leaked memory
==95522==
==95522== For counts of detected and suppressed errors, rerun with: -v
==95522== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

# Fail
$ valgrind ./sapi/cli/php -r '$random = new Random(); $random->nextInt()
=== $random->nextInt();'
==95395== Memcheck, a memory error detector
==95395== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==95395== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright
info
==95395== Command: ./sapi/cli/php -r $random\ =\ new\ Random();\
$random-\>nextInt()\ ===\ $random-\>nextInt();
==95395==
==95395== Conditional jump or move depends on uninitialised value(s)
==95395==    at 0x966925: ZEND_IS_IDENTICAL_SPEC_VAR_VAR_HANDLER
(zend_vm_execute.h:27024)
==95395==    by 0x99AC27: execute_ex (zend_vm_execute.h:57236)
==95395==    by 0x99C902: zend_execute (zend_vm_execute.h:59026)
==95395==    by 0x8DB6B4: zend_eval_stringl (zend_execute_API.c:1191)
==95395==    by 0x8DB861: zend_eval_stringl_ex (zend_execute_API.c:1233)
==95395==    by 0x8DB8D6: zend_eval_string_ex (zend_execute_API.c:1243)
==95395==    by 0xA4DAE4: do_cli (php_cli.c:995)
==95395==    by 0xA4E8E2: main (php_cli.c:1366)
==95395==
==95395==
==95395== HEAP SUMMARY:
==95395==     in use at exit: 1,286 bytes in 32 blocks
==95395==   total heap usage: 28,445 allocs, 28,413 frees, 4,333,070 bytes
allocated
==95395==
==95395== LEAK SUMMARY:
==95395==    definitely lost: 0 bytes in 0 blocks
==95395==    indirectly lost: 0 bytes in 0 blocks
==95395==      possibly lost: 0 bytes in 0 blocks
==95395==    still reachable: 1,286 bytes in 32 blocks
==95395==         suppressed: 0 bytes in 0 blocks
==95395== Rerun with --leak-check=full to see details of leaked memory
==95395==
==95395== For counts of detected and suppressed errors, rerun with: -v
==95395== Use --track-origins=yes to see where uninitialised values come
from
==95395== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

However, the detection is internal to the Zend VM and the cause has not
been identified. From the code, it looks like memory management is being
done properly.

I have a somewhat tricky way of allocating memory to make the process
common, do I need to give some hints to Valgrind?

If you know, I would appreciate your advice on this issue.

Regards,
Go Kudo