rfc:random_extension_improvement

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revisionPrevious revision
Next revision		Previous revision
rfc:random_extension_improvement [2022/06/20 15:56] – drop combinedlcg zeriyoshirfc:random_extension_improvement [2022/07/22 09:55] (current) – reword two issues as the problem (VS the proposed solution) guilliamxavier
Line 1: Line 1:
 ====== PHP RFC: Random Extension Improvement ====== ====== PHP RFC: Random Extension Improvement ======
-  * Version: 1.3+  * Version: 1.5.4
   * Date: 2022-06-16   * Date: 2022-06-16
   * Author: Go Kudo <g-kudo@colopl.co.jp> <zeriyoshi@gmail.com>   * Author: Go Kudo <g-kudo@colopl.co.jp> <zeriyoshi@gmail.com>
-  * Status: Under Discussion+  * Status: Implemented
   * First Published at: http://wiki.php.net/rfc/rng_extension   * First Published at: http://wiki.php.net/rfc/rng_extension
  
 ===== Introduction ===== ===== Introduction =====
  
-There are several issues with RFC: Random Extension 5.x that are already in the voting phase due to lack of proper discussion. +Several issues with [[rfc:rng_extension|the "Random Extension 5.x" RFC]] were only raised after voting started (due to lack of proper discussion):
- +
-Key issues include:+
  
 ==== Engine implementations are not final ==== ==== Engine implementations are not final ====
  
-The Random Extension has classes that are natively implemented as RNG engines, but they are not marked as final.+The Random Extension has classes that are natively implemented as RNG engines, but they are not marked as <php>final</php>.
 This allows classes to be created that inherit from native classes, but as stated in the previous RFC, user-implemented engines are inferior to native classes in terms of execution efficiency. This is true even for inheritance without method overrides, which often leads to confusion. This allows classes to be created that inherit from native classes, but as stated in the previous RFC, user-implemented engines are inferior to native classes in terms of execution efficiency. This is true even for inheritance without method overrides, which often leads to confusion.
  
-The extension already provides a Random\Engine interface with a single generate(): string method.+The extension already provides a ''Random\Engine'' interface with a single ''generate(): string'' method.
 Even if the native classes are made final, it is easy for the user to create an alternative class using delegates. Even if the native classes are made final, it is easy for the user to create an alternative class using delegates.
  
 This is clearly an API design error, and the native implementations of the interface should be marked as final. This is clearly an API design error, and the native implementations of the interface should be marked as final.
  
-==== CombinedLCG is outdated ====+==== Random\SerializableEngine is not useful ====
  
-The newly added Random\Engine\CombinedLCG is only for use with PHP's lcg_value() function.+This interface is a remnant from when the <php>Serializable</php> interface was still useful. This interface is no longer needed in PHP, as serializability is now determined by the existence of a magic method. 
 + 
 +For this reason, remove ''Random\SerializableEngine''. 
 + 
 +This means that an Engine that implements SerializableEngine will no longer implement it.  
 +However, serializability is determined by the implementation of the magic method in current PHP, so it has no effect. 
 + 
 +==== Random\Engine\CombinedLCG is low quality ==== 
 + 
 +The newly added ''Random\Engine\CombinedLCG'' is only for use with PHP's <php>lcg_value()</php> function.
 However, this algorithm is very classical and the quality of the output random numbers is at the lowest level. However, this algorithm is very classical and the quality of the output random numbers is at the lowest level.
  
 In order to preserve the implementation of the lcg_value() function, the internal implementation is retained, but the implementation as a class is being dropped to prevent users from unintentionally using it. In order to preserve the implementation of the lcg_value() function, the internal implementation is retained, but the implementation as a class is being dropped to prevent users from unintentionally using it.
  
-==== Randomizer lacks array_rand() replacement method ====+==== There is no equivalent of array_rand() ====
  
-array_rand() uses RNG internally, but there is no alternative method in Randomizer.+<php>array_rand()</php> uses RNG internally, but there is no alternative method in ''Randomizer''.
 As per the previous RFC, this was the intent, but upon further investigation, array_rand() is used by many packages and should probably be drop-in replaceable. As per the previous RFC, this was the intent, but upon further investigation, array_rand() is used by many packages and should probably be drop-in replaceable.
  
-However, the name of the method may need to be reconsidered. How about Randomizer::pickArrayKey(array $array, int $num = 1): int|string|array or something similar?+So add a method <php>Randomizer::pickArrayKeys(array $array, int $num): array</php>.
  
-==== "string" means a binary ====+It may look incompatible with <php>array_rand(array $array, int $num = 1): int|string|array</php>, but you can get completely consistent results by doing the following:
  
-In PHP, "string" means a binary. This is often a problem when using multibyte characters. For example, str_shuffle on a Japanese (UTF-8) string will give messed up results.+<code php> 
 +$array = ['foo', 'bar', 'baz'];
  
-Therefore, it may be better to change the alternative method of str_shuffle(), Randomizer::shuffleString(), to Randomizer::shuffleBytes(). This is a more appropriate name.+// Before: 
 +mt_srand(1234, MT_RAND_PHP); 
 +$single = array_rand($array); // (int) 0 
 +$multiple = array_rand($array, 2); // (array) [1, 2]
  
-==== PCG64 is ambiguous ====+// After: 
 +$engine = new Random\Engine\Mt19937(1234, MT_RAND_PHP); 
 +$randomizer = new Random\Randomizer($engine); 
 +$single = $randomizer->pickArrayKeys($array, 1)[0]; // (int) 0 
 + // or: [$single] = $randomizer->pickArrayKeys($array, 1); 
 +$multiple = $randomizer->pickArrayKeys($array, 2); // (array) [1, 2] 
 +</code>
  
-The Random Extension has implemented PCG64 as a new PRNG, but it is officially one variant of PCG64, oneseq-128-xsl-rr-64, and may need to be made more easily understood.+==== "string" means a binary ====
  
-Therefore, change the class name from Random\Engine\PCG64 to Random\Engine\PCGOneseq128XslRr64.+In PHP, "string" means a binary. This is often a problem when using multibyte characters. For example, <php>str_shuffle()</php> on a Japanese (UTF-8) string will give messed up results.
  
-==== Mersenne Twister is ambiguous ====+Therefore, it may be better to change the alternative method of str_shuffle(), ''Randomizer::shuffle**String**()'', to ''Randomizer::shuffle**Bytes**()''. This is a more appropriate name.
  
-When PCG makes the choice to reflect a more detailed implementation in the name, Random\Engine\MersenneTwister is also an issue at the same time.+==== Engine classnames are not precise ====
  
-Random\Engine\MersenneTwister is precisely an implementation of the MT19937 algorithm.+To clearly identify the implemented algorithm, the PCG64 and MersenneTwister engines should be renamed to their canonical upstream name:
  
-MT19937 also has an algorithm called MT19937_64 that generates 64-bit values. If for some reason MT19937_64 were to be implemented, the fact that the name Mersenne Twister is already in use could be problematic.+  * ''Random\Engine\**PCG64**'' → ''Random\Engine\**PcgOneseq128XslRr64**'' 
 +  * ''Random\Engine\**MersenneTwister**'' → ''Random\Engine\**Mt19937**''
  
 ==== PCG is not so famous ==== ==== PCG is not so famous ====
  
 PCG is a very good algorithm, boasting great randomness and performance. PCG is a very good algorithm, boasting great randomness and performance.
- 
 However, I think its name recognition is in some ways inferior to Vigna's RNG, which started with Xorshift. However, I think its name recognition is in some ways inferior to Vigna's RNG, which started with Xorshift.
  
-Therefore, I reimplement Xoshiro256**, which was previously mentioned as a candidate, and create the class Random\Engine\Xoshiro256StarStar. This will avoid the problem of using the old MT19937 since it is not familiar with PCG.+Therefore, I reimplement %%Xoshiro256**%%, which was previously mentioned as a candidate, and create the class ''Random\Engine\Xoshiro256StarStar''. This will avoid the problem of using the old MT19937 since it is not familiar with PCG.
  
 ===== Proposal ===== ===== Proposal =====
Line 68: Line 85:
 For each of these issues, we will create a ballot option and make a decision. For each of these issues, we will create a ballot option and make a decision.
  
-==== CombinedLCG is outdated ====+==== Engine implementations are not final ====
  
-<doodle title="Drop Random\Engine\CombinedLCG" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Make all implemented engines final?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-==== Engine implementations are not final ====+==== Random\SerializableEngine is not useful ====
  
-<doodle title="Engine implementations to final" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Remove the SerializableEngine interface?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-==== Randomizer lacks array_rand() replacement method ====+==== Random\Engine\CombinedLCG is low quality ====
  
-<doodle title="Add Random\Randomizer::pickArrayKey(array $array, int $num = 1): int|string|array" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Remove the CombinedLCG class?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-==== "string" means a binary ====+==== There is no equivalent of array_rand() ====
  
-<doodle title="Rename Random\Randomizer::shuffleString() to Random\Randomizer::shuffleBytes()" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Add the pickArrayKeys() method to the Randomizer?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-==== PCG64 is ambiguous ====+==== "string" means a binary ====
  
-<doodle title="Rename Random\Engine\PCG64 to Random\Engine\PCGOneseq128XslRr64" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Rename Randomizer::shuffleString() to Randomizer::shuffleBytes()?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-==== Mersenne Twister is ambiguous ====+==== Engine classnames are not precise ====
  
-<doodle title="Rename Random\Engine\MersenneTwister to Random\Engine\MT19937" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Rename PCG64 and MersenneTwister?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
Line 112: Line 129:
 ==== PCG is not so famous ==== ==== PCG is not so famous ====
  
-<doodle title="Implement Random\Engine\Xoshiro256StarStar" auth="zeriyoshi" voteType="single" closed="true">+<doodle title="Add the Xoshiro256StarStar engine and class?" auth="zeriyoshi" voteType="single" closed="true">
    * Yes    * Yes
    * No    * No
Line 119: Line 136:
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
  
-The following names have been reserved and will no longer be available (optional):+The following names have been reserved and will no longer be available: 
 + 
 +  * Random\Engine\Mt19937 
 +  * Random\Engine\PcgOneseq128XslRr64 
 +  * Random\Engine\Xoshiro256StarStar 
 + 
 +The following class names will be made available again:
  
-  * Random\Engine\PCGOneseq128XslRr64 +  * Random\Engine\CombinedLCG 
-  * Random\Engine\Xoshiro256StarStar (optional) +  * Random\Engine\MersenneTwister 
-  * Random\Engine\MT19937+  * Random\Engine\PCG64 
 +  * Random\SerializableEngine
  
 ===== Proposed PHP Version(s) ===== ===== Proposed PHP Version(s) =====
Line 145: Line 169:
  
 ===== Patches and Tests ===== ===== Patches and Tests =====
-currently none+  * https://github.com/php/php-src/pull/8094
  
 ===== References ===== ===== References =====
  
   * https://externals.io/message/117939   * https://externals.io/message/117939
rfc/random_extension_improvement.1655740617.txt.gz · Last modified: 2022/06/20 15:56 by zeriyoshi

Page Tools