Re: [PHP-DEV] Which IDE do you recommend for php-src development?
2024年9月15日(日) 19:49 Christoph M. Becker : > On 14.09.2024 at 23:44, Barel wrote: > > > […] One important feature > > would be to easily work with the project running on a docker container > > You may be interested in <https://github.com/php/php-src/pull/15203>. > > Christoph > I would like to move this forward. DevContainers + VSCode is probably the simplest and easiest development environment we can offer right now. I've been switching to the branch of this Pull Request to do development, but if we could merge this, it would save me the trouble of switching and be very helpful. Best regards, Go Kudo
Re: [PHP-DEV] Debug Build Container Image for GitHub Packages
2024年9月15日(日) 0:15 Daniil Gentili : > It seems like the discussion on github has stalled... > Hello, I've been continuing to approach this issue and I'm considering whether we could publish nightly builds of php-src as container images on GitHub Packages. If this becomes a reality, it would allow for quicker reproduction of bugs and setup of development environments. https://github.com/zeriyoshi/php-containers-development/pkgs/container/php-containers-development I feel we could meet the requirements if we could create an image based on this one that's compatible with the php image on DockerHub. Best regards, Go Kudo
Re: [PHP-DEV] Debug Build Container Image for GitHub Packages
2024年9月3日(火) 10:35 Ben Ramsey : > > On Sep 2, 2024, at 19:36, Bob Weinand wrote: > > > > On 3.9.2024 02:18:30, Ben Ramsey wrote: > >>> On Sep 2, 2024, at 18:53, Bob Weinand wrote: > >>> > >>> On 3.9.2024 01:44:21, Ben Ramsey wrote: > >>> > >>>>> On Sep 2, 2024, at 08:11, Go Kudo wrote: > >>>>> > >>>>> Hi Internals. > >>>>> > >>>>> PHP currently does not provide official container images. Given that > DockerHub adequately maintains these and considering the maintenance costs, > we haven't felt the need to change the status quo. > >>>>> > >>>>> However, the official DockerHub images lack debug builds, which can > be somewhat inconvenient when trying to report bugs or reproduce issues. > >>>>> > >>>>> What if we were to provide debug build container images that are > compatible with the official DockerHub images? Fortunately, we already > conduct most of our development on GitHub, which has a container registry > called Packages. > >>>>> > >>>>> This could be achieved simply by creating a single repository under > the php organization on GitHub. What are your thoughts on this? > >>>>> > >>>>> Best Regards. > >>>>> Go Kudo > >>>>> > >>>> Since the folks who do the DockerHub builds already have all the > infrastructure set up to maintain the images, I think it might be easier to > work with them to have them provide debug builds. > >>>> > >>>> Perhaps there’s someone from that team on this list who can speak to > that? > >>>> > >>>> Cheers, > >>>> Ben > >>>> > >>> Hey Ben, > >>> > >>> what I'd _really_ like to see is not debug-builds, but debug symbols. > >>> > >>> Basically, you'd have a docker image "php:8.3" and a docker image > "php:8.3-dbgsym". The former image then just has a gnu_debuglink. The > latter has the actual symbols file included and is based on the former. > >>> > >>> > >>> Thanks, > >>> > >>> Bob > >>> > >> > >> I think the team who manages the Docker builds could also provide > images with debug symbols. Since they’re already equipped for it and have > the experience, why don’t we partner with them to provide these images to > the community? > >> > >> Cheers, > >> Ben > > Yes. Absolutely. > > The problem, however, if you want to provide a build with debug symbols > and one without, the primary value for me would be being able to take a > core dump produced in an image without debug symbols and then simply open > the image with debug symbols and inspect it there. > > To the best of my knowledge however, it isn't trivially possible to > build two docker images with the same docker build invocation. Basically > you'd need an intermediary image, from which you then create both (so, 3 > different docker builds - one base image which has the build and the > symbols but is not uploaded. And two images where the base images files are > copied into.). I believe this would not be compatible with the way > docker-library/php is built currently. > > So, yes, please reach out to them what they are willing to do. > > > > Thanks, > > Bob > > > I opened an issue on their GitHub tracker to gauge their interest: > https://github.com/docker-library/php/issues/1538 > > Cheers, > Ben > > > > Hi. I think it's great that drop-in replacement images are being provided on DockerHub. Thank you for the suggestion! However, I believe it would be beneficial to have image variants as well. For instance, providing images with LLVM MemorySanitizer enabled builds would allow us to easily conduct various tests using our local PHP code. This would also be useful for testing third-party extensions. For safely developing PHP extensions, I primarily use the following build variants: - The build provided by DockerHub - GCC debug build - GCC debug with gcov build - GCC Valgrind build - Clang MSan build - Clang ASan build - Clang UBSan build These images are intended for development purposes, and it might be challenging to host them on DockerHub, which typically hosts images for general use. That said, I already maintain these [1], and I think it could be beneficial to offer them as official PHP images. Of course, I plan to continue maintaining them if we decide to do so. Additionally, I'd like to point out that we could utilize the existing GitHub infrastructure for both the build environment and hosting environment when publishing these images. [1] https://github.com/colopl/pskel Best Regards, Go Kudo
[PHP-DEV] Debug Build Container Image for GitHub Packages
Hi Internals. PHP currently does not provide official container images. Given that DockerHub adequately maintains these and considering the maintenance costs, we haven't felt the need to change the status quo. However, the official DockerHub images lack debug builds, which can be somewhat inconvenient when trying to report bugs or reproduce issues. What if we were to provide debug build container images that are compatible with the official DockerHub images? Fortunately, we already conduct most of our development on GitHub, which has a container registry called Packages. This could be achieved simply by creating a single repository under the php organization on GitHub. What are your thoughts on this? Best Regards. Go Kudo
[PHP-DEV] [Discussion] Add Development Containers definitions
Hello. How about adding a definition of Development Containers to php-src? https://github.com/php/php-src/pull/15203 This makes it easier to build and contribute to the php-src development environment. It also allows you to use GitHub Codespaces. I'd be glad to get your opinion. Best regards, Go Kudo
Re: [PHP-DEV] Re: [Discussion] Add date_test_set_now() function
Hi. > So don't do that. Relying on global mutable state is a bug. That older parts of the stdlib made that mistake doesn't mean we should continue it. (See also the Random extension, which also works to avoid global mutable state.) I generally agree. However, I have some questions. It's true that relying on global state is generally undesirable. I dislike stateful approaches so much that I proposed and implemented ext-random in PHP 8.2. However, the concept of date and time is stateful in the real world we live in. For example, time varies due to various factors such as time zones and daylight saving time. Even now, if you execute `date_default_timezone_set()`, the current time can potentially rewind. https://3v4l.org/FJn4e This behavior completely depends on the internal state of ext-date, but I don't think it should be abolished because changing the time zone to consider during execution is also a realistic scenario. Similarly, I don't think the proposed `date_test_set_now()` will create new flaws. There's also the issue of convenience. It's possible to take an approach similar to ext-random for ext-date, but that would result in a significant increase in the amount of code to write. Additionally, there's the problem that the current `\DateTimeInterface` doesn't cover all the functionality provided by ext-date functions, making a drop-in replacement impossible. (ext-random is completely replaceable) As an aside, I once proposed deprecating the `mt_rand()`, `mt_srand()`, `rand()`, and `srand()` functions. However, it was rejected on the grounds that it was unnecessary and excessive for many workloads. Couldn't the same be said for this case? https://wiki.php.net/rfc/deprecations_php_8_3 I look forward to your reply. Best Regards Go Kudo 2024年7月2日(火) 1:41 Larry Garfield : > On Mon, Jul 1, 2024, at 3:56 PM, Go Kudo wrote: > > > I apologize, the main point of my explanation was off. > > > > To put it simply, I'm suggesting that it might be good to implement > > convenient and commonly used features from Carbon at the ext-date level. > > > > I'm proposing the date_test_set_now() function for the following reasons: > > > > User-land libraries like Carbon / Chronos have a setTestNow method, > > indicating a potential demand for this feature. > > It's impossible to determine whether a value is relative or absolute > > time from either user-land or Extension. > > While this is convenient for maintaining legacy systems, that's not the > > essence of this proposal. > > > > As you pointed out, this is an issue that should ideally be solved in > > user-land. I deeply understand that. > > > > However, in reality, PHP's time-related processing is diverse, and to > > comprehensively handle all of these, it seems necessary to address this > > at the ext-date level. > > > > https://www.php.net/manual/en/ref.datetime.php > > > > For example, you might want to use the date() function to display the > > current time on a whim. However, doing this ruins everything. > > > > Even if other parts of your code use Carbon or comply with PSR-20, > > using the date() function is problematic because the current time it > > references cannot be modified. > > > > `date_test_now(\DateInterval $shiftInterval)` can solve this problem. > > > > Of course, there might be various side effects. However, seeing > > `Carbon::setTestNow()` being used in various places, I think this > > feature might be necessary. > > > > I would appreciate your thoughts on this. > > > > Best Regards, > > Go Kudo > > They are unchanged. > > > For example, you might want to use the date() function to display the > > current time on a whim. > > So don't do that. Relying on global mutable state is a bug. That older > parts of the stdlib made that mistake doesn't mean we should continue it. > (See also the Random extension, which also works to avoid global mutable > state.) > > > Of course, there might be various side effects. > > Exactly. This is not something to just brush aside with a comment. > > The way Carbon does it is wrong, for the same reason: It just sets a > global state. The correct answer is to use PSR-20 and inject a fixed-time > instance of the Clock. > > --Larry Garfield >
[PHP-DEV] Re: [Discussion] Add date_test_set_now() function
2024年7月1日(月) 22:07 Go Kudo : > Hi, Internals. > > I've been absent for a long time due to poor health. I'm finally back. > > I maintain a legacy application written in PHP, and occasionally need to > fake the current time for testing purposes. However, PHP doesn't provide a > standard way to fake the current time, so I've been changing the OS's > current time to do this, which is quite painful. > > This can be avoided by using third-party libraries (such as > Carbon::setTestNow()). However, it's almost impossible to modify all parts > of a legacy application that depend on the current time. > > Another option is to use libfaketime ( > https://github.com/wolfcw/libfaketime), but this is also quite painful to > use. > > Since this was absolutely necessary for my work, I implemented this > functionality as a PHP Extension. (Please ignore the dirty implementation > related to PDO. I'm not planning to propose it this time.) > > https://github.com/colopl/php-colopl_timeshifter > > However, this Extension has some problems. > > The first is that there's no way to determine whether the format passed to > the ext-date parser is relative or absolute time, resulting in a dirty hack > using usleep. The second is that it depends on timelib, so it breaks when > upstream changes related to timelib are made. > > So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)` > function to ext-date? > > This function would treat the current time as shifted by the passed > DateInterval. Since it's implemented on the ext-date side, there's no need > for dirty hacks using usleep. > > I'd like to hear your opinions. Thank you. > > Best Regards, > Go Kudo > I apologize, the main point of my explanation was off. To put it simply, I'm suggesting that it might be good to implement convenient and commonly used features from Carbon at the ext-date level. I'm proposing the date_test_set_now() function for the following reasons: User-land libraries like Carbon / Chronos have a setTestNow method, indicating a potential demand for this feature. It's impossible to determine whether a value is relative or absolute time from either user-land or Extension. While this is convenient for maintaining legacy systems, that's not the essence of this proposal. As you pointed out, this is an issue that should ideally be solved in user-land. I deeply understand that. However, in reality, PHP's time-related processing is diverse, and to comprehensively handle all of these, it seems necessary to address this at the ext-date level. https://www.php.net/manual/en/ref.datetime.php For example, you might want to use the date() function to display the current time on a whim. However, doing this ruins everything. Even if other parts of your code use Carbon or comply with PSR-20, using the date() function is problematic because the current time it references cannot be modified. `date_test_now(\DateInterval $shiftInterval)` can solve this problem. Of course, there might be various side effects. However, seeing `Carbon::setTestNow()` being used in various places, I think this feature might be necessary. I would appreciate your thoughts on this. Best Regards, Go Kudo
[PHP-DEV] [Discussion] Add date_test_set_now() function
Hi, Internals. I've been absent for a long time due to poor health. I'm finally back. I maintain a legacy application written in PHP, and occasionally need to fake the current time for testing purposes. However, PHP doesn't provide a standard way to fake the current time, so I've been changing the OS's current time to do this, which is quite painful. This can be avoided by using third-party libraries (such as Carbon::setTestNow()). However, it's almost impossible to modify all parts of a legacy application that depend on the current time. Another option is to use libfaketime (https://github.com/wolfcw/libfaketime), but this is also quite painful to use. Since this was absolutely necessary for my work, I implemented this functionality as a PHP Extension. (Please ignore the dirty implementation related to PDO. I'm not planning to propose it this time.) https://github.com/colopl/php-colopl_timeshifter However, this Extension has some problems. The first is that there's no way to determine whether the format passed to the ext-date parser is relative or absolute time, resulting in a dirty hack using usleep. The second is that it depends on timelib, so it breaks when upstream changes related to timelib are made. So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)` function to ext-date? This function would treat the current time as shifted by the passed DateInterval. Since it's implemented on the ext-date side, there's no need for dirty hacks using usleep. I'd like to hear your opinions. Thank you. Best Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Vote] PHP 8.3 deprecations
rocess. To avoid this, the sequence must be reseeded again after the fork is executed. If only `srand()` / `mt_srand()` is deprecated, this cannot be avoided. The only way to completely solve this problem is to have no random number states in the global area. For this reason, I think everything should be deprecated. If you need pure random numbers without the need for reproducibility, we already have `random_int()` available. This is the only complete and safe way. Most of all, while `mt_rand()` is deprecated, we can wait for PHP 9 to actually remove it from the core. By that time, migration methods using tools such as Rector will probably have become major. The purpose of the deprecation is to avoid including it in new codebases that will be written in the future. I look forward to your reply. Best Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations
2023年5月30日(火) 4:49 Tim Düsterhus : > Hi > > On 5/29/23 08:44, Go Kudo wrote: > > I realized I was about to add the deprecation of `lcg_value()` and forgot > > to do so, so I added it. > > > > https://wiki.php.net/rfc/deprecations_php_8_3#global_combined_lcg > > > > As usual, my English is of low quality, so I would appreciate it if you > > could point out any problems. > > I think it's too early for that. Because: > > 1. The replacement is only available as of PHP 8.3. Thus there won't be > a single version where "replacement is available" and "the function does > not emit deprecation notices" is both true. It should be deprecated with > PHP 8.4 at the earliest to give folks at least (!) one version to > cleanly migrate existing code without suppressing any errors / notices / > deprecations. > > 2. It's not seedable, thus the implementation can be switched to use a > different engine without affecting existing code. > > 3. It's not as commonly misused as mt_rand() is. Primarily because the > possible use-cases are much more rare. > > Best regards > Tim Düsterhus > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi > It should be deprecated with PHP 8.4 at the earliest to give folks at least Indeed, I agree that `lcg_value()` should be deprecated at least in PHP 8.4. However, `lcg_value()` remains a dangerous function. It still has a weak initial seeding problem (PID, time), not to mention global state. This is extremely dangerous for workloads on containers where PIDs tend to be fixed. Perhaps this should be documented at the time of PHP 8.3 release. Because of the above, I have removed my `lcg_value()` deprecation entry from the RFC. Thanks! Best Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations
2023年5月30日(火) 0:42 Nikita Popov : > On Mon, May 29, 2023, at 08:05, Máté Kocsis wrote: > > Hi Everyone, > > > > Together with multiple authors, we'd like to start the discussion of the > > usual > > deprecation RFC for the subsequent PHP version. You can find the link > below: > > https://wiki.php.net/rfc/deprecations_php_8_3 > > > > Regards: > > Máté Kocsis > > I don't think we should deprecate mt_rand(). > > There are plenty of use-cases that require neither a seedable > (predictable) RNG sequence, nor a cryptographically-secure RNG. For those > use-cases (and especially one-off uses), mt_rand() is perfect, and going > through Randomizer is an entirely unnecessary complication. > > I think I could get on board with deprecating srand/mt_srand to make > rand/mt_rand non-seedable, directing people who need a seedable RNG to use > Randomizer, which is much better suited to that use-case. However, we > should retain rand/mt_rand themselves for non-seeded use-cases. > > With srand/mt_srand removed, we also would not have to produce any > particular sequence, and would be free to switch the internal RNG to > something other than Mt19937. > > The same extends to array_rand(), shuffle() and str_shuffle() -- in fact > the RFC is missing an important voting option, which is "leave them alone", > or rather "convert to some non-seedable non-CSPRNG" if you prefer. > > Regards, > Nikita +1 I too feel that the `Randomizer` is overkill for many applications. However, I believe that there is a suitable alternative to `mt_rand()` / `rand()` : `random_int()` It probably makes the most sense to use it. On the other hand, there is no suitable alternative for `lcg_value()`. The simpler `Randomizer::getFloat()` would be fine, but on the other hand, floating-point random numbers are very hard to handle, so some say that the `Randomizer` class should be used. I would like to hear your opinion on this. Best Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations
2023年5月29日(月) 15:05 Máté Kocsis : > Hi Everyone, > > Together with multiple authors, we'd like to start the discussion of the > usual > deprecation RFC for the subsequent PHP version. You can find the link > below: > https://wiki.php.net/rfc/deprecations_php_8_3 > > Regards: > Máté Kocsis > Hi. I realized I was about to add the deprecation of `lcg_value()` and forgot to do so, so I added it. https://wiki.php.net/rfc/deprecations_php_8_3#global_combined_lcg As usual, my English is of low quality, so I would appreciate it if you could point out any problems. Best regards. Go Kudo
Re: [PHP-DEV] ext-random: add random_float() ?
2022年12月29日(木) 22:25 Tim Düsterhus : > Hi > > On 12/20/22 07:27, Go Kudo wrote: > > Now that my work is done, I was thinking about a proposal for a > sunsetting > > of existing functions for PHP 8.3 based on the features introduced in > > ext-random, and I realized that there is no alternative to `lcg_value()`. > > > > Essentially, this could be completely replaced by > > Random\Randomizer::getFloat(), but there are easier `random_int()` and > > `random_bytes()` functions for ints and strings. > > > > The Randomizer may be overkill and complicated for PHP use cases where > > random number reproducibility is not required in many cases. > > > > So, why not add a `random_float(): float` function? This function, like > the > > others, uses CSPRNG and returns a value between 0.0 and 1.0. This > behavior > > is `Closed` `Closed`. > > > > Opinions are welcome. > > I'm not convinced that a random_float() function is a good addition. > Especially not with the proposed behavior: > > 1. Using the (0, 1, ClosedClosed) behavior does neither match > Randomizer::nextFloat(), nor Randomizer::getFloat(). > > 2. I consider the ClosedOpen behavior to be the "correct" default, > because the interval can then be cleanly split into equally sized > subintervals. > > > > But even when random_float() would work like Randomizer::nextFloat() > (i.e. (0, 1, ClosedOpen)), I would not consider this a good addition: > > Users would likely attempt to scale the returned value to arbitrary > ranges using the `($min + random_float() * ($max - $min))` construction, > instead of using Randomizer::getFloat(). > > As per Drawing Random Floating-Point Numbers from an Interval. Frédéric > Goualard, ACM Trans. Model. Comput. Simul., 32:3, 2022 this construction > is unsafe, because is is both biased and also may return values outside > the expected interval. > > > > So random_float() would rather need to behave like > Randomizer::getFloat() instead of Randomizer::nextFloat() and then it > would not be much simpler than using the Randomizer class directly. I > further expect that needing to generate random floats is much rarer than > needing to generate random integers or random bytes (for tokens), thus > having convenience functions for those two, but not floats is acceptable > and keeps the API surface simple, making it easier to document all the > gotchas. Users can add convenience wrappers themselves. > > Best regards > Tim Düsterhus > Hi Tim Thank you for the clear explanation. I generally understood the intent. And once again I recognized that floating point random numbers are difficult to handle. I agree that the addition of the random_float() function is inappropriate. However, with the goal of deprecating it in PHP 8.3 and deprecating RNGs with virtual machine dependent state in PHP 9.x, I think we need to provide a useful replacement for the lcg_value() function. Do you have any good ideas on this? Most likely, since the random numbers generated by lcg_value() are of low quality and do not appear to be used very often, one idea would be to simply remove it. However, I believe that the absence of an alternative function is likely to generate negative opinions about its elimination. Regards, Go Kudo
Re: [PHP-DEV] ext-random: add random_float() ?
2022年12月20日(火) 18:14 Go Kudo :
> 2022年12月20日(火) 17:15 Hans Henrik Bergan :
>
>> btw while we're on the topic, does anyone know if this function gives
>> biased results/is-safe or not? i honestly don't know:
>> function random_float(float $min, float $max): float
>> {
>> if ($min > $max) throw new \InvalidArgumentException("min must be
>> less than max");
>> if ($min === $max) return $min;
>> return $min + random_int(0, PHP_INT_MAX) / PHP_INT_MAX * ($max -
>> $min);
>> }
>>
>>
>> On Tue, 20 Dec 2022 at 09:06, Hans Henrik Bergan
>> wrote:
>> >
>> > >returns a value between 0.0 and 1.0.
>> >
>> > wouldn't it be better to follow random_int(int $min, int $max) design?
>> eg
>> > random_float(float $min, float $max): float
>> >
>> > On Tue, 20 Dec 2022 at 07:27, Go Kudo wrote:
>> > >
>> > > Hi Internals.
>> > >
>> > > Congratulations on the release of PHP 8.2.
>> > > I just recently upgraded production PHP from 7.4 to 8.1 :)
>> > >
>> > > Now that my work is done, I was thinking about a proposal for a
>> sunsetting
>> > > of existing functions for PHP 8.3 based on the features introduced in
>> > > ext-random, and I realized that there is no alternative to
>> `lcg_value()`.
>> > >
>> > > Essentially, this could be completely replaced by
>> > > Random\Randomizer::getFloat(), but there are easier `random_int()` and
>> > > `random_bytes()` functions for ints and strings.
>> > >
>> > > The Randomizer may be overkill and complicated for PHP use cases where
>> > > random number reproducibility is not required in many cases.
>> > >
>> > > So, why not add a `random_float(): float` function? This function,
>> like the
>> > > others, uses CSPRNG and returns a value between 0.0 and 1.0. This
>> behavior
>> > > is `Closed` `Closed`.
>> > >
>> > > Opinions are welcome.
>> > >
>> > > Regards,
>> > > Go Kudo
>>
>
> Hi.
>
> The dangers of generating floating-point random numbers from PHP integer
> values are explained in detail in Tim and Joshua's RFC.
>
> https://wiki.php.net/rfc/randomizer_additions
>
> I missed it at first too, and was negative about adding
> `Randomizer::getFloat()` ...
> In general, I think this problem is less noticeable and the absence of the
> random_float() function increases the likelihood of incorrect
> implementations in userland.
>
> Regards,
> Go Kudo
>
In the PHP-8.3 current development branch, you can already polyfill as
follows However, it is complicated.
```php
getFloat($min, $max,
$intervalBoundary);
}
```
Re: [PHP-DEV] ext-random: add random_float() ?
2022年12月20日(火) 17:15 Hans Henrik Bergan :
> btw while we're on the topic, does anyone know if this function gives
> biased results/is-safe or not? i honestly don't know:
> function random_float(float $min, float $max): float
> {
> if ($min > $max) throw new \InvalidArgumentException("min must be
> less than max");
> if ($min === $max) return $min;
> return $min + random_int(0, PHP_INT_MAX) / PHP_INT_MAX * ($max - $min);
> }
>
>
> On Tue, 20 Dec 2022 at 09:06, Hans Henrik Bergan
> wrote:
> >
> > >returns a value between 0.0 and 1.0.
> >
> > wouldn't it be better to follow random_int(int $min, int $max) design? eg
> > random_float(float $min, float $max): float
> >
> > On Tue, 20 Dec 2022 at 07:27, Go Kudo wrote:
> > >
> > > Hi Internals.
> > >
> > > Congratulations on the release of PHP 8.2.
> > > I just recently upgraded production PHP from 7.4 to 8.1 :)
> > >
> > > Now that my work is done, I was thinking about a proposal for a
> sunsetting
> > > of existing functions for PHP 8.3 based on the features introduced in
> > > ext-random, and I realized that there is no alternative to
> `lcg_value()`.
> > >
> > > Essentially, this could be completely replaced by
> > > Random\Randomizer::getFloat(), but there are easier `random_int()` and
> > > `random_bytes()` functions for ints and strings.
> > >
> > > The Randomizer may be overkill and complicated for PHP use cases where
> > > random number reproducibility is not required in many cases.
> > >
> > > So, why not add a `random_float(): float` function? This function,
> like the
> > > others, uses CSPRNG and returns a value between 0.0 and 1.0. This
> behavior
> > > is `Closed` `Closed`.
> > >
> > > Opinions are welcome.
> > >
> > > Regards,
> > > Go Kudo
>
Hi.
The dangers of generating floating-point random numbers from PHP integer
values are explained in detail in Tim and Joshua's RFC.
https://wiki.php.net/rfc/randomizer_additions
I missed it at first too, and was negative about adding
`Randomizer::getFloat()` ...
In general, I think this problem is less noticeable and the absence of the
random_float() function increases the likelihood of incorrect
implementations in userland.
Regards,
Go Kudo
[PHP-DEV] ext-random: add random_float() ?
Hi Internals. Congratulations on the release of PHP 8.2. I just recently upgraded production PHP from 7.4 to 8.1 :) Now that my work is done, I was thinking about a proposal for a sunsetting of existing functions for PHP 8.3 based on the features introduced in ext-random, and I realized that there is no alternative to `lcg_value()`. Essentially, this could be completely replaced by Random\Randomizer::getFloat(), but there are easier `random_int()` and `random_bytes()` functions for ints and strings. The Randomizer may be overkill and complicated for PHP use cases where random number reproducibility is not required in many cases. So, why not add a `random_float(): float` function? This function, like the others, uses CSPRNG and returns a value between 0.0 and 1.0. This behavior is `Closed` `Closed`. Opinions are welcome. Regards, Go Kudo
Re: [PHP-DEV] RFC [Discussion]: Randomizer Additions
2022年11月6日(日) 0:34 Go Kudo : > 2022年10月14日(金) 4:38 Joshua Rüsweg via internals : > >> Hi >> >> Tim Düsterhus and I have created an RFC to add new methods that solve >> commonly encountered use cases to \Random\Randomizer. Specifically >> creating a random string consisting of specific bytes and generating >> random floating point values. >> >> You can find the RFC at: >> >> https://wiki.php.net/rfc/randomizer_additions >> >> Proof of concept implementation is in: >> >> * https://github.com/php/php-src/pull/9664 >> * https://github.com/php/php-src/pull/9679 >> >> >> >> Some open questions to start the discussion: >> >> * Are you missing other commonly useful operations that are also useful >> to have in core? >> * Do you agree with the method names? Within the PR we received comments >> that "alphabet" might not be an appropriate term. >> * Shall an option be added to getFloat() that changes the logic to >> select from [$min, $max] (i.e. allowing the maximum to be returned)? And >> how should that look like? Boolean parameter? Enum? >> >> >> >> We're looking forward to your feedback. >> >> Cheers >> >> Joshua Rüsweg >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: https://www.php.net/unsub.php >> >> > (I made a mistake in replying to your message, so I will reply again here.) > > Hi > > I have read the RFC. I think the content is generally good and adequately > compensates for the areas that could not be completed. > > I am skeptical only about getFloat(). The use cases are limited and seem > somewhat excessive. Do you have examples of how this is supported in other > languages? > > Regards > Go Kudo > Sorry, I did not read the updated RFC. The latitude and longitude samples were very clear and even my limited understanding of floating point numbers helped me understand why they were necessary. I am so far in favor of this RFC and positive about both features. I also agree that as a member of the maintainer I will maintain it properly. Thank you for your very coherent writing and implementation. Best Regards Go Kudo
Re: [PHP-DEV] RFC [Discussion]: Randomizer Additions
2022年10月14日(金) 4:38 Joshua Rüsweg via internals : > Hi > > Tim Düsterhus and I have created an RFC to add new methods that solve > commonly encountered use cases to \Random\Randomizer. Specifically > creating a random string consisting of specific bytes and generating > random floating point values. > > You can find the RFC at: > > https://wiki.php.net/rfc/randomizer_additions > > Proof of concept implementation is in: > > * https://github.com/php/php-src/pull/9664 > * https://github.com/php/php-src/pull/9679 > > > > Some open questions to start the discussion: > > * Are you missing other commonly useful operations that are also useful > to have in core? > * Do you agree with the method names? Within the PR we received comments > that "alphabet" might not be an appropriate term. > * Shall an option be added to getFloat() that changes the logic to > select from [$min, $max] (i.e. allowing the maximum to be returned)? And > how should that look like? Boolean parameter? Enum? > > > > We're looking forward to your feedback. > > Cheers > > Joshua Rüsweg > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > (I made a mistake in replying to your message, so I will reply again here.) Hi I have read the RFC. I think the content is generally good and adequately compensates for the areas that could not be completed. I am skeptical only about getFloat(). The use cases are limited and seem somewhat excessive. Do you have examples of how this is supported in other languages? Regards Go Kudo
Re: [PHP-DEV] RFC [Discussion]: Randomizer Additions
2022年10月14日(金) 13:33 Alexandru Pătrănescu : > On Thu, Oct 13, 2022, 22:37 Joshua Rüsweg via internals < > internals@lists.php.net> wrote: > > > Hi > > > > Tim Düsterhus and I have created an RFC to add new methods that solve > > commonly encountered use cases to \Random\Randomizer. Specifically > > creating a random string consisting of specific bytes and generating > > random floating point values. > > > > You can find the RFC at: > > > > https://wiki.php.net/rfc/randomizer_additions > > > > Proof of concept implementation is in: > > > > * https://github.com/php/php-src/pull/9664 > > * https://github.com/php/php-src/pull/9679 > > > > > > > > Some open questions to start the discussion: > > > > * Are you missing other commonly useful operations that are also useful > > to have in core? > > > > For completeness, it would be good to have nextBool() as well. > > * Do you agree with the method names? Within the PR we received comments > > that "alphabet" might not be an appropriate term. > > > > Yes, alphabet is fine by me but I can suggest also: character set, > character list, character dictionary, byte set, byte list, byte dictionary > or some shortcuts of it, like charset. > > * Shall an option be added to getFloat() that changes the logic to > > select from [$min, $max] (i.e. allowing the maximum to be returned)? And > > how should that look like? Boolean parameter? Enum? > > > > No, IMO. Mathematically it doesn't really make sense and talking about > floats, it will also be a very corner case not reached in tests that might > happen in production rarely and break things. > > > > > > > > We're looking forward to your feedback. > > > > > > Cheers > > > > Joshua Rüsweg > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: https://www.php.net/unsub.php > > > > > > > > I am having another small issue. > As the Randomizer class is final, I guess this will not be perfectly > polyfillable in userland. > So... , if accepted, would it be completely wrong to have these new methods > in PHP 8.2? What can it break? > > As the Randomizer class is final, I guess this will not be perfectly polyfillable in userland. What case do you envision for this? Probably, most cases can be polyfilled by code: https://3v4l.org/JludD And it seems that someone has already done such an implementation (which is very nice and I would like to express my utmost appreciation). https://github.com/arokettu/php-random-polyfill Regards Go Kudo
Re: [PHP-DEV] Pre RFC - Additions to the randomizer
2022年10月6日(木) 6:44 Joshua Rüsweg via internals : > Hi > > I would like to introduce a new method for the new Randomizer class [1]. > I would like to have a function that generates a random string for me > based on a given alphabet. This function is useful as a building block, > as many use cases require generating random strings with a specified > list of characters (e.g. random passwords, voucher codes, numeric > strings larger than integers) and implementing this in userland requires > multiple lines of code for what effectively is a very simple operation. > Furthermore the obvious implementation based on ->getInt() is > inefficient, as it requires at least one call to the engine per > character, whereas a 64 Bit engine could generate randomness for 8 > characters at once. > > I have opened a Pull-Request on PHP to demonstrate the implementation > [2] but I am unsure about the method name and decided for > `getBytesFromAlphabet()` but I am open for better suggestions. > > During code review Go Kudo requested that this goes through an RFC and > Tim Düsterhus requested to handle multiple new methods in bulk. Tim > implemented a new method, which returns a random float value [3], since > that's non-trivial to do in userland and an equally useful building > block like my introduced method. > > Since the class into which this function is built is final anyway, the > methods can be built in with full backwards compatibility. > > I look forward to feedback from you! > > [1] https://wiki.php.net/rfc/random_extension_improvement > [2] https://github.com/php/php-src/pull/9664 > [3] https://github.com/php/php-src/pull/9679 > > PS: Since this is my first contribution, someone needs to give me karma > to open an RFC. My account name is `josh`. > > Cheers > > Joshua Rüsweg > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi I have read the RFC. I think the content is generally good and adequately compensates for the areas that could not be completed. I am skeptical only about getFloat(). The use cases are limited and seem somewhat excessive. Do you have examples of how this is supported in other languages? Regards Go Kudo P.S. It's been too long and I had forgotten everything about how to use ML. Apologies for replying directly to you.
[PHP-DEV] ksort breaking change
Hi Internals. In the actively supported version of PHP, `ksort()` has been modified to include BC Break. https://github.com/php/php-src/issues/9296 This may seem like an appropriate bug fix, but it is a clear BC Break. I think this change should only be introduced in PHP 8.2 and later. Fortunately, there is not yet a release in each version that incorporates this change. I think it is possible to revert now. What do you think? Best regards, Go Kudo
Re: [PHP-DEV] xoshiro** edge case (all zeros)
2022年8月4日(木) 17:10 Anton Smirnov :
> Hi!
>
> Randomness again. Sorry if I just missed some relevant discussion
>
> xoshiro** has a known edge case: all-zero seed
>
>
> $engine = new \Random\Engine\Xoshiro256StarStar(str_repeat("\0", 32));
>
> while (true) {
> echo hex2bin($engine->generate()), PHP_EOL; //
> }
>
> It should be documented and/or handled
>
> It's only for a string seed, int seed is not affected
>
> --
> Anton
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
Hi.
Thanks for the report! This is dangerous behavior and we will attempt to
fix it for 8.2beta3.
Best regards
Go Kudo
Re: [PHP-DEV] What do you think CSPRNG in PHP
2022年7月28日(木) 1:47 Tim Düsterhus : > Hi > > On 7/16/22 23:33, Tim Düsterhus wrote: > > Personally I likely wouldn't have merged the PR in question for the same > > reasons. But at least in that case glibc is at fault :-) > > For those following along: > > It turns out the glibc "userland" implementation of arc4random() was > questionable and was simplified to be a relatively simple wrapper around > getrandom(): > > https://github.com/php/php-src/pull/8984#issuecomment-1195986646 > > and > > https://sourceware.org/pipermail/libc-alpha/2022-July/140939.html > > Best regards > Tim Düsterhus > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi Thank you. After considering various points of view, I realized that my proposal is very dangerous. The language side should not be working on something that will cause confusion even at the libc layer. Also, the newly discussed vDSO implementation of getrandom (which I see no safe way to do at the moment) seems like a better option that would benefit all Linux distributions. Perhaps waiting for this is the better option than anything else. Thank you! Regards, Go Kudo
Re: [PHP-DEV] What do you think CSPRNG in PHP
2022年7月25日(月) 20:32 Jakub Zelenka : > > > On Mon, Jul 25, 2022 at 12:14 PM Go Kudo wrote: > >> 2022年7月17日(日) 6:33 Tim Düsterhus : >> >> > Hi >> > >> > On 7/15/22 17:54, Go Kudo wrote: >> > > However, there are several challenges to this. >> > > >> > > - Increased maintenance costs >> > > - Requires optimization for CPU architecture >> > > - Requires familiarity with CSPRNG >> > > >> > > PHP already bundles xxHash and appears ready to make this happen. >> > > >> > > Also, an appropriate CSPRNG implementation may be able to resolve the >> > > current complex macro branching. >> > > >> > > What do you think about this? >> > >> > This would be a strong no from my side. There's all types of failure >> > modes that decrease the security of the CSPRNG (i.e. making it insecure) >> > and we really don't want to be the ones to blame if something goes >> > wrong. And historically many non-kernel CSPRNGs later proved to be >> > insecure in specific situations. >> > >> > I also would assume that for a typical PHP application both of the >> > following is true: >> > - The majority of the requests don't need any randomness. >> > - The majority of the requests that *need* randomness don't need any >> > significant amount of randomness. >> > - The majority of the requests that need significant amounts of >> > randomness are fine with a regular PRNG (e.g. Xoshiro or Pcg). >> > - The cost of a few getrandom() syscalls is not really measurable >> > compared to the time spent waiting for the database, file IO or template >> > rendering. >> > >> > Attempting to optimize the speed of the CSPRNG is premature >> > optimization. That also the reason why I suggested to use the 'Secure' >> > engine by default in the Randomizer: It's a safe default choice for the >> > vast majority of users. >> > >> > Personally I likely wouldn't have merged the PR in question for the same >> > reasons. But at least in that case glibc is at fault :-) >> > >> > Best regards >> > Tim Düsterhus >> > >> >> Hi Tim. >> >> You are right. Implementing a CSPRNG on your own obviously increases >> maintenance costs and security risks. >> >> However, I still think the overhead of the getrandom syscall in a Linux >> environment is significant and should be considered. >> > > There is already a good CSPRNG available in OpenSSL which we expose > with openssl_random_pseudo_bytes (except on Windows which is historical and > should change) so for those that are impacted by the syscall overhead, this > might be the best option considering that most users are using at least > OpenSSL version 1.1.1 where the new CSPRNG is available. > > Regards > > Jakub > Hi (Sorry, I sent you directly) Indeed, But ext-openssl is not always available. To use it in a ext-session, etc., it must be bundled reliably. Best Regards Go Kudo
Re: [PHP-DEV] What do you think CSPRNG in PHP
2022年7月17日(日) 6:33 Tim Düsterhus :
> Hi
>
> On 7/15/22 17:54, Go Kudo wrote:
> > However, there are several challenges to this.
> >
> > - Increased maintenance costs
> > - Requires optimization for CPU architecture
> > - Requires familiarity with CSPRNG
> >
> > PHP already bundles xxHash and appears ready to make this happen.
> >
> > Also, an appropriate CSPRNG implementation may be able to resolve the
> > current complex macro branching.
> >
> > What do you think about this?
>
> This would be a strong no from my side. There's all types of failure
> modes that decrease the security of the CSPRNG (i.e. making it insecure)
> and we really don't want to be the ones to blame if something goes
> wrong. And historically many non-kernel CSPRNGs later proved to be
> insecure in specific situations.
>
> I also would assume that for a typical PHP application both of the
> following is true:
> - The majority of the requests don't need any randomness.
> - The majority of the requests that *need* randomness don't need any
> significant amount of randomness.
> - The majority of the requests that need significant amounts of
> randomness are fine with a regular PRNG (e.g. Xoshiro or Pcg).
> - The cost of a few getrandom() syscalls is not really measurable
> compared to the time spent waiting for the database, file IO or template
> rendering.
>
> Attempting to optimize the speed of the CSPRNG is premature
> optimization. That also the reason why I suggested to use the 'Secure'
> engine by default in the Randomizer: It's a safe default choice for the
> vast majority of users.
>
> Personally I likely wouldn't have merged the PR in question for the same
> reasons. But at least in that case glibc is at fault :-)
>
> Best regards
> Tim Düsterhus
>
Hi Tim.
You are right. Implementing a CSPRNG on your own obviously increases
maintenance costs and security risks.
However, I still think the overhead of the getrandom syscall in a Linux
environment is significant and should be considered.
I would suggest deprecating mt_srand()/srand() and using php_random_bytes()
in sessions etc. for PHP 8.3 for better security.
However, as Nikita mentioned before, the overhead of the getrandom syscall
in a Linux environment is significant and this proposal may result in
performance degradation.
https://github.com/php/php-src/commit/53ee3f7f897f7ee33a4c45210014648043386e13
Therefore, I have created a PoC to buffer php_random_bytes. This has
resulted in a significant performance improvement.
https://github.com/zeriyoshi/php-src/tree/random_buf
```
# perf result of current implementation:
Samples: 120 of event 'cpu-clock:pppH', Event count (approx.): 3000
Overhead Command Shared Object Symbol
32.50% php [kernel.kallsyms] [k] preempt_count_sub
30.00% php libc.so.6 [.] syscall
18.33% php [kernel.kallsyms] [k] do_el0_svc
2.50% php php[.] execute_ex
1.67% php [kernel.kallsyms] [k] __this_cpu_preempt_check
1.67% php [kernel.kallsyms] [k] __uaccess_mask_ptr
1.67% php [kernel.kallsyms] [k] _raw_spin_unlock_irqrestore
0.83% php [kernel.kallsyms] [k] __arm64_sys_getrandom
0.83% php [kernel.kallsyms] [k] __kern_my_cpu_offset
0.83% php [kernel.kallsyms] [k] __mod_lruvec_state
0.83% php [kernel.kallsyms] [k] __mod_memcg_state
0.83% php [kernel.kallsyms] [k] __next_zones_zonelist
0.83% php [kernel.kallsyms] [k] arch_local_irq_restore
0.83% php [kernel.kallsyms] [k] arch_local_irq_save
0.83% php [kernel.kallsyms] [k] check_stack_object
0.83% php ld-linux-aarch64.so.1 [.] 0xa260
0.83% php php[.] php_random_bytes
0.83% php php[.] syscall@plt
0.83% php php[.] zend_hash_add
0.83% php php[.]
zend_hash_graceful_reverse_destroy
0.83% php php[.]
zend_string_init_interned_permanent
$ time ./sapi/cli/php -r 'for ($i = 0; $i < 65535; $i++){ random_bytes(4);
}'
real 0m0.069s
user 0m0.025s
sys 0m0.044s
# PoC result:
Samples: 19 of event 'cpu-clock:pppH', Event count (approx.): 475
Overhead Command Shared Object Symbol
31.58% php [kernel.kallsyms] [k] __flush_cache_user_range
15.79% php php[.] _efree
10.53% php php[.] zend_register_functions
5.26% php [kernel.kallsyms] [k] __rcu_read_unlock
5.26% php [kernel.kallsyms] [k] page_add_file_rmap
5.26% php [kernel.kallsyms] [k] pfn_valid
5.26% php [kernel.kallsyms] [k] pre
[PHP-DEV] Random Extension 5.x needs follow-up
Hi Internals. The Random Extension 5.x and Random Extension Improvement RFCs have been passed and recently merged into the master branch. However, the proposal is still problematic and implementation fixes are needed. Currently, the mt_rand() function is overloading its arguments, and I have designed a Randomizer::getInt() signature based on this. However, argument overloading has several problems, including reflection, and is now not recommended. I wish I had been aware of it during the RFC proposal, but that did not happen. To solve the problem, I thought of separating the argumentless `Randomizer::getInt()` as `Randomizer::nextInt()`. `Randomizer::getInt()` must require `int $min, int $max` arguments. I have received support for this proposal from several people, but there will be a discrepancy between the RFC and the implementation. I believe this should be corrected to avoid future BC Break, what do you think about this? Discussions: - https://externals.io/message/118163#118269 - https://github.com/php/php-src/pull/8094/files#r919693108 (just scroll a little please) Fix PR: https://github.com/php/php-src/pull/9057 Best Regards Go Kudo
Re: [PHP-DEV] [VOTE] Random Extension Improvement
2022年7月3日(日) 12:00 Go Kudo : > Hello internals. > > Voting began on 2022-07-02 03:00:00 (UTC) and will end on 2022-07-16 > 03:00:00 (UTC). > > https://wiki.php.net/rfc/random_extension_improvement > > Best Regards, > Go Kudo > Hi Voting was closed and all proposals were accepted. https://wiki.php.net/rfc/random_extension_improvement https://github.com/php/php-src/pull/8094 However, as pointed out in the PR, there are still areas that need to be improved and corrected. First of all, overloading of `getInt()` should be stopped. This is due to the fact that the signature of the `mt_rand()` function is still used, but it was pointed out that no new implementation should be made. https://github.com/php/php-src/pull/8094/files#r919693108 Second, there is an issue regarding the handling of header files for compatibility. This was left in because it was considered necessary in previous discussions, but it has been suggested that it is unnecessary. I too would like to remove it if possible. https://github.com/php/php-src/pull/8094/files#r917308676 However, if you try to make this change by creating an RFC as usual, you are already out of time. I am really confused and don't really know what to do about this. Can anyone give me some good ideas? Best Regards, Go Kudo
[PHP-DEV] Re: What do you think CSPRNG in PHP
2022年7月16日(土) 0:54 Go Kudo : > Hi Internals. > > Random Extension is accepted and being implemented in PHP 8.2. Many thanks > for the review. > > The changes to enable arc4random in glibc were recently merged. > > https://github.com/php/php-src/pull/8984 > > This has the effect of reducing the number of getrandom system calls > issued on Linux, which is effective in improving performance. > > However, this will only work in environments that use GNU libc, and will > not work on Linuxes that use other libc (e.g. Alpine Linux that uses musl). > > As we discussed a bit above in PR (which is inherently a bad thing, > because it's not a good thing), the following is an example of a "good" PR > campaign Apologies), if we could implement CSPRNG on PHP, for example, it > would improve performance on all platforms. > > However, there are several challenges to this. > > - Increased maintenance costs > - Requires optimization for CPU architecture > - Requires familiarity with CSPRNG > > PHP already bundles xxHash and appears ready to make this happen. > > Also, an appropriate CSPRNG implementation may be able to resolve the > current complex macro branching. > > What do you think about this? > > Regards > Go Kudo > xxHash has nothing to do with it. Forget it.
Re: [PHP-DEV] [VOTE] Random Extension Improvement
2022年7月14日(木) 16:14 Claude Pache :
>
>
> > Le 14 juil. 2022 à 06:32, Guilliam Xavier a
> écrit :
> >
> > On Thursday, July 14, 2022, Go Kudo wrote:
> >
> >> 2022年7月13日(水) 1:10 Tim Düsterhus :
> >>
> >>> Hi
> >>>
> >>> On 7/12/22 18:04, Tim Düsterhus wrote:
> >>>> I also think that both '$string' and '$binary' are appropriate
> parameter
> >>>> names in this case, so particular preference from my side.
> >>>
> >>> Sorry for the follow-up, there's two mistakes in that sentence. It
> >>> should read:
> >>>
> >>> I also think that both '$string' and **'$bytes'** are appropriate
> >>> parameter names in this case, so **no** particular preference from my
> >>> side.
> >>>
> >>> Best regards
> >>> Tim Düsterhus
> >>>
> >>
> >> Hi
> >>
> >> I agree with you. I will change the parameter name from `$string` to
> >> `$bytes` as I don't see any problem.
> >>
> >> I will try to explain the changes more rigorously in future proposals.
> >> Thank you.
> >>
> >> Regards,
> >> Go Kudo
> >>
> >
> > Hi,
> >
> > I was waiting for more opinions but... so here's mine:
> >
> > I would prefer to keep "$string", as [that's how I read the RFCs, and]
> when
> > calling e.g. shuffleBytes('foobar') I don't feel like I'm passing "bytes"
> > (or "a binary") but a string (to be shuffled byte-wise rather than
> > character-wise or codepoint-wise, but that's from the function, not the
> > argument)...
> > Granted, not compelling, and probably won't matter in practice, but hey
> ;)
> >
> > Regards
> >
> > PS: sent from mobile
> >
> >
> > --
> > Guilliam Xavier
>
> I agree with Guilliam: this function is about «shuffling the bytes of the
> given string» (as opposed to, say, «array of ints»)
>
> As precedent, there are `bin2hex($string)` and `md5($string)`, which are
> unambiguously working with bytes from data given in the form of string,
> where «string» is a PHP type, which can hold data that is not necessarily
> UTF-8-encoded or Shift-JIS-encoded text.
>
> —Claude
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
Thanks for the input. This is certainly something to consider.
For example, what about the `$binaryString` argument name? I think it is
clearer. I have adopted this as the name of the internal API:
`php_binary_string_shuffle()`.
As stated in the GitHub PR, this implementation probably requires
additional changes. After voting on the current RFC is complete, we will
again send an email to the ML.
Regards,
Go Kudo
[PHP-DEV] What do you think CSPRNG in PHP
Hi Internals. Random Extension is accepted and being implemented in PHP 8.2. Many thanks for the review. The changes to enable arc4random in glibc were recently merged. https://github.com/php/php-src/pull/8984 This has the effect of reducing the number of getrandom system calls issued on Linux, which is effective in improving performance. However, this will only work in environments that use GNU libc, and will not work on Linuxes that use other libc (e.g. Alpine Linux that uses musl). As we discussed a bit above in PR (which is inherently a bad thing, because it's not a good thing), the following is an example of a "good" PR campaign Apologies), if we could implement CSPRNG on PHP, for example, it would improve performance on all platforms. However, there are several challenges to this. - Increased maintenance costs - Requires optimization for CPU architecture - Requires familiarity with CSPRNG PHP already bundles xxHash and appears ready to make this happen. Also, an appropriate CSPRNG implementation may be able to resolve the current complex macro branching. What do you think about this? Regards Go Kudo
Re: [PHP-DEV] Re: [VOTE] Random Extension Improvement
2022年7月13日(水) 1:10 Tim Düsterhus : > Hi > > On 7/12/22 18:04, Tim Düsterhus wrote: > > I also think that both '$string' and '$binary' are appropriate parameter > > names in this case, so particular preference from my side. > > Sorry for the follow-up, there's two mistakes in that sentence. It > should read: > > I also think that both '$string' and **'$bytes'** are appropriate > parameter names in this case, so **no** particular preference from my side. > > Best regards > Tim Düsterhus > Hi I agree with you. I will change the parameter name from `$string` to `$bytes` as I don't see any problem. I will try to explain the changes more rigorously in future proposals. Thank you. Regards, Go Kudo
[PHP-DEV] Re: [VOTE] Random Extension Improvement
2022年7月3日(日) 12:00 Go Kudo : > Hello internals. > > Voting began on 2022-07-02 03:00:00 (UTC) and will end on 2022-07-16 > 03:00:00 (UTC). > > https://wiki.php.net/rfc/random_extension_improvement > > Best Regards, > Go Kudo > Hi Internals. Currently, the renaming of Randomizer::shuffleString() to Randomizer::shuffleBytes() seems acceptable. I forgot to note the change regarding arguments when I submitted this RFC. With this change, the argument was supposed to be changed from `string $string` to `string $bytes`. - https://github.com/php/php-src/pull/8094#discussion_r916630626 Is this change acceptable? Or should I keep `string $string`? Regards, Go Kudo
Re: [PHP-DEV] [VOTE] Random Extension Improvement
2022年7月8日(金) 1:44 Tim Düsterhus : > Hi > > On 7/7/22 17:52, Go Kudo wrote: > > Implementation is now proceeding. > > It includes fixes to some of the issues that were pointed out previously. > > > > https://github.com/php/php-src/pull/8094 > > > > `Randomizer::arrayPickKeys()` is currently not implemented for now, since > > it is most likely to be rejected. > > (Of course, we are ready to revert to the other items if they are also > > rejected.) > > > > I want to see if it looks good to you. > > I've reviewed the current implementation of the engines with a specific > focus on the tests. > > 1. I have verified the Xoshiro256** tests against a pure PHP > implementation (which I verified against the C reference implementation > [1]). > > 2. I have verified the Pcg64OneseqXslRr64 pcgoneseq128xslrr64_value.phpt > test against the reference C implementation [2] with the attached > pcg-verification.c. > > 3. I have verified the Mt19937 mt_value.phpt test against the C++ > standard library implementation [3] with the attached mt-verification.cc. > > The three engines do what they are supposed to do. I'm not qualified to > review with regard to PHP Internals best practices. It would be good if > someone with the necessary expertise could proceed with that part of the > review. > > Best regards > Tim Düsterhus > > [1] https://prng.di.unimi.it/xoshiro256starstar.c > [2] https://www.pcg-random.org/download.html#c-implementation > [3] > https://en.cppreference.com/w/cpp/numeric/random/mersenne_twister_engine Hi Tim. Sorry for the late reply. Thanks for the verification. I changed the serialization format to hex string as you pointed out earlier. You should now be able to validate your PHP implementation. Regards, Go Kudo
Re: [PHP-DEV] [VOTE] Random Extension Improvement
2022年7月3日(日) 12:00 Go Kudo : > Hello internals. > > Voting began on 2022-07-02 03:00:00 (UTC) and will end on 2022-07-16 > 03:00:00 (UTC). > > https://wiki.php.net/rfc/random_extension_improvement > > Best Regards, > Go Kudo > Hi. Implementation is now proceeding. It includes fixes to some of the issues that were pointed out previously. https://github.com/php/php-src/pull/8094 `Randomizer::arrayPickKeys()` is currently not implemented for now, since it is most likely to be rejected. (Of course, we are ready to revert to the other items if they are also rejected.) I want to see if it looks good to you. Regards, Go Kudo
[PHP-DEV] Re: [VOTE] Random Extension Improvement
2022年7月3日(日) 12:00 Go Kudo : > Hello internals. > > Voting began on 2022-07-02 03:00:00 (UTC) and will end on 2022-07-16 > 03:00:00 (UTC). > > https://wiki.php.net/rfc/random_extension_improvement > > Best Regards, > Go Kudo > Hi. Once again, I apologize for the delay in starting the voting due to various reasons. One point, the RFC Status was still Under Discussion, which has been corrected. No other changes have been made to the text. I will be able to resume work today and will continue implementation and refactoring with PHP 8.2 in mind. P.S. > Add Randomizer::pickArrayKeys(array $array, int $num): array method This seems to be relatively unpopular, and I would like to know the reasons for those who voted No. Is it because this function, like array_rand, is overly complex in its behavior? Regards, Go Kudo
[PHP-DEV] [VOTE] Random Extension Improvement
Hello internals. Voting began on 2022-07-02 03:00:00 (UTC) and will end on 2022-07-16 03:00:00 (UTC). https://wiki.php.net/rfc/random_extension_improvement Best Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月29日(水) 17:40 Guilliam Xavier : > >> > https://wiki.php.net/rfc/random_extension_improvement > >> > >> I just realized a little thing: in the array_rand() example, for > >> $beforeSingle, it would probably be "more realistic" to omit `, 1` > >> (which is already the default for $num). > >> > >> Note: for `Randomizer::pickArrayKeys(array $array, int $num): array`, > >> it makes sense that $num does *not* have a default value (1 would be > >> "weird" because the method always returns a *list of keys*, and > >> count($array) [via null] would be "useless" because keys are returned > >> *in their original order* [so it would make the method equivalent to > >> array_keys($array) by default]), > >> and that's probably a good thing (it forces to update the call by > >> adding an explicit `, 1` argument and reminds to add a `[0]` or > >> similar on the returned value). > >> > >> An alternative design would be `Randomizer::pickArrayKey(array > >> $array): int|string`, but migrating existing uses with $num != 1 would > >> be harder, so probably not better. > > > > This is certainly a complicated issue. > > > > I proposed the signature `Randomizer::arrayPickKeys(array $array, int > $num): array` because it can be solved with the current PHP sugar syntax > and the default value of $num is 1 despite the name "arrayPickKeys". > > > > However, this is a bit tricky and may not be user-friendly for the > average user. > > > > So, how about adding two methods, `Randomizer::arrayPickKey(array > $array): int|string` and `Randomizer::arrayPickKeys(array $array, int > $num): array`? > > > > This may seem redundant, but it may avoid user confusion. > > Sorry if I wasn't clear: I just suggested to make this little change > in the example: > > ```diff > -$beforeSingle = array_rand(['foo' => 'foo', 'bar' => 'bar', 'baz' => > 'baz'], 1); // (string) foo > +$beforeSingle = array_rand(['foo' => 'foo', 'bar' => 'bar', 'baz' => > 'baz']); // (string) foo > ``` > > to make it more "realistic". > > As concerns the rest (about pickArrayKeys): sorry for the digression, > I was just "thinking out loud", I *don't* want any change there > (first, it makes sense that pickArrayKeys has `int $num` *without* a > default value [even if array_rand has 1]; second, "pickArrayKey" [if > really wanted] is trivial to implement in userland as a wrapper around > pickArrayKeys [but the opposite would not be so], and I don't think > that adding *both* methods to Randomizer is desirable either [better > keep it simple/minimal]). > > Regards, > > -- > Guilliam Xavier > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi Thank you. That feels true. I will try to keep the RFC as it currently stands. Sorry for the delay in replying. I was a little held up with personal business. I will delay the start of voting by one day. Best regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月29日(水) 0:39 Guilliam Xavier : > > Hi Internals. > > > > Random Extension 5.x has been accepted by a vote of 20(+1)/0. (I made a > > mistake in timing the closing of the vote and thus received one more > vote) > > Therefore, voting on the Random Extension Improvement RFC will begin on > > 2022-07-02 as scheduled. > > > > Please check the RFC. This is the last chance to improve the > implementation. > > > > https://wiki.php.net/rfc/random_extension_improvement > > Hi, > > I just realized a little thing: in the array_rand() example, for > $beforeSingle, it would probably be "more realistic" to omit `, 1` > (which is already the default for $num). > > Note: for `Randomizer::pickArrayKeys(array $array, int $num): array`, > it makes sense that $num does *not* have a default value (1 would be > "weird" because the method always returns a *list of keys*, and > count($array) [via null] would be "useless" because keys are returned > *in their original order* [so it would make the method equivalent to > array_keys($array) by default]), > and that's probably a good thing (it forces to update the call by > adding an explicit `, 1` argument and reminds to add a `[0]` or > similar on the returned value). > > An alternative design would be `Randomizer::pickArrayKey(array > $array): int|string`, but migrating existing uses with $num != 1 would > be harder, so probably not better. > > Regards, > > -- > Guilliam Xavier > This is certainly a complicated issue. I proposed the signature `Randomizer::arrayPickKeys(array $array, int $num): array` because it can be solved with the current PHP sugar syntax and the default value of $num is 1 despite the name "arrayPickKeys". However, this is a bit tricky and may not be user-friendly for the average user. So, how about adding two methods, `Randomizer::arrayPickKey(array $array): int|string` and `Randomizer::arrayPickKeys(array $array, int $num): array`? This may seem redundant, but it may avoid user confusion. Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月18日(土) 4:42 Go Kudo : > Hi internals. > > An RFC has been created to fix an issue in Random Extension 5.x. > > https://wiki.php.net/rfc/random_extension_improvement > > Voting on this RFC will begin in two weeks (2022-07-02), in time for the > PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is > 2022-07-19) > > In the unlikely event that the Random Extension 5.x RFC is rejected, this > RFC will become invalid regardless of the outcome of the vote. > > Best regards > Go Kudo > Hi Internals. Random Extension 5.x has been accepted by a vote of 20(+1)/0. (I made a mistake in timing the closing of the vote and thus received one more vote) Therefore, voting on the Random Extension Improvement RFC will begin on 2022-07-02 as scheduled. Please check the RFC. This is the last chance to improve the implementation. https://wiki.php.net/rfc/random_extension_improvement Best regards Go Kudo
[PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月14日(火) 9:01 Go Kudo : > Hello internals. > > Voting began on 2022-06-14 00:00:00 (UTC) and will end on 2022-06-28 > 00:00:00 (UTC). > > https://wiki.php.net/rfc/rng_extension > > The implementation is not yet complete and has some issues. > See TODO in Pull Request for details. > > https://github.com/php/php-src/pull/8094 > > Best Regards, > Go Kudo > Hello Internals. Voting is now closed as the deadline has passed. This RFC accepted with 20 yes / 0 no. https://wiki.php.net/rfc/rng_extension Meanwhile, a new RFC will be voted on to improve the content of the RFC. https://wiki.php.net/rfc/random_extension_improvement https://externals.io/message/117994 Thank you for your continued support. Best regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月23日(木) 0:02 Tim Düsterhus : > Hi > > On 6/22/22 16:35, Go Kudo wrote: > > No additional comments seemed to be forthcoming, so the RFC was upgraded > to > > 1.5. > > The following changes have been made > > > > https://wiki.php.net/rfc/random_extension_improvement > > > > 1. Add: `Refine classnames` > > 2. Add: `Random\SerializableEngine is outdated` > > 3. Add `Add Randomizer::pickArrayKeys(array $array, int $num): array > > method` *1 > > 4. Add `Random\SerializableEngine is outdated` > > 5. Remove: `PCG64 is ambiguous` (replaced by 1) > > 6. Remove: `Mersenne Twister is ambiguous` (replaced by 1) > > 7. Remove: `Randomizer lacks array_rand() replacement method` (replaced > by > > 3) > > > > *1: Added with a little sample code. > > Thank you for the update. The grouping makes sense to me and it looks > very organized. > > Let me just propose some wording changes: > > a) > > > Random\SerializableEngine is outdated > > I would rename the headline to "Random\SerializableEngine is not > useful", that's a little more fitting. > > b) > > > CombinedLCG is outdated > > I would rename the headline to "Random\Engine\CombinedLCG is low > quality", that's a little more accurate. > > c) > > In the "Refine classnames" section: > > > To make it more readable and regular, the class name is changed as > follows: > > I would reword this as: > > To clearly identify the implemented algorithm the PCG64 and > MersenneTwister twister engines should be renamed to their canonical > upstream name: > > The issue with the previous wording is it's not clear what "more > regular" means. > > d) > > For the vote titles I propose the following changes for a more > consistent wording that succinctly describes the change to avoid voter > confusion: > > Engine implementations to final >to > Make all implemented engines 'final'? > > Remove Random\SerializableEngine >to > Remove the SerializableEngine interface? > > Drop Random\Engine\CombinedLCG >to > Remove the CombinedLCG engine? > > Add Random\Randomizer::pickArrayKeys(array $array, int $num): array >to > Add the pickArrayKeys() method to the Randomizer? > > Rename Random\Randomizer::shuffleString() to > Random\Randomizer::shuffleBytes() >to > Rename Randomizer::shuffleString() to Randomizer::shuffleBytes()? > > Change classnames >to > Rename PCG64 and MersenneTwister? > > Implement Random\Engine\Xoshiro256StarStar >to > Add the Xoshiro256StarStar engine? > > Best regards > Tim Düsterhus > Hi Tim Thanks for the suggestion! It looked much better to me and I have reflected it in the RFC. "Remove the CombinedLCG engine?" replaced by "Remove the CombinedLCG class?". The reason is that the implementation will still remain for backward compatibility. (only the class will be removed). Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月18日(土) 4:42 Go Kudo : > Hi internals. > > An RFC has been created to fix an issue in Random Extension 5.x. > > https://wiki.php.net/rfc/random_extension_improvement > > Voting on this RFC will begin in two weeks (2022-07-02), in time for the > PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is > 2022-07-19) > > In the unlikely event that the Random Extension 5.x RFC is rejected, this > RFC will become invalid regardless of the outcome of the vote. > > Best regards > Go Kudo > Hi No additional comments seemed to be forthcoming, so the RFC was upgraded to 1.5. The following changes have been made https://wiki.php.net/rfc/random_extension_improvement 1. Add: `Refine classnames` 2. Add: `Random\SerializableEngine is outdated` 3. Add `Add Randomizer::pickArrayKeys(array $array, int $num): array method` *1 4. Add `Random\SerializableEngine is outdated` 5. Remove: `PCG64 is ambiguous` (replaced by 1) 6. Remove: `Mersenne Twister is ambiguous` (replaced by 1) 7. Remove: `Randomizer lacks array_rand() replacement method` (replaced by 3) *1: Added with a little sample code. Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月21日(火) 18:23 Guilliam Xavier : > On Mon, Jun 20, 2022 at 5:25 PM Christoph M. Becker > wrote: > > > > On 20.06.2022 at 16:44, Go Kudo wrote: > > > > > 2022年6月20日(月) 23:37 Lynn : > > > > > >> On Mon, Jun 20, 2022 at 3:15 PM Guilliam Xavier < > guilliam.xav...@gmail.com > > >> wrote: > > >> > > >>>> https://wiki.php.net/rfc/random_extension_improvement > > >>> > > >>> Thanks, but I am not sure about your argument in "Classnames are not > > >>> canonicalized": does "PHP applies strict PascalCase to class names" > > >>> (which remains to be proved) really imply to rename *acronyms* (e.g. > > >>> "CombinedLCG" to "CombinedLcg")? especially given existing classes > > >>> like "SimpleXMLElement" (not "SimpleXmlElement"), and that the > > >>> accepted "Class Naming" RFC (https://wiki.php.net/rfc/class-naming) > > >>> voted for "PascalCase except Acronyms" (not "Always PascalCase") -- > > >>> excerpts: > > >> > > >> Not specifically directed at this discussion, but perhaps this needs a > > >> revision. HTTPStatus is much harder to read for me than HttpStatus > and it's > > >> unclear where the boundary of an acronym starts or stops. If anyone > ever > > >> decides to make an RFC for this, you have my vote. These Acronyms are > > >> treated as words and thus should follow the same naming convention. > If they > > >> shouldn't be treated as words, write their full name: > > >> HypertextTransferProtocolStatus. > > > > > > I support "PascalCase except Acronyms" for readability, but would like > to > > > see this > > > clarified as I get very lost when implementing new features. > > > I think it is necessary because I expect various OO APIs will be added > in > > > the future, > > > like cURL. > > > > In my opinion, <https://wiki.php.net/rfc/class-naming> was a bit > > unfortunate. It may have been better to decide on a case by case basis. > > > > For instance, we have introduced several Curl* classes in PHP 8.0[1], > > and these adhere to the appropriate example in the RFC, although CURL is > > clearly an acronym[2], and the canonical spelling is even cURL. Maybe > > even worse, the previously introduced CURLFile[3] uses different > > capitalization, and CURLStringFile[4] which was introduced in PHP 8.1 is > > aligned to that spelling. > > > > So, obviously, the RFC didn't have a good impact on some of the namings > > so far. > > That seems unfortunate indeed :/ and there are others, e.g. > "XMLReader" but "XmlParser"... Moreover, I see that > > https://github.com/php/php-src/blob/master/CODING_STANDARDS.md#user-functionsmethods-naming-conventions > item 7 only says "should", not "must". > > I too find "HttpStatus" [or "HTTP_status", for that matter] more > readable than "HTTPStatus" (where I first see "HTTPS" before noticing > that the S actually starts a second word), and "PcgOneseq128XslRr64" > than "PCGOneseq128XSLRR64" (especially if not already familiar with > "PCG" and "XSL-RR")... > > So, it may indeed be OK (and preferred?) to "canonicalize" the > proposed class names (just, "PHP applies strict PascalCase to class > names" wasn't a good argument for it). > > PS @Go Kudo: please don't be offended, but in general, maybe you > should wait "a bit more" [or even ask] for other opinions, rather than > changing the RFC "too fast" after only one (especially when I said "I > am not sure" and asked a question) > > Regards, > > -- > Guilliam Xavier > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi It's a difficult problem... At least in this thread, "Always PascalCase" seems to be more favored. (I know this is probably not the best way to put it, but I can't think of any other analogy...) I am wondering how to write to the RFC, but if there are no problems, I will make the following changes. - Change to `Randomizer::pickArrayKey(array $array, int $num = 1): int|string|array` -> `Randomizer::pickArrayKeys(array $array, int $num): array` - Apply "Always PascalCase" based on ML's opinion > PS Thanks for the advice. I am not very familiar with the OSS community, so it helps a lot. Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月21日(火) 0:42 Tim Düsterhus : > Hi > > On 6/20/22 17:12, Go Kudo wrote: > >> CombinedLCG > > > > This is provided as an OOP implementation for the `lcg_value()` function, > > but I don't actually > > want it to be used anymore, so I probably shouldn't provide a class for > it. > > > > And to begin with, the current CombinedLCG cannot even be seeded with > > arbitrary values. > > > > However, I think it needs to remain in the internal API either way. (The > > option of not providing > > it to userland is a valid one.) > > I wouldn't object to dropping CombinedLCG, especially since its internal > parameters are not defined via the name (contrary to MT19937). > > > What do you think about the `Random\CryptoSecureEngine` interface? > > It is just a marker interface with no methods. > > > > However, I currently think it is better than adding a method like > > `isSecure(): bool` > > to the `Random\Engine`. > > > > I *much* prefer the marker interface. > > Best regards > Tim Düsterhus > Hi Added option to discontinue CombinedLCG. https://wiki.php.net/rfc/random_extension_improvement I am struggling with the following issue: 1. change `Randomizer::pickArrayKey(array $array, int $num =1): int|string|array` to `Randomizer::pickArrayKeys(array $array, in $num): array`. 2. Change the class name to "Always PascalCase" or "PascalCase except Acronyms". I hope I can solve the discussion by e-mail, but if I can't, I will further add it to the RFC options. Best regards, Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月20日(月) 23:58 Nicolas Grekas : > > An RFC has been created to fix an issue in Random Extension 5.x. > > > > https://wiki.php.net/rfc/random_extension_improvement > > > > Voting on this RFC will begin in two weeks (2022-07-02), in time for the > > PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is > > 2022-07-19) > > > > In the unlikely event that the Random Extension 5.x RFC is rejected, this > > RFC will become invalid regardless of the outcome of the vote. > > > > Hi, thanks for the update, that makes sense to me. > > I'm wondering: does Random\SerializableEngine extend Random\Engine? Can > this be mentioned in the RFC? If not, what about making it this way? Having > this interface only contain __(un)serialize would look strange to me, aka > too broad for the name and the purpose. > > I'm also wondering: is CombinedLCG worth it? I must admit I don't know when > I should use it instead of MT19937. > > Since the names are all super opaque to many of us, documentation should be > very clear about the use case of each implementation... (if can reduce the > number of implementations, that's even better :) ) > > Cheers, > Nicolas > Hi > Having this interface only contain __(un)serialize would look strange to me, aka > too broad for the name and the purpose. Indeed. This was designed back when the Serializable interface was still going strong, so it is already outdated. The OO approach to serialization no longer applies, and this may need to be eliminated. > CombinedLCG This is provided as an OOP implementation for the `lcg_value()` function, but I don't actually want it to be used anymore, so I probably shouldn't provide a class for it. And to begin with, the current CombinedLCG cannot even be seeded with arbitrary values. However, I think it needs to remain in the internal API either way. (The option of not providing it to userland is a valid one.) What do you think about the `Random\CryptoSecureEngine` interface? It is just a marker interface with no methods. However, I currently think it is better than adding a method like `isSecure(): bool` to the `Random\Engine`. Best regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月20日(月) 23:37 Lynn : > On Mon, Jun 20, 2022 at 3:15 PM Guilliam Xavier > > wrote: > > > Hi, > > > > > https://wiki.php.net/rfc/random_extension_improvement > > > > Thanks, but I am not sure about your argument in "Classnames are not > > canonicalized": does "PHP applies strict PascalCase to class names" > > (which remains to be proved) really imply to rename *acronyms* (e.g. > > "CombinedLCG" to "CombinedLcg")? especially given existing classes > > like "SimpleXMLElement" (not "SimpleXmlElement"), and that the > > accepted "Class Naming" RFC (https://wiki.php.net/rfc/class-naming) > > voted for "PascalCase except Acronyms" (not "Always PascalCase") -- > > excerpts: > > > > Not specifically directed at this discussion, but perhaps this needs a > revision. HTTPStatus is much harder to read for me than HttpStatus and it's > unclear where the boundary of an acronym starts or stops. If anyone ever > decides to make an RFC for this, you have my vote. These Acronyms are > treated as words and thus should follow the same naming convention. If they > shouldn't be treated as words, write their full name: > HypertextTransferProtocolStatus. > I support "PascalCase except Acronyms" for readability, but would like to see this clarified as I get very lost when implementing new features. I think it is necessary because I expect various OO APIs will be added in the future, like cURL. But, I do not have the right to vote. :)
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月20日(月) 22:16 Guilliam Xavier : > Hi, > > > https://wiki.php.net/rfc/random_extension_improvement > > Thanks, but I am not sure about your argument in "Classnames are not > canonicalized": does "PHP applies strict PascalCase to class names" > (which remains to be proved) really imply to rename *acronyms* (e.g. > "CombinedLCG" to "CombinedLcg")? especially given existing classes > like "SimpleXMLElement" (not "SimpleXmlElement"), and that the > accepted "Class Naming" RFC (https://wiki.php.net/rfc/class-naming) > voted for "PascalCase except Acronyms" (not "Always PascalCase") -- > excerpts: > > | Abbreviations start with a capital letter followed by lowercase > letters, whereas acronyms and initialisms are written according to > their standard notation. > > | Good: > | 'CurlResponse' > | 'HTTPStatusCode' > > | Bad: > | 'curl_response' > | 'HttpStatusCode' > > (Not that I really care but...) > > Regards, > > -- > Guilliam Xavier > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi > "Class Naming" RFC (https://wiki.php.net/rfc/class-naming) My apologies! I had missed this RFC. I had assumed from the `XmlParser` implementation that it was "Always PascalCase". Actually, I much prefer "PascalCase except Acronyms". I have corrected the RFC and removed the section. Thanks! Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月19日(日) 16:25 Marc : > Hi Go Kudo, > > About "pickArrayKey" I don't like the behavior change on number of array > keys to return either a single key or an array of keys. > It would be better to have different methods for both cases. > > Also it does not describe the behavior if you are requesting more elements > as in the given array. > > Will the returned list of keys be unique? > > Additionally in array_rand documentation: > > > If multiple keys are returned, they will be returned in the order they > were present in the original array. > > Does that make sense? And would your method behave the same? > > > Am 17.06.2022 21:41 schrieb Go Kudo : > > Hi internals. > > An RFC has been created to fix an issue in Random Extension 5.x. > > https://wiki.php.net/rfc/random_extension_improvement > > Voting on this RFC will begin in two weeks (2022-07-02), in time for the > PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is > 2022-07-19) > > In the unlikely event that the Random Extension 5.x RFC is rejected, this > RFC will become invalid regardless of the outcome of the vote. > > Best regards > Go Kudo > > > Hi Marc. > It would be better to have different methods for both cases. I agree. The current specification is intended to be a drop-in replacement for `array_rand()`. A better API should be provided. In my opinion, the ability to return a single key by value when `int $num` is 1 is unnecessary. I don't need the ability to return a single key by value if `int $num` is 1, since I can simply refer to the first element of the array even if it is returned as an array. What about a signature like `Randomizer::pickArrayKeys(array $array, int $num): array`? The previous behavior when `int $num` is 1 can be easily reproduced as follows: ```php $randomizer = new Random\Randomizer(new Random\Engine\Mt19937(1234, MT_RAND_PHP)); [$key] = $randomizer->pickArrayKeys(['foo', 'bar', 'baz'], 1); ```php > If multiple keys are returned, they will be returned in the order they were present in the original array. Perhaps there is no particular significance to this specification. I don't think it is necessary, but considering the drop-in replacement from array_rand(), I think it is desirable to leave it as is. At least as of PHP 8.2, we would like to avoid BC Break as much as possible. Best regards Go Kudo
[PHP-DEV] Re: [RFC] [Under Discussion] Random Extension Improvement
2022年6月18日(土) 4:41 Go Kudo : > Hi internals. > > An RFC has been created to fix an issue in Random Extension 5.x. > > https://wiki.php.net/rfc/random_extension_improvement > > Voting on this RFC will begin in two weeks (2022-07-02), in time for the > PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is > 2022-07-19) > > In the unlikely event that the Random Extension 5.x RFC is rejected, this > RFC will become invalid regardless of the outcome of the vote. > > Best regards > Go Kudo > Hello. The new name for PCG64 was not appropriate (Pcg64OneseqXslRr64) and was changed to something more appropriate (PcgOneseq128XslRr64). Best regards. Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
2022年6月18日(土) 4:50 Tim Düsterhus : > Hi > > On 6/17/22 21:41, Go Kudo wrote: > > https://wiki.php.net/rfc/random_extension_improvement > > > > There's still a section for array_rand() at the top of the RFC > ( > https://wiki.php.net/rfc/random_extension_improvement#randomizer_lacks_array_rand_replacement_method), > > but no voting section. > > My understanding is that you don't plan to add an array_rand() > replacement, then you should remove that section. > > Best regards > Tim Düsterhus > Oops, I fixed it. Thank you! Best regards Go Kudo
Re: [PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月18日(土) 4:16 Tim Düsterhus : > Hi > > On 6/17/22 20:37, Go Kudo wrote: > > For the reasons stated above, we will abandon the addition of new > methods. > > sorry. > > Okay, that's fine for me, no problem. > > > `str_shuffle()` and `shuffleString()` already have similar problems. So > > perhaps an alternative > > method name for `str_shuffle()` might be `bytesShuffle()` instead of > > `stringShuffle()`. > > > > No opinion there. > > To make sure that the updates are voted on in time I suggest that you > open the official 2-week discussion with a dedicated thread. > > Best regards > Tim Düsterhus > OK, My greatest thanks to you for your cooperation! Best regards Go Kudo
[PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
Hi internals. An RFC has been created to fix an issue in Random Extension 5.x. https://wiki.php.net/rfc/random_extension_improvement Voting on this RFC will begin in two weeks (2022-07-02), in time for the PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is 2022-07-19) In the unlikely event that the Random Extension 5.x RFC is rejected, this RFC will become invalid regardless of the outcome of the vote. Best regards Go Kudo
Re: [PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月18日(土) 3:13 Tim Düsterhus : > Hi > > On 6/17/22 19:46, Go Kudo wrote: > > I was fundamentally wrong, I understand now. > > As you said, there was no interoperability with `pickArrayKey()` in the > > first place... > > > >> stringFromAlphabet() > > > > Hmmm. I guess randomString would be better then. At the same time, it > would > > be nice to have an array version of randomArray. > > > > However, I don't want to add more methods without any thought. > > I think operations that can be done on userland should be done on > userland. > > That is why I did not implement the array_rand() function in the first > > place. > > Yes, I agree here. But I believe that "generate a string with a given > alphabet" is a very common operation that would be useful to include in > the standard library. In any case it's better to leave something out > than to implement something badly, so if you don't feel comfortable with > that, then leave it out. There will be more PHP versions after 8.2. > > For me both of these: > > ->randomString(string $alphabet, int $length) > ->stringFromAlphabet(string $alphabet, int $length) > > with the description "Return a string of $length characters selected > from the given $alphabet. Characters may be selected more than once". > > would be acceptable names. > > Best regards > Tim Düsterhus > I noticed one important thing. "String" in PHP means binary, and operating on multibyte characters often causes problems. Although I rarely deal with multibyte characters in my work, this can probably be a big problem for Japanese PHP users like myself. To work around this, the mbstring extension must be used properly, but since mbstring is not built-in, it is appropriate to implement it in userland. For the reasons stated above, we will abandon the addition of new methods. sorry. `str_shuffle()` and `shuffleString()` already have similar problems. So perhaps an alternative method name for `str_shuffle()` might be `bytesShuffle()` instead of `stringShuffle()`. Regards Go Kudo
Re: [PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月18日(土) 2:37 Tim Düsterhus : > Hi > > On 6/17/22 19:28, Go Kudo wrote > >> I don't think that ->pickString() is a good name > > > > I see. But I think `randomString()` is ambiguous with `getBytes()`. > > > > `stringFromCharset(string $string, int $num): string` solves that, but I > > think it is possible > > that the meaning of "char" is not well known in the PHP world (although I > > think this name is most appropriate) > > ->stringFromAlphabet()? > > > How about adding an optional `?int $num` argument to > `shuffleString(string > > $string, ?int $num): string`? > > > > No, because it would be pretty unclear what that `$num` argument would > do there. It specifically would be different from the `$num` of > `pickArrayKey()`. pickArrayKey() returns every key only once. Generating > a string from a given charset may return the same character multiple > times. Don't overload a single method with too many purposes. > > Best regards > Tim Düsterhus > I was fundamentally wrong, I understand now. As you said, there was no interoperability with `pickArrayKey()` in the first place... > stringFromAlphabet() Hmmm. I guess randomString would be better then. At the same time, it would be nice to have an array version of randomArray. However, I don't want to add more methods without any thought. I think operations that can be done on userland should be done on userland. That is why I did not implement the array_rand() function in the first place. I would like to hear other people's opinions in this area. Incidentally, our own PHP implementation of the library (Xorshift128+) has equivalents for arrayPickKey, stringPick, arrayPickValue, randomArray, randomString. And it is useful. Regards Go Kudo
Re: [PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月18日(土) 2:14 Tim Düsterhus :
> Hi
>
> On 6/17/22 19:04, Go Kudo wrote:
> > RFC has been updated. Includes corrections to areas pointed out by Tim
> and
> > changes MersenneTwister to MT19937.
> > I also made it possible to vote for each item.
>
> I suggest to split the "PCG is not so famous" vote into 2 votes to make
> it clear how exactly the majority is calculated and to have a clear
> primary vote as indicated in
>
> https://wiki.php.net/rfc/voting#required_majority
>
> > https://wiki.php.net/rfc/random_extension_improvement
> >
> > How about it?
> >
> > for Tim:
> >
> >> I believe you missed my suggestion (4)
> >
> > My apologies! I had completely missed that.
> > That new feature sounds good to me. But, I think the method name
> > `pickString()` would be better. (It is interoperable with
> `pickArrayKey()`)
>
> I don't think that ->pickString() is a good name, because it is not
> really comparable to pickArrayKey(): pickArrayKey() will return each key
> only once. It is more comparable to:
>
> substr($r->shuffleString('0123456789abcdef'), 0, 6)
>
> My proposed ->randomString() must be able to return a character multiple
> times.
>
> If you don't like ->randomString(), I have an alternative suggestion:
> ->stringFromCharset()
>
> Best regards
> Tim Düsterhus
>
> "PCG is not so famous" vote into 2 votes
My apologies. This is a complete mistake.
Since PCG64 has already clarified its implementation in an earlier RFC,
removing it
does not seem to be an option. The item has been removed.
> I don't think that ->pickString() is a good name
I see. But I think `randomString()` is ambiguous with `getBytes()`.
`stringFromCharset(string $string, int $num): string` solves that, but I
think it is possible
that the meaning of "char" is not well known in the PHP world (although I
think this name is most appropriate)
How about adding an optional `?int $num` argument to `shuffleString(string
$string, ?int $num): string`?
Regards
Go Kudo
[PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月14日(火) 9:01 Go Kudo : > Hello internals. > > Voting began on 2022-06-14 00:00:00 (UTC) and will end on 2022-06-28 > 00:00:00 (UTC). > > https://wiki.php.net/rfc/rng_extension > > The implementation is not yet complete and has some issues. > See TODO in Pull Request for details. > > https://github.com/php/php-src/pull/8094 > > Best Regards, > Go Kudo > Hi. RFC has been updated. Includes corrections to areas pointed out by Tim and changes MersenneTwister to MT19937. I also made it possible to vote for each item. https://wiki.php.net/rfc/random_extension_improvement How about it? for Tim: > I believe you missed my suggestion (4) My apologies! I had completely missed that. That new feature sounds good to me. But, I think the method name `pickString()` would be better. (It is interoperable with `pickArrayKey()`) Added to the RFC.
Re: [PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月16日(木) 22:37 Tim Düsterhus : > Hi > > On 6/16/22 14:52, Tim Düsterhus wrote: > > (3) Naming of PCG64: > > > > I must admit that the fact that PCG is a full family of similar, but not > > identical generators is one thing that made me (and still makes me) > > prefer the xoshiro family which has clearer names for its variants. > > > > It was also pretty hard to find the PCG definitions on the PCG website, > > but I believe that it refers to this: > > > > https://www.pcg-random.org/using-pcg-c.html#low-level-api? > > > > In that case PCG64S would be consistent with the upstream high level API > > name. I am not sure if Pcg64s would be more readable, though. > > > > It would definitely need a good explanation in the documentation which > > exact variant it is, though. > > I've now had a look at the Paper [1], because I wanted to find out what > the various bits and pieces within the full oneseq-128-xsl-rr-64 name > mean and in the paper I came across section "6.3 Specific > Implementations" which notes: > > > The library provides named generators based on > > their properties, not their underlying implementations (e.g., > pcg32_unique for a > > general-purpose 32-bit generator with a unique stream). That way, when > future family > > members that perform even better are discovered and added (hopefully due > to the > > discoveries of others), users can switch seamlessly over to them. > > This implies that the official Pcg64s name might refer to a different > implementation in the future. This makes it hard for PHP to keep the > compatibility guarantee that a specific engine will always refer to a > specific well-defined RNG. This implies that the engine needs to be > named "PcgOneseq128XslRr64" to accurately describe the implementation. > This - of course - is absolutely unwieldy. Note sure what the best > course of action is here. > > [1] https://www.pcg-random.org/pdf/hmc-cs-2014-0905.pdf > > Best regards > Tim Düsterhus > Hello Tim Thank you for your continued support. (1) Thanks! (2) You are indeed correct. I renamed it to pickArrayKey(). (3) This is a very tricky problem. As you point out, it took me a while to figure out PCG64 too. However, after statistical testing PCG64 (pcg_oneseq-128-xsl-rr-64) seems to do a good job. (Though Xoshiro256** does as well, and has a more obvious name.) Regarding the naming issue, I have looked at implementations in other languages (Python's NumPy and Rust) and none of them seem to be very clear. I agree with you about keeping the names clear. Although it is complicated, I don't think anyone would be particularly bothered by the name PcgOneseq128XslRr64. However, some people may continue to use MT because they don't understand it well. I think the possible options are as follows A. Rename PCG64 to PcgOneseq128XslRr64 B. Rename PCG64 to PcgOneseq128XslRr64 and then re-implement the more obvious Xoshiro256StarStar C. Remove PCG64 and re-implement Xoshiro256StarStart Personally, I think B is the best. What do you think? Regards Go Kudo
Re: [PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月16日(木) 19:56 Guilliam Xavier : > Hi, > > > https://wiki.php.net/rfc/random_extension_improvement > > Thanks; some editorial remarks: > > - I think "Engine is not final" should rather be "Engine > implementations are not final", and similarly in the text: "Even if > the native class is made final" to "Even if the native classes are > made final", "and the native implementation of the class should be > marked as final" to "and the native implementations of the interface > should be marked as final" > - The sentence "The engine already provides an interface with only a > generate(): string method called Random\Engine." sounds weird, maybe > rather "The extension already provides a Random\Engine interface with > a single generate(): string method."? > - "Proposal" is a subsection inside "Introduction", it should rather > be a root section (at the same level as "Backward Incompatible > Changes") > - What would be the signature of Randomizer::pickKey()? (whatever > the naming, important is parameters and return types) -- I guess the > same as existing array_rand(), but better be explicit > > Regards, > > -- > Guilliam Xavier > Hi Xavier. Thanks very much for the detailed point of view. I am very ashamed of my poor English. Maximum thanks! I have corrected all the points you pointed out. Regards Go Kudo
[PHP-DEV] Re: [RFC] [VOTE] Random Extension 5.x
2022年6月14日(火) 9:01 Go Kudo : > Hello internals. > > Voting began on 2022-06-14 00:00:00 (UTC) and will end on 2022-06-28 > 00:00:00 (UTC). > > https://wiki.php.net/rfc/rng_extension > > The implementation is not yet complete and has some issues. > See TODO in Pull Request for details. > > https://github.com/php/php-src/pull/8094 > > Best Regards, > Go Kudo > Hello internals. Thank you for voting for RFC. Now, as Tim pointed out *1, there are several problems with the current RFC implementation. However, the RFC is already in the voting phase, and it is not advisable to change the content now. I have drafted a new RFC to fix this. https://wiki.php.net/rfc/random_extension_improvement I was hesitant to create a new RFC page while the current RFC was still in the early voting stage, but I thought it was time to do it now, given the lack of progress in previous discussions and the upcoming PHP 8.2 Feature Freeze. If the content is acceptable, we would like to change the status of the RFC to Under Discussion and make an announcement thread to internals ML. Can anyone review the content? *1: https://externals.io/message/117939#117955 Best Regards Go Kudo
Re: [PHP-DEV] [RFC] [VOTE] Random Extension 5.x
2022年6月16日(木) 2:23 Tim Düsterhus : > Hi > > On 6/14/22 02:01, Go Kudo wrote: > > Voting began on 2022-06-14 00:00:00 (UTC) and will end on 2022-06-28 > > 00:00:00 (UTC). > > > > https://wiki.php.net/rfc/rng_extension > > > > The implementation is not yet complete and has some issues. > > See TODO in Pull Request for details. > > > > https://github.com/php/php-src/pull/8094 > > > > Unfortunately the vote has already started and I'm not sure if that's a > change that might change the outcome of the vote, but while looking > through the implementation once more I noticed that the engine > implementations are not 'final' (and extending those engines is actually > tested with the existing tests). > > However I believe they should be final: > > a) I generally believe that it's a best practice to make everything > 'final' by default. > > b) It's easily possible to use composition with engines, as the > interface only has a single method. > > c) Especially for 'Random\Engine\Secure' I believe that allowing > subclassing is actively harmful, as basically any adjustment of the > engine's behavior violates the contract that the engine returns > cryptographically secure randomness. But also for other engines changing > the behavior also changes the implied behavior given by the engine's name. > > What do you think? > > Best regards > Tim Düsterhus > Hi Tim > However I believe they should be final That is correct, indeed. The interface is already provided and creating a composition is easy. However, the voting has already started. It would be impossible to edit the RFC now. Fortunately, the Feature Freeze for PHP 8.2 is 7/19. Even after the current Random Extension 5.x RFC voting is over, there is still time to create and vote on RFCs to make changes. I will create an additional RFC like PHP 8.0 Attribute. Regards Go Kudo
[PHP-DEV] [RFC] [VOTE] Random Extension 5.x
Hello internals. Voting began on 2022-06-14 00:00:00 (UTC) and will end on 2022-06-28 00:00:00 (UTC). https://wiki.php.net/rfc/rng_extension The implementation is not yet complete and has some issues. See TODO in Pull Request for details. https://github.com/php/php-src/pull/8094 Best Regards, Go Kudo
[PHP-DEV] Re: [RFC] [Vote] Pre-vote announcement for Random Extension 5.x
2022年5月31日(火) 18:54 Go Kudo : > Hi internals. > > Although I have explained that due to the global turmoil I will delay > voting on the RFC as long as possible, it is time to start voting on the > RFC in order to get the implementation to full status by the PHP 8.2 > Feature Freeze. > > I apologize for the delay in responding to the points you had already > pointed out. It has been addressed as follows. > > - Random\Engine\PCG64::__construct() now takes an `int|string` $inc as its > second argument > - This makes it fully compatible with the PCG64 original implementation > - Fixed an algorithm implementation error in PCG64 > - Fixed compatibility issues with PHP 8.2 in test cases > - More detailed description in RFC > > Previous discussion thread: https://externals.io/message/117295 > > Voting will begin at 2022-06-14 00:00:00 (UTC). > > https://wiki.php.net/rfc/rng_extension > > Regards, > Go Kudo > Good evening. While refactoring, I found an error in the PCG64 algorithm we are implementing. It should be implemented as pcg64_oneseq_128 (XSL-RR), but currently pcg64_setseq_128 (XSL-RR-RR) is implemented. The RFC also incorrectly states that NumPy implements pcg64_oneseq_128. https://wiki.php.net/rfc/rng_extension The RFC has been corrected for the above. This change removes the int|string $inc argument from RandomEngine\PCG64::__construct(), leaving only int|string|null $seed. Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Vote] Pre-vote announcement for Random Extension 5.x
2022年6月13日(月) 16:14 Tim Düsterhus : > Hi > > On 6/13/22 04:23, Go Kudo wrote: > > I have created a PoC that allows all internal operations to be performed > in > > 64-bit environments to achieve the same results, although the efficiency > of > > execution in 32-bit environments will be reduced. (Note that > > Randomizer::getInt() with no argument is still incompatible.) > > > > > https://github.com/php/php-src/commit/dbed218bfcd45e713fa3df2c88a4c2efce9f0651 > > > > I believe this is a good solution. The majority of the modern setups > (i.e. the setups that are using PHP 8.2) will likely be 64-bit anyway > and reduced performance on 32-bit is then acceptable. > > Best regards > Tim Düsterhus > Hi Tim Thanks! I have updated the implementation and RFCs. https://wiki.php.net/rfc/rng_extension Voting will open tomorrow (2022-06-14 00:00:00 UTC) as planned. I have reviewed the implementation and would like to clean up the implementation as it is cluttered with past spec changes. This may possibly be too late to start voting, but I do not plan to change the behavior in any way. Best regards, Go Kudo
Re: [PHP-DEV] [RFC] [Vote] Pre-vote announcement for Random Extension 5.x
2022年6月10日(金) 19:42 Tim Düsterhus : > Hi > > On 6/10/22 12:02, Go Kudo wrote: > >> It has a single generate(): string method that generates random numbers > > as a binary string. This string must be non-empty and attempting to > return > > an empty will result in a RuntimeException. > >> If you implement a random number generator in PHP, the generated numbers > > must be converted to binary using the pack() function, and the values > must > > be little-endian. > > Thanks, that looks good to me. > > > * class Random Randomizer > > > >> The same current PHP algorithm is used to generate random numbers within > > the specified range in Randomizer::getInt(). This is necessary for > backward > > compatibility. > >> It also provides a guarantee of consistency in the future. > > > > However, I understand that perhaps this fix will not satisfy your > request. > > I just do not have a good understanding of your intentions due to my poor > > English > > Don't worry. I understand that using a foreign language can sometimes be > hard - I am not a native speaker of English either and I suspect that > this also applies to many other participants. > > > I am considering the following possibilities regarding your intentions: > > > > 1. Should be stated in detail so that consistency of results is > maintained > > in the future. > > 2. Current PHP ranged random number generation algorithm has room for > > improvement and should be examined further > > 3. Consistency of results is difficult to maintain in the future (Maybe > > incorrect) > > > > In case 1, I think the following statement would satisfy the requirement. > > > >> The values generated by the seedable Engine guarantee future > > reproducibility of results, and the Randomizer uses the results to > process > > them, so if the results generated by the Engine are identical, the > > Randomizer's results will also be consistent. > > > > Although the consistency of the Randomizer results is not mentioned here, > > it would be a clear BC Break if the results were to change after the > > Randomizer is officially implemented, so I do think it is sufficient that > > an RFC be created and voted on as necessary. > > If I understand you correctly, then (1) is what I am looking for: It > should be spelled out explicitly what behavior the user may rely on and > what should be considered an implementation detail. > > Something like the following would work for me: > > > > The engines implement a specific well-defined random number generator. > For a given seed it is guaranteed that they return the same sequence as > the reference implementation. > > For the Randomizer it is considered a breaking change if the observable > behavior of the methods changes. For a given seeded engine and identical > method parameters the following must hold: > > - The number of calls to the Engine's ->generate() method remains the same. > - The return value remains the same for a given result retrieved from > ->generate(). > > Any changes to the Randomizer that violate these guarantees require a > separate RFC. > > > > > In case 2, I also thought about it along the way. Nikita also taught me > > about better algorithms: https://externals.io/message/115918#115982 > > > > But, I also think that the current PHP implementation is good enough, and > > there is no need to change it to the point of breaking compatibility. > > > > I think the current global scope MT implementation is very troublesome > for > > some use cases and should first be able to be drop-in-replaceable with > this > > implementation. > > > > In case 3, I think it is necessary to guarantee consistency at least once > > at the language level, even though this may change in the future. I have > > already indicated the need for this in the RFC. > > Can you comment on whether the Randomizer behaves identically on both 32 > and 64 bit PHP and also on little endian and big endian architectures? > As an example: Will ->getInt() calculate the same "uniform distribution" > on all bitnesses? If not I consider that a bug. > > The engines *should* behave identically, because of the "little endian > string" return value. > > If that's already the case then something like the following should be > added to the RFC guarantees: > > - The implementation will guarantee that the same results are returned > independent of the processor architecture (little endian / big endian) > and integer bit length's (3
Re: [PHP-DEV] [RFC] [Vote] Pre-vote announcement for Random Extension 5.x
2022年6月8日(水) 0:24 Tim Düsterhus : > Hi > > On 5/31/22 11:54, Go Kudo wrote: > > - More detailed description in RFC > > > > a) > > For the Random\Engine interface I suggest to clarify that the returned > bytestring will be interpreted as a little endian integer. > > b) > > I'm still missing an explanation of the guarantees the Randomizer > implementation will make (last bullet point in > https://externals.io/message/117295#117299). > > To me the guarantees is the most important thing about this RFC. As a > user of the API I need to know what of the behavior can and what cannot > change in future PHP versions. I don't really care whether the RNG is > PCG64 or some Xoshiro. Both are great. > > The Engine part is pretty solid: They implement a well-defined RNG and > any numbers are returned as little endian integers (see (a)). Either the > implementation is correct or it is not. This is not likely to change in > the future. > > However the Randomizer part is pretty undefined: As an example: > ->getInt() will return an integer uniformly selected from the given > range. But there's more than one way to perform this uniform selection. > Will the algorithm stay the safe in future PHP versions? There are more > examples in my previous emails. > > Best regards > Tim Düsterhus > Hi Tim. Thanks for the continued feedback! I have added the following regarding the points you pointed out. * interface Random\Engine > It has a single generate(): string method that generates random numbers as a binary string. This string must be non-empty and attempting to return an empty will result in a RuntimeException. > If you implement a random number generator in PHP, the generated numbers must be converted to binary using the pack() function, and the values must be little-endian. * class Random Randomizer > The same current PHP algorithm is used to generate random numbers within the specified range in Randomizer::getInt(). This is necessary for backward compatibility. > It also provides a guarantee of consistency in the future. However, I understand that perhaps this fix will not satisfy your request. I just do not have a good understanding of your intentions due to my poor English I am considering the following possibilities regarding your intentions: 1. Should be stated in detail so that consistency of results is maintained in the future. 2. Current PHP ranged random number generation algorithm has room for improvement and should be examined further 3. Consistency of results is difficult to maintain in the future (Maybe incorrect) In case 1, I think the following statement would satisfy the requirement. > The values generated by the seedable Engine guarantee future reproducibility of results, and the Randomizer uses the results to process them, so if the results generated by the Engine are identical, the Randomizer's results will also be consistent. Although the consistency of the Randomizer results is not mentioned here, it would be a clear BC Break if the results were to change after the Randomizer is officially implemented, so I do think it is sufficient that an RFC be created and voted on as necessary. In case 2, I also thought about it along the way. Nikita also taught me about better algorithms: https://externals.io/message/115918#115982 But, I also think that the current PHP implementation is good enough, and there is no need to change it to the point of breaking compatibility. I think the current global scope MT implementation is very troublesome for some use cases and should first be able to be drop-in-replaceable with this implementation. In case 3, I think it is necessary to guarantee consistency at least once at the language level, even though this may change in the future. I have already indicated the need for this in the RFC. Most of all, I do not believe you intend this to be the case. (And this sentence is not intended to offend you either. Please don't misunderstand me.) I would like to hear more about your opinion. Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Vote] Pre-vote announcement for Random Extension 5.x
2022年6月4日(土) 18:03 Côme Chilliet : > Le 31 mai 2022 11:54:18 GMT+02:00, Go Kudo a écrit : > >Hi internals. > > > >Although I have explained that due to the global turmoil I will delay > >voting on the RFC as long as possible, it is time to start voting on the > >RFC in order to get the implementation to full status by the PHP 8.2 > >Feature Freeze. > > > >I apologize for the delay in responding to the points you had already > >pointed out. It has been addressed as follows. > > > >- Random\Engine\PCG64::__construct() now takes an `int|string` $inc as its > >second argument > >- This makes it fully compatible with the PCG64 original > implementation > >- Fixed an algorithm implementation error in PCG64 > >- Fixed compatibility issues with PHP 8.2 in test cases > >- More detailed description in RFC > > > >Previous discussion thread: https://externals.io/message/117295 > > > >Voting will begin at 2022-06-14 00:00:00 (UTC). > > > >https://wiki.php.net/rfc/rng_extension > > > >Regards, > >Go Kudo > > The first section starts with "However", which sounds like an error from a > text reorganization? > Also: > "adding any randomization functions between the seeding the intended usage > would break the code. " -> missing "and" I suppose > "Even with JIT enabled, the native implementation is not far behind. " -> > it *is* far behind no? It's not even behind but ahead as I see it so maybe > I just misunderstand this comment. > > Côme > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > > The first section starts with "However", which sounds like an error from a text reorganization? > "adding any randomization functions between the seeding the intended usage would break the code. " -> missing "and" I suppose Thanks, I fixed it. > "Even with JIT enabled, the native implementation is not far behind. " -> it *is* far behind no? It's not even behind but ahead as I see it so maybe I just misunderstand this comment. Just wanted to explain that the native implementation is faster. (C > PHP) I changed it to a more clear form. (I was replying directly to you. sorry)
[PHP-DEV] [RFC] [Vote] Pre-vote announcement for Random Extension 5.x
Hi internals. Although I have explained that due to the global turmoil I will delay voting on the RFC as long as possible, it is time to start voting on the RFC in order to get the implementation to full status by the PHP 8.2 Feature Freeze. I apologize for the delay in responding to the points you had already pointed out. It has been addressed as follows. - Random\Engine\PCG64::__construct() now takes an `int|string` $inc as its second argument - This makes it fully compatible with the PCG64 original implementation - Fixed an algorithm implementation error in PCG64 - Fixed compatibility issues with PHP 8.2 in test cases - More detailed description in RFC Previous discussion thread: https://externals.io/message/117295 Voting will begin at 2022-06-14 00:00:00 (UTC). https://wiki.php.net/rfc/rng_extension Regards, Go Kudo
[PHP-DEV] LTO support
At present, PHP cannot be built using LTO (Link Time Optimization).
LTO is a compiler feature that can improve performance by optimizing at
link time.
Chromium is also using this feature.
https://gcc.gnu.org/onlinedocs/gccint/LTO-Overview.html
The reason for the LTO failure appears to be the Zend Engine's use of GCC's
global register variables. Conversely, it is otherwise compatible with LTO.
(except: opcache).
I modified toolchain too to enable and validate LTOs outside of Zend Engine
and OPcache:
https://github.com/zeriyoshi/php-src/commit/b25c237837fec2f82d268d8dbd45ec886baf474f
I tested the following compilation options:
```
gcc: (Debian 10.2.1-6) 10.2.1 20210110
CFLAGS="-fstack-protector-strong -fpic -fpie -O3 -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64"
CXXFLAGS="${CFLAGS}"
LDFLAGS="-Wl,-O1 -pie"
$ ./sapi/cli/php ./Zend/bench.php
simple 0.010
simplecall 0.005
simpleucall0.010
simpleudcall 0.013
mandel 0.052
mandel20.049
ackermann(7) 0.012
ary(5) 0.003
ary2(5)0.002
ary3(2000) 0.020
fibo(30) 0.035
hash1(5) 0.004
hash2(500) 0.004
heapsort(2)0.012
matrix(20) 0.012
nestedloop(12) 0.010
sieve(30) 0.006
strcat(20) 0.002
Total 0.261
```
vs
```
gcc: (Debian 10.2.1-6) 10.2.1 20210110
CFLAGS="-fstack-protector-strong -fpic -fpie -O3 -flto -fuse-linker-plugin
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64"
CXXFLAGS="${CFLAGS}"
LDFLAGS="-Wl,-O1 -pie -flto -fuse-linker-plugin"
$ ./sapi/cli/php ./Zend/bench.php
simple 0.006
simplecall 0.003
simpleucall0.010
simpleudcall 0.014
mandel 0.050
mandel20.049
ackermann(7) 0.012
ary(5) 0.003
ary2(5)0.002
ary3(2000) 0.020
fibo(30) 0.035
hash1(5) 0.004
hash2(500) 0.004
heapsort(2)0.012
matrix(20) 0.011
nestedloop(12) 0.010
sieve(30) 0.007
strcat(20) 0.002
Total 0.255
```
In fact, it does not seem to work very well. However, it may be effective
when large numbers of extensions are built into PHP.
Is anyone interested in supporting this?
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 5.x
2022年3月10日(木) 3:07 Larry Garfield : > On Wed, Mar 9, 2022, at 4:48 AM, Go Kudo wrote: > > Hello internals. > > > > Proposed RFCs and implementations have been reorganized. The main changes > > are as follows > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/8094 > > > > - Remove XorShift128Plus, Xoshiro256StarStar > > - To avoid confusion, Can be added if needed > > - All classes can now omit the seed argument > > - Seeding with CSPRNG > > - Throws RuntimeException if number generation fails > > > > The merge window to PHP 8.2 will remain open for some time to come. Due > to > > the global turmoil, I will try to delay the start of the RFC voting phase > > as long as possible. > > > > However, I believe that PHP RFCs tend to become more controversial once > the > > voting phase begins. > > > > Therefore, I would like to solicit feedback on this RFC, whatever it may > > be. (Just only positive or negative feedback would be okay) > > > > Regards, > > Go Kudo > > This looks good to me overall, although I have a few comments. > > * There's several places where a sentence or portion of a sentence repeats > itself. I assume this is just an editing error. > > * The list headers "implemented of" doesn't really make grammatical > sense. I think you mean "implemented by"? > > * The last section "PRNG Shootout" seems to imply that you're only > implementing one algorithm, but it doesn't say which one. The earlier > sections, though, show several algorithms that you are implementing. > Again, I assume this is an editing error from many revisions but it's > confusing as is. > > * If no engine is provided to the randomizer, what gets used? If the > default is listed somewhere in the RFC, I missed it. I'm assuming there > should be one, for usability, but changing it in the future would be an > RFC-level change (much like changing the defaults for password_hash()). > > * The example of using different algorithms in different environments > needs much higher billing. You're burying the lead there. The ability to > have a bad but predictable random seed for tests but then a CSPRNG for > production is *huge*. The approach is similar to the nearly-done PSR-20 > spec in FIG, which does essentially the same thing for the "current time" > routines, albeit in userspace. I would highlight this benefit way more, as > it's probably the biggest benefit from these changes that most developers > will see and has me more excited about this RFC than anything else in the > entire document. > > * How do the existing random functions interact with the new API? Do they > shift to be shells over the new classes with an internal shared global, or > something else? > > * A side effect of the current design is that SeedableEngine is just a > marker interface; there's no actual standardized way to set the seed. The > examples imply that passing it as a constructor is the recommended way (and > I agree it probably should be), but that's not explicit. I'm also unclear > then what value the marker interface is, then. Does the randomizer do > something different with a seedable engine? I'm not against the current > design, but its explanation/implications could be improved. > > Looking forward to this passing in a few weeks! > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Hi Larry, I have attempted to correct what you pointed out. However, I don't think the RFC is complete at this point. We will work on correcting this over the next few days. I'll answer a few questions. (These have been corrected in the RFC) > If no engine is provided to the randomizer, what gets used? If the default is listed somewhere in the RFC, I missed it. I'm assuming there should be one, for usability, but changing it in the future would be an RFC-level change (much like changing the defaults for password_hash()). As Tim has answered, Random\Engine\Secure is automatically generated and used. > The example of using different algorithms in different environments needs much higher billing This is a very important aspect, but also a side effect of the current problem solving. I am very concerned about how to appeal to this. For the time being, I have added a sentence to the Proposal. > How do the existing random functions interact with the new API As BC break is none, it does not affect existing APIs. The internal implementation will be changed. This is to avoid redundancy in implementation. https://github.com/php/php-src/pull/8094/files#diff-8ba10cd3f979cdab4491a2d02149ff10638b88854e79664748b3ab620fd3f1bfR255-R259 I am very concerned about how to make the RFC easier to read. I Will continue to work on improving it. Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 5.x
2022年3月10日(木) 3:57 Tim Düsterhus : > Hi > > On 3/9/22 11:48, Go Kudo wrote: > > Proposed RFCs and implementations have been reorganized. The main changes > > are as follows > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/8094 > > > > I've compared the RFC against the implementation once more: > > - The RFC should clarify that returning an empty string in > Engine::generate() is not allowed. > - The corresponding test > 'ext/random/tests/03_randomizer/user_unsafe.phpt' should verify not just > an exception is thrown, but also the exception message (I'll add a > review comment). > > - PCG64 effectively has 256 Bits of internal state (128 Bits for 's' and > 128 Bits for 'inc'), but it only accepts 128 Bits for 's'. 'inc' cannot > be provided by the user. The purpose of 'inc' is not entirely clear to > me, but the user likely should be able to specify it for full > reproducibility. > - The default initialization of PCG64 with a null seed will fill 'inc', > not 's'. This likely is a bug? At least it's inconsistent with the > previous point. > > - The RFC is missing an explanation of the guarantees the implementation > will (or will not) make. This is important for the user if they are > relying on reproducibility of the sequences and outputs. The more > guarantees we give, the less can be changed in the future. I've > previously mentioned that in the thread for '4.x': > https://externals.io/message/117026#117061 > > Best regards > Tim Düsterhus > Hi Tim Sorry, I must have missed some of your emails. We will make corrections to the areas you pointed out. I will reply to your email first as I am not familiar with English and it may take a little time. Thank you!
[PHP-DEV] [RFC] [Under Discussion] Random Extension 5.x
Hello internals. Proposed RFCs and implementations have been reorganized. The main changes are as follows https://wiki.php.net/rfc/rng_extension https://github.com/php/php-src/pull/8094 - Remove XorShift128Plus, Xoshiro256StarStar - To avoid confusion, Can be added if needed - All classes can now omit the seed argument - Seeding with CSPRNG - Throws RuntimeException if number generation fails The merge window to PHP 8.2 will remain open for some time to come. Due to the global turmoil, I will try to delay the start of the RFC voting phase as long as possible. However, I believe that PHP RFCs tend to become more controversial once the voting phase begins. Therefore, I would like to solicit feedback on this RFC, whatever it may be. (Just only positive or negative feedback would be okay) Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月21日(月) 21:44 Tim Düsterhus : > Hi > > On 2/21/22 03:57, Go Kudo wrote: > > I am sorry for the delay in replying. > > Don't worry, there was a weekend inbetween and I totally understand that > one wants to take their weekend off. > > > Thank you for the clear explanation. > > It is true that the RFC in its current form lacks explanation. I'll try > to > > fix this first. > > Sounds good. > > > Also, as I look into other languages' implementations, I see the need to > > add some RNGs such as PCG. I will update the RFC to include these. > > I suggest you avoid "feature creep" within the RFC. Additional engines > can be added easily later on if the need arises. But for now it's more > important to get a reliable basis that one can build onto. > > Having a choice of a multitude of different engine just distracts from > that goal and can be confusing for the user. With xoshiro256** there's a > very good choice that already is part of the RFC, no need to have > something else that might or might not be slightly better in some case. > > Best regards > Tim Düsterhus > Hi RFC has been updated. Is this up to the required standard? https://wiki.php.net/rfc/rng_extension I acknowledge that the previous RFC may have been difficult to discuss. If the problem has been solved, I would like to make another ML-wide announcement and wait for two weeks from there. I added PCG64 because according to the RNG experts, there seems to be a mild conflict between Xorshiro256 and PCG64. Also, as mentioned in the RFC, Rust and NumPy also implement PCG64. In order to verify the feasibility of PCG64, we created a PoC in C. So far, it seems to work fine. https://github.com/zeriyoshi/pcg64_example Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月18日(金) 19:46 Tim Düsterhus :
> Hi
>
> On 2/18/22 07:31, Go Kudo wrote:
> > I have been looking into output buffering, but don't know the right way
> to
> > do it. The buffering works fine if all RNG generation widths are static,
> > but if they are dynamic so complicated.
>
> I believe the primary issue here is that the engines are expected to
> return an uint64_t, instead of a buffer with raw bytes. This requires
> you to perform many conversions between the uint64 and the raw buffer:
>
> When calling Randomizer::getBytes() for a custom engine the following
> needs to happen:
>
> - The Engine returns a byte string.
> - This bytestring is then internally converted into an uint64_t.
> - Then calling Randomizer::getBytes() this uint64_t needs to be
> converted back to a bytestring.
>
> To avoid those conversations without sacrificing too much performance it
> might be possible to return a struct that contains a single 4 or 8-byte
> array:
>
> struct four_bytes {
> unsigned char val[4];
> };
>
> struct four_bytes r;
> r.val[0] = (result >> 0) & 0xff;
> r.val[1] = (result >> 8) & 0xff;
> r.val[2] = (result >> 16) & 0xff;
> r.val[3] = (result >> 24) & 0xff;
>
> return r;
>
> .val can be treated as a bytestring, but it does not require dynamic
> allocation. By doing that the internal engines (e.g. Xoshiro) would be
> consistent with the userland engines.
>
> > It is possible to solve this problem by allowing generate() itself to
> > specify the size it wants, but this would significantly slow down
> > performance.
>
> I don't think it's a good idea to add a size parameter to generate().
>
> > I've looked at the sample code, but do you really need support for
> > Randomizer? Engine::generate() can output dynamic binaries up to 64 bits.
> > You can use Engine directly, instead of Randomizer::getBytes().
> >
> > What exactly is the situation where buffering by Randomizer is needed?
>
> *I* don't need anything. I'm just trying to think of use-cases and
> edge-cases. Basically: What would a user attempt to do and what would
> their expectations be?
>
> I'm not saying that this buffering *must* be implemented, but this is
> something we need to think about. Because changing the behavior later is
> pretty much impossible, as users might rely on a specific behavior for
> their seeded sequences. The behavior might also need to be part of the
> documentation.
>
> Basically what we need to think about is what guarantees we give. As an
> example:
>
> 1. Calling Engine::generate() with the same seed results in the same
> sequence (This guarantee we give, and it is useful).
> 2. Calling Randomizer::getInt() with the same seeded engine results in
> the same numbers for the same parameters (I think this also is useful).
> 3. Calling Randomizer::getBytes() with the same seeded engine results in
> the same byte sequence (This is something we are currently discussing).
> 4. Calling Randomizer::getBytes() simply concatenates the raw bytes
> retrieved by the Engine (This ties into (3)).
> 5. Calling Randomizer::shuffleArray() with the same seeded engine
> results in the same result for the same string (This one is more
> debatable, because then we must maintain the exact same shuffleArray()
> implementation forever).
>
> All these guarantees should be properly documented within the RFC. The
> RFC template (https://wiki.php.net/rfc/template) says:
>
> > Remember that the RFC contents should be easily reusable in the PHP
> Documentation.
>
> So by thinking about this now and putting it in the RFC, the
> explanations can easily be copied into the documentation if the RFC
> passes the vote.
>
> One should not need to look into the implementation to understand how
> the Engines and the Randomizer is supposed to work.
>
> > Also worried that buffering will cut off random numbers at arbitrary
> sizes.
> > It may cause bias in the generated results.
> >
>
> If there's bias in specific bits or bytes of the generated number then
> getBytes(32) will already be biased even without buffering, as the raw
> bytes are what's of interest here. It does not matter if they are at the
> 1st or 4th position (for a 32-bit engine).
>
> Best regards
> Tim Düsterhus
>
Hi
I am sorry for the delay in replying.
Thank you for the clear explanation.
It is true that the RFC in its current form lacks explanation. I'll try to
fix this first.
Also, as I look into other languages' implementations, I see the need to
add some RNGs such as PCG. I will update the RFC to include these.
Here is a Rust example:
https://docs.rs/rand/latest/rand/
PCG:
https://www.pcg-random.org/index.html
Regards
Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月17日(木) 19:25 Tim Düsterhus :
> Hi
>
> On 2/17/22 08:37, Go Kudo wrote:
> > The following points have been fixed:
> >
> > - `nextByteSize(): int` has been removed from Random\Engine
> > - If the width of the RNG is statically defined, it will now be used
> > preferentially
> > - Added Xoshiro256StarStar
> > - Fixed an endianness issue
> >
> > And updated RFC
> >
> > https://wiki.php.net/rfc/rng_extension
> >
> > [...]
> > This seems to have solved the whole problem. How about it?
>
> Awesome, this is feeling much much better now. As you might've seen I've
> made some comments on GitHub regarding implementation bugs.
>
> I have two more conceptional questions:
>
> ---
>
> 1)
>
> However I believe you did not answer my question regarding the following
> and that's something that should be clear in the RFC and documentation:
>
>
> use Random\Engine\Xoshiro256StarStar;
> use Random\Randomizer;
>
> $g1 = new Xoshiro256StarStar(1);
> $g2 = clone $g1;
> $r1 = new Randomizer($g1);
> $r2 = new Randomizer($g2);
>
> var_dump(\bin2hex($r1->getBytes(8))); // string(16) "c510c70f6daff2b3"
> var_dump(\bin2hex($r2->getBytes(4) . $r2->getBytes(4))); //
> string(16) "c510c70fea4c3647"
>
>
> In this example I get 8 bytes from the randomizer. One time by getting
> all 8 bytes at once, the second time by getting 4 bytes and then another
> 4 bytes.
>
> I think that both lines should result in the same output, because in
> both cases I am getting 8 bytes, without any other operations that might
> affect the engine state in between. As a user I should not be required
> to know how the Randomizer works internally (by always getting 8 bytes
> from the engine and throwing away unused bytes).
>
> If you disagree and think this is the correct behavior then this should
> be documented accordingly in the RFC. If you agree, then this should be
> fixed and a testcase be added.
>
> ---
>
> 2)
>
> This ties into (1): Currently any additional bytes returned by the
> engine are silently ignored. Either the bytes should be processed in
> full, or an error emitted if the returned bytestring is too long.
>
> Consider the attached test case with a Sha1-based RNG. If I grab 20
> bytes (the length of a SHA-1 hash) from the Randomizer then the
> 'generate()' function will be called 3 times, despite it returning
> sufficient bytes on the first attempt. If I want to make sure that no
> bytes are wasted, then I need to implement a pretty complex construction
> (Sha1_2) to always return exactly 8 bytes.
>
> Best regards
> Tim Düsterhus
Hi
Hello.
I have been looking into output buffering, but don't know the right way to
do it. The buffering works fine if all RNG generation widths are static,
but if they are dynamic so complicated.
It is possible to solve this problem by allowing generate() itself to
specify the size it wants, but this would significantly slow down
performance.
I've looked at the sample code, but do you really need support for
Randomizer? Engine::generate() can output dynamic binaries up to 64 bits.
You can use Engine directly, instead of Randomizer::getBytes().
What exactly is the situation where buffering by Randomizer is needed?
Also worried that buffering will cut off random numbers at arbitrary sizes.
It may cause bias in the generated results.
If you don't want to waste the generated values, users can still implement
their own buffering structures into Random\Engine following.
```php
gen = $this->stream($seed);
$this->buffer = '';
}
private function stream($state)
{
while (true) {
$state = \sha1($state, true);
for ($i = 0; $i < \strlen($state); $i++) {
yield $state[$i];
}
}
}
public function generate(): string
{
echo __METHOD__, PHP_EOL;
$result = "";
for ($i = 0; $i < 8; $i++) {
$result .= $this->gen->current();
$this->gen->next();
}
return $result;
}
public function getBytes(int $length): string
{
$result = '';
while (\strlen($result) < $length) {
if ($this->buffer === '') {
$this->buffer = $this->generate();
}
$required = $length - \strlen($result);
$temp = \substr($this->buffer, 0, $required);
$this->buffer = \substr($this->buffer, $required);
$result .= $temp;
}
return $result;
}
}
```
Buffering mechanism that was implemented in a limited way has been reverted
recently.
Best Regards
Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月17日(木) 19:25 Tim Düsterhus : > Hi > > On 2/17/22 08:37, Go Kudo wrote: > > The following points have been fixed: > > > > - `nextByteSize(): int` has been removed from Random\Engine > > - If the width of the RNG is statically defined, it will now be used > > preferentially > > - Added Xoshiro256StarStar > > - Fixed an endianness issue > > > > And updated RFC > > > > https://wiki.php.net/rfc/rng_extension > > > > [...] > > This seems to have solved the whole problem. How about it? > > Awesome, this is feeling much much better now. As you might've seen I've > made some comments on GitHub regarding implementation bugs. > > I have two more conceptional questions: > > --- > > 1) > > However I believe you did not answer my question regarding the following > and that's something that should be clear in the RFC and documentation: > > > use Random\Engine\Xoshiro256StarStar; > use Random\Randomizer; > > $g1 = new Xoshiro256StarStar(1); > $g2 = clone $g1; > $r1 = new Randomizer($g1); > $r2 = new Randomizer($g2); > > var_dump(\bin2hex($r1->getBytes(8))); // string(16) "c510c70f6daff2b3" > var_dump(\bin2hex($r2->getBytes(4) . $r2->getBytes(4))); // > string(16) "c510c70fea4c3647" > > > In this example I get 8 bytes from the randomizer. One time by getting > all 8 bytes at once, the second time by getting 4 bytes and then another > 4 bytes. > > I think that both lines should result in the same output, because in > both cases I am getting 8 bytes, without any other operations that might > affect the engine state in between. As a user I should not be required > to know how the Randomizer works internally (by always getting 8 bytes > from the engine and throwing away unused bytes). > > If you disagree and think this is the correct behavior then this should > be documented accordingly in the RFC. If you agree, then this should be > fixed and a testcase be added. > > --- > > 2) > > This ties into (1): Currently any additional bytes returned by the > engine are silently ignored. Either the bytes should be processed in > full, or an error emitted if the returned bytestring is too long. > > Consider the attached test case with a Sha1-based RNG. If I grab 20 > bytes (the length of a SHA-1 hash) from the Randomizer then the > 'generate()' function will be called 3 times, despite it returning > sufficient bytes on the first attempt. If I want to make sure that no > bytes are wasted, then I need to implement a pretty complex construction > (Sha1_2) to always return exactly 8 bytes. > > Best regards > Tim Düsterhus Hi Tim Thanks for your continued help! As for your question, buffering the output can lead to counter-intuitive behavior in code like the following. ```php getBytes(2); // Generate a new 64 bits (to waste) $engine->generate(); // Retrieve 64 bits (first 48 bits from buffer, but last 16 bits newly generated) // numerical continuity will be lost. $str2 = $randomizer->getBytes(8); ``` However, the Engine will probably not be used by itself very often. I think buffering should be implemented, but I'd like to solicit opinions on this in the ML. I'll implement value buffering first. Regards Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月16日(水) 21:25 Tim Düsterhus :
> Hi
>
> On 2/16/22 12:39, Go Kudo wrote:
> >> Is the nextByteSize() method actually required? PHP strings already
> know
> > their own length.
> >
> > This is a convenience of the current implementation.
>
> You already said that you will think of some good ideas, but I'd like to
> be clear that the convenience of the internal implementation should not
> be something that affects the user-facing implementation.
>
> In fact with the current implementation I can trivially create a
> memory-unsafety bug:
>
>
> use Random\Engine;
> use Random\Randomizer;
>
> final class Bug implements Engine {
> public function generate(): string
> {
> return '';
> }
>
> public function nextByteSize(): int {
> return 7;
> }
> }
>
> $e = new Bug();
> $g = new Randomizer($e);
>
> var_dump(\bin2hex($g->getBytes(8)));
>
> Results in:
>
> > ==116755== Use of uninitialised value of size 8
> > ==116755==at 0x6180C8: php_bin2hex (string.c:111)
> > ==116755==by 0x6185D2: zif_bin2hex (string.c:220)
> > ==116755==by 0x79BDB4: ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER
> (zend_vm_execute.h:1312)
> > ==116755==by 0x8194F0: execute_ex (zend_vm_execute.h:55503)
> > ==116755==by 0x81ED86: zend_execute (zend_vm_execute.h:59858)
> > ==116755==by 0x75A923: zend_execute_scripts (zend.c:1744)
> > ==116755==by 0x69C8C4: php_execute_script (main.c:2535)
> > ==116755==by 0x8E0B19: do_cli (php_cli.c:965)
> > ==116755==by 0x8E1DF9: main (php_cli.c:1367)
> > ==116755==
> > ==116755== Use of uninitialised value of size 8
> > ==116755==at 0x618100: php_bin2hex (string.c:112)
> > ==116755==by 0x6185D2: zif_bin2hex (string.c:220)
> > ==116755==by 0x79BDB4: ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER
> (zend_vm_execute.h:1312)
> > ==116755==by 0x8194F0: execute_ex (zend_vm_execute.h:55503)
> > ==116755==by 0x81ED86: zend_execute (zend_vm_execute.h:59858)
> > ==116755==by 0x75A923: zend_execute_scripts (zend.c:1744)
> > ==116755==by 0x69C8C4: php_execute_script (main.c:2535)
> > ==116755==by 0x8E0B19: do_cli (php_cli.c:965)
> > ==116755==by 0x8E1DF9: main (php_cli.c:1367)
> > ==116755==
> > string(16) ""
>
> For userland implementations you really must derive the size from the
> returned bytestring. Otherwise it's easy for a developer to create an
> implementation where nextByteSize() and generate() disagree. Even if the
> memory-unsafety is fixed, this will result in frustration for the user.
>
> For native implementations you can keep some explicit width in the code,
> if that helps with performance.
>
> Best regards
> Tim Düsterhus
>
Hi Tim
The following points have been fixed:
- `nextByteSize(): int` has been removed from Random\Engine
- If the width of the RNG is statically defined, it will now be used
preferentially
- Added Xoshiro256StarStar
- Fixed an endianness issue
And updated RFC
https://wiki.php.net/rfc/rng_extension
I also had a PHP implementation of Xorshift128Plus on hand, so I included
it in the tests.
https://github.com/colopl/php-src/blob/upstream_rfc/scoped_rng_for_pr/ext/random/tests/engine/user_xorshift128plus.phpt
Xoshiro128PlusPlus has been dropped from the bundle due to width issues. If
necessary, it will be implemented as an extension to PECL.
However, it is built in as a test of the userland implementation
https://github.com/colopl/php-src/blob/upstream_rfc/scoped_rng_for_pr/ext/random/tests/engine/user_xoshiro128plusplus.phpt
This seems to have solved the whole problem. How about it?
Regards
Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月16日(水) 20:24 Tim Düsterhus : > Hi > > On 2/16/22 12:04, Go Kudo wrote: > > Thanks for the good idea. I changed the NumberGenerator to Engine and > > changed generate() to return a string as suggested. > > Thanks, I've already seen the updated PR and played around with it. This > feels much better now. > > As a test I implemented xoshiro128++ in pure userland (it being a 32 Bit > RNG avoids the signedness issues in PHP userland) and compared it > against the reference implementation in C. > > Find my implementations attached. Both versions give the same results > (little endian encoding): > > $ gcc xoshiro128pp.c ; ./a.out > fa3c872c > $ sapi/cli/php test_rng.php > string(8) "fa3c872c" > > > The main changes since last time are as follows: > > > > - The userland implementation can now specify the width of the random > > number sequence that can be generated > > - Random\Engine::nextByteSize() has been added > > Is the nextByteSize() method actually required? PHP strings already know > their own length. > > > - Random\Engine::generate() now returns a string > > I've looked into your C implementation and it appears it still is > affected by endianness issues. You can't simply memcpy the raw uintXX_t > bytes into the char array. I believe the following should do the trick > to for a consistent little endian encoding: > > bytes[0] = (generated >> 0) & 0xff; > bytes[1] = (generated >> 8) & 0xff; > bytes[2] = (generated >> 16) & 0xff; > bytes[3] = (generated >> 24) & 0xff; > > The choice of endianness is arbitrary, but it needs to be consistent for > every platform. > > Likewise when converting back to a number from bytes: > > number = (bytes[0] << 0) | (bytes[1] << 8) (bytes[2] << 16) | (bytes[3] > << 24); > > I believe the same issue exists when initializing the XorShift with a > string. > > > I have not yet come to a final conclusion on whether XorShift128Plus > should > > be switched to another RNG. For example, what about implementing > > XorShift128Plus, but adding Xoshiro256** as well? > > > > That would be fine for me as well. But it might make it harder for the > end user to choose an appropriate RNG. > > Best regards > Tim Düsterhus Thanks Tim I forgot about the endian problem. Will try to fix it. Thank you very much. > Is the nextByteSize() method actually required? PHP strings already know their own length. This is a convenience of the current implementation. The native implementation does not use strings for performance reasons. This is because there is no way to know the valid width information they generate. I'm trying to think of some good ideas. > That would be fine for me as well. OK, I'm going to add Xoshiro128++ and Xoshiro256**. In order to make PHP easier to maintain, I think it is best not to add more implementations at random, so I will not add any more.
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月15日(火) 22:09 Tim Düsterhus :
> Hi
>
> On 2/15/22 12:48, Go Kudo wrote:
> > At first, I updated the RFC to the latest status.
> >
> > https://wiki.php.net/rfc/rng_extension
>
> Thank you, the examples are useful.
>
> > I need some time to think about the current issue. I understand its
> > usefulness, but I feel uncomfortable with the fact that the
> NumberGenerator
> > generates a string.
>
> Would you feel more comfortable if the interface would be called
> \Random\Engine or \Random\Generator (i.e. leaving out the "Number" from
> the interface name)?
>
> Engine is the term used by C++:
> https://en.cppreference.com/w/cpp/numeric/random/mersenne_twister_engine
> Generator is the term used by Java:
>
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>
> --
>
> With the 'FixedForUnitTest' example you mentioned in the RFC: While for
> that specific implementation it appears pretty obvious that increasing
> numbers are generated, in practice:
>
> 1. This will result in inconsistent behavior based on the architecture.
> I can't test it due to the lack of the necessary architectures, but I
> believe the following to be accurate:
>
> $g = new FixedForUnitTest();
>
> $r = new Randomizer($g);
>
> // 0100 in 64 Bit little endian
> // 01000200 in 32 Bit little endian
> // 0001 in 64 Bit big endian
> var_dump(bin2hex($r->getBytes(8)));
>
> 2. This analogy completely breaks down for the 'shuffle' functions which
> call the generator internally an unspecified number of times:
>
> $g = new FixedForUnitTest();
>
> $r = new Randomizer($g);
>
> var_dump($r->shuffleString("abcdefghijklmnopqrstuvwxyz")); //
> wosqyrupatvxznmlkjihgfedcb
> var_dump($r->shuffleString("abcdefghijklmnopqrstuvwxyz")); //
> fwrtjndlsyvpzuhxbqomkigeca
>
> The resulting strings look somewhat ordered, but there is no clear
> pattern, despite the underlying generator being completely predictable!
>
> > I also wonder about the point of changing RNG to XorShift128Plus. There
> are
> > a number of derived implementations, which RNG do you think is more
> > suitable?
> >
>
> I'm not an expert in RNGs, but based off this page:
> https://prng.di.unimi.it/ and the linked papers it appears that
> xoshiro256** is the overall best choice if memory usage is not a concern.
>
> Best regards
> Tim Düsterhus
>
Hi Tim.
Thanks for the good idea. I changed the NumberGenerator to Engine and
changed generate() to return a string as suggested.
The main changes since last time are as follows:
- The userland implementation can now specify the width of the random
number sequence that can be generated
- Random\Engine::nextByteSize() has been added
- Random\Engine::generate() now returns a string
- Random\Randomizer::getInt() now accepts an empty argument (like mt_rand())
At the same time, I have updated the RFC.
https://wiki.php.net/rfc/rng_extension
I have not yet come to a final conclusion on whether XorShift128Plus should
be switched to another RNG. For example, what about implementing
XorShift128Plus, but adding Xoshiro256** as well?
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月15日(火) 19:03 Tim Düsterhus :
> Hi
>
> On 2/15/22 04:58, Go Kudo wrote:
> >> Regarding "unintuitive": I disagree. I find it unintuitive that there
> are
> > some RNG sequences that I can't access when providing a seed.
> >
> > This is also the case for RNG implementations in many other languages.
> For
> > example, Java also uses long (64-bit) as the seed value of the argument
> for
> > Math.
> >
> >
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Random.html#%3Cinit%3E(long)
>
> java.util.Random is a LCG with only 48 Bits of state. A single 64-bit
> signed long is sufficient to represent the state.
>
> > On the other hand, some languages have access to the complete internal
> > state. Python, for example, accepts bytes or bytearrays.
> >
> > https://docs.python.org/3/library/random.html#random.seed
> >
> > However, making strings available in PHP may lead to incorrect usage.
> >
> > I think we can safely do this by making the seed argument accept both int
> > and string, and only using it as the internal state if string is
> specified
> > and it's 128-bits long.
>
> That's a solution that would work for me.
>
> >> 1. Would you expect those two 'var_dump' calls to result in the same
> > output?
> >
> > Added __debugInfo() magic method supports.
> >
> >
> https://github.com/php/php-src/pull/8094/commits/78efd2bd1e0ac5db48c272b364a615a5611e8caa
>
> Don't forget to update the RFC accordingly. It would probably be helpful
> if you would put the full class stubs into the RFC. I find that easier
> to understand than a list of methods.
>
> >> generate() should return raw bytes instead of a number (as I suggested
> > before).
> >
> > I don't think this is a very good idea.
> >
> > The RNG is a random number generator and should probably not be
> generating
> > strings.
>
> I'd say that the 'number' part in RNG is not technically accurate. All
> RNGs are effectively generators for a random sequence of bits. The
> number part is just an interpretation of those random sequence of bits
> (e.g. 64 of them).
>
> > Of course, I am aware that strings represent binary sequences in PHP.
> > However, this is not user-friendly.
> >
> > The generation of a binary string is a barrier when trying to implement
> > some kind of operation using numeric computation.
>
> I believe the average user of the RNG API would use the Randomizer
> class, instead of the raw generators, thus they would not come in
> contact with the raw bytes coming from the generator.
>
> However by getting PHP integers out of the generator it is much harder
> for me to process the raw bits and bytes, if that's something I need for
> my use case.
>
> As an example if I want to implement the following in userland. Then
> with getting raw bytes:
> - For Randomizer::getBytes() I can just concatenate the raw bytes.
> - For a random uint16BE I can grab 2 bytes and call unpack('n', $bytes)
>
> If I get random 64 Bit integers then:
> - For Randomizer::getBytes() I need to use pack and I'm not even sure,
> whether I need to use 'q', 'Q', 'J', 'P' to receive an unbiased result.
> - For uint16BE I can use "& 0x", but would waste 48 Bits, unless I
> also perform bit shifting to access the other bytes. But then there's
> also the same signedness issue.
>
> Interpreting numbers as bytes and vice versa in C / C++ is very easy.
> However in PHP userland I believe the bytes -> numbers direction is
> easy-ish. The numbers -> bytes direction is full of edge cases.
>
> > If you want to deal with the problem of generated size, it would be more
> > appropriate to define a method such as getGenerateSize() in the
> interface.
> > Even in this case, generation widths greater than PHP_INT_SIZE cannot be
> > supported, but generation widths greater than 64-bit are not very useful
> in
> > the first place.
> >
> >> The 'Randomizer' object should buffer unused bytes internally and only
> > call generate() if the internal buffer is drained.
> >
> > Likewise, I think this is not a good idea. Buffering reintroduces the
> > problem of complex state management, which has been made so easy. The
> user
> > will always have to worry about the buffering size of the Randomizer.
>
> Unfortunately you did not answer the primary question. The ones you
> answered were just follow-up conclusions from the answer I would give:
>
> var_
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
idths greater than PHP_INT_SIZE cannot be supported, but generation widths greater than 64-bit are not very useful in the first place. > The 'Randomizer' object should buffer unused bytes internally and only call generate() if the internal buffer is drained. Likewise, I think this is not a good idea. Buffering reintroduces the problem of complex state management, which has been made so easy. The user will always have to worry about the buffering size of the Randomizer. > Why xorshift instead of xoshiro / xoroshiro? The XorShift128Plus algorithm is still in use in major browsers and is dead in a good way. Also, in our local testing, SplitMix64 + XorShift128Plus performed well in terms of performance and random number quality, so I don't think it is necessary to choose a different algorithm. If this RFC passes, it will be easier to add algorithms in the future. If a new algorithm is needed, it can be implemented immediately. Regards, Go Kudo
Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
2022年2月14日(月) 20:40 Tim Düsterhus : > Hi > > On 2/14/22 12:11, Go Kudo wrote: > > The refreshed RFC and implementation are available at the following URL: > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/8094 > > > > If there are no specific comments, I would like to start voting as soon > as > > the two-week pre-announcement phase is over. > > 1) XorShift128+ has a 128 Bit internal state, but takes an integer seed > within the constructor. Thus only 64 Bits of seed can be provided. > > Maybe the seed parameter should be a 16-byte string instead? > Initializing the generator with a completely random seed would then be: > > new XorShift128Plus(\random_bytes(16)); > > instead of the much more complicated: > > new XorShift128Plus(\random_int(\PHP_INT_MIN, \PHP_INT_MAX)); > > Perhaps the following API would be even clearer: > > XorShift128Plus::fromSeed(\random_bytes(16)); > XorShift128Plus::fromGenerator(new Secure()); // Takes 16 bytes from the > given generator. > > 2) I would adjust the 'Randomizer' to use the 'Secure' generator as a > safe default. If absolute performance or a reproducible sequence is > required then one can use a custom generator, but the default will be > the secure CSPRNG, making it harder to misuse. > > 3) The RFC is inconsistent in the example code. Is it 'stringShuffle' or > 'shuffleString'? > > 4) The RFC should document the 'NumberGenerator' interface. Specifically > I'm interested in the return type of the 'generate' method. Does it > return bytes or integers? Is it legal to implement the interface in > userland code? > > Best regards > Tim Düsterhus > Hi > 1) XorShift128+ has a 128 Bit internal state, but takes an integer seed within the constructor. Thus only 64 Bits of seed can be provided. This is for convenience. Other software that uses XorShift128+, such as Chromium (V8), also uses a 64-bit value for the initial seed value. I think that 128-bit value seeding with strings is unintuitive and not very good for performance. https://chromium.googlesource.com/v8/v8/+/refs/heads/main/src/base/utils/random-number-generator.h > 2) I would adjust the 'Randomizer' to use the 'Secure' generator as a safe default. If absolute performance or a reproducible sequence is required then one can use a custom generator, but the default will be the secure CSPRNG, making it harder to misuse. Certainly, this may be appropriate. But, in this case, the Randomizer generated with the default parameters will not be serializable. Is that acceptable? Personally, I think this is a good change. > 3) The RFC is inconsistent in the example code. Is it 'stringShuffle' or 'shuffleString'? RFC Fixed :) > 4) The RFC should document the 'NumberGenerator' interface. Specifically I'm interested in the return type of the 'generate' method. Does it return bytes or integers? Is it legal to implement the interface in userland code? It returns an int. Also, as pointed out in GH, the generated value is implicitly treated as the size equivalent of PHP_INT_SIZE (zend_long) on the environment. This means that it is not possible to implement the userland Mersenne twister (32-bit) in a 64-bit environment. https://github.com/php/php-src/pull/8094#pullrequestreview-881660425
[PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
It's been a while Internals. I'm Go Kudo. First of all, I would like to apologize for leaving my previous RFC, object scoped RNG, and the preliminary RFC, split random extension, without any progress. The implementation of these RFCs was not sophisticated and failed to be tested for a long time. In particular, we could not find the time to work on them on Windows, and they were abandoned. You may have noticed that my email address has changed. This is because I have been assigned to work on this RFC and implementation as part of my company. This has given me the equipment and time to debug in a variety of environments. Some people may have a negative impression about this change, but don't worry. The RFC is still in the form of a feature proposal, and all rights are reserved by the PHP Group. Before we get into the RFC, let me explain again how this RFC came to be proposed. First, I started to implement an object-level scoped RNG in PHP to solve the current issue. This is still available in PECL today. https://pecl.php.net/package/orng https://github.com/zeriyoshi/php-ext-orng Later, I found in the PHP Internals ML logs that an implementation of RNG with object-level scoping had been considered before, and was well received at the time. However, it seems that the actual implementation was not done at that time. https://externals.io/message/98021#98130 After that, I started the first RFC and Vote. The result was a Decline, but that was mainly due in part to the strange APIs. https://wiki.php.net/rfc/object_scope_prng The refreshed RFC and implementation are available at the following URL: https://wiki.php.net/rfc/rng_extension https://github.com/php/php-src/pull/8094 If there are no specific comments, I would like to start voting as soon as the two-week pre-announcement phase is over. Regards. Go Kudo
Re: [PHP-DEV] [RFC] Random Extension 3.0
The RFCs are in limbo, but we are currently thinking of including the following RNGs in the proposal - XorShift128Plus - MT19937 (for compatibility) - MT19937_64 (for more entropy and wider range) While it is clear that MT19937 is simply MersenneTwister, it is not accurate since there is MT19937_64. Also, currently, the constant name that can be passed to mt_srand() is MT_RAND_MT19937, so I think it is consistent to use MT19937. If you mean that it should be MT19937 instead of MT19937_32, then I think you are right. Please let me know your opinion. 2021年10月7日(木) 21:56 Kamil Tekiela : > Please don't add more answers to the class name. There is already going to > be a backlash if we name it "MT19973" instead of "MersenneTwister" >
Re: [PHP-DEV] [Pre-Vote Announcement] Move RNG functions Random Extension
2021年9月22日(水) 21:01 Aleksander Machniak : > On 22.09.2021 13:21, Go Kudo wrote: > > The voting phase for the following RFCs will begin as soon as two weeks > > have passed. > > The RFC title is not correct English (missing "to" or "into"). The first > paragraph in Introduction is also hard to read. > > -- > Aleksander Machniak > Kolab Groupware Developer[https://kolab.org] > Roundcube Webmail Developer [https://roundcube.net] > > PGP: 19359DC1 # Blog: https://kolabian.wordpress.com > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > > Sorry. I'm not native English speaker and I am not good at English. I have improved the title first. I don't know how to fix the first chapter, so I left it as it is. If this causes problems in the voting phase of the RFC, I will try to fix it further, how about that?
Re: [PHP-DEV] [RFC] Random Extension 3.0
2021年9月23日(木) 2:05 Dan Ackroyd : > Go Kudo wrote: > > Dan Ackroyd wrote: > >> you can _simply_ include ext/random/random.h." sounds pretty > >> dismissive of causing possibly unneeded work for downstream projects. > > > > The point I was trying to make was that while BC Breaks do occur, they > are very easy to solve. > > > I've found it useful to think about "value = benefit minus cost". > > I was calling out that it seems to be that this is a change you want > to do for aesthetic reasons and are justifying it by trivialising the > work involved. i.e. it has a tiny benefit that only has a positive > value if the cost is also tiny. > > Although the change is simple: > > * Any code base that includes standard/php_rand.h won't compile, and > people will have to find out what changed. Even though the fix may be > easy to implement, each of those people will have to figure it out for > themselves. > > * quite a few people will have to learn (or relearn) how to use #if to > compare PHP version to include either standard/php_rand.h or > random/php_random.h if they want their code to work on more than 1 PHP > version. > > * it makes more work for downstream distributors of PHP, as their > patches (including security patches) will need to be moved around. > > If there's a good reason to move stuff around, then fine, but avoiding > creating extra work downstream is worth doing. And possibly is a hot > topic right now. > > You could probably avoid a lot of downstream work by leaving a stub > file at standard/php_rand.h that includes random/php_random.h. > > cheers > Dan > Ack > > btw, if you could please avoid top-posting, that would be appreciated: > https://github.com/php/php-src/blob/master/docs/mailinglist-rules.md > > "Do not top post. Place your answer underneath anyone you wish to > quote and remove any previous comment that is not relevant to your > post." > I apologize for not knowing the rules of ML. Is this correct? It is true that it is better to maintain compatibility that can be maintained. However, I think this should be sorted out sometime. Perhaps it will be in PHP 9.0. First of all, I would like to change these RFC and implementations. https://wiki.php.net/rfc/random_ext https://wiki.php.net/rfc/rng_extension Thank you.
Re: [PHP-DEV] [RFC] Random Extension 3.0
I had misunderstood this for a long time. Now I understand.
This is certainly something to think about.
However, I think we also need to consider compatibility. How about
something like the following?
```php
:
> On Thu, Sep 9, 2021 at 6:31 AM Go Kudo wrote:
>
>> Hi Nikita, sorry for the late reply.
>>
>> This is a difficult problem. For now, MT19937 is left for compatibility.
>> In other words, if you don't need compatibility, there is no benefit to
>> using it.
>>
>> What about implementing both a new MT and a compatible MT? A compatible
>> MT would have the following signature
>>
>> ```php
>>
>> use Random\NumberGenerator\Numbergenerator;
>>
>> /* for legacy compatibility */
>> class MT19937 implements NumberGenerator
>> {
>> public function __construct(int $mode = MT_RAND_MT19937, int $seed) {}
>> }
>>
>> /* a new implementation */
>> class MersenneTwister implements NumberGenerator
>> {
>> public function __construct(int $seed) {}
>> }
>> ```
>>
>> I had originally removed the MT_RAND_PHP implementation on the grounds
>> that legacy implementations should not be retained, but if the regular
>> Mersenne twister is to be retained for compatibility, I think it should be
>> as well.
>>
>> Also, maybe we should opt for a more SIMD friendly RNG.
>>
>
> To clarify, what I had in mind here is not the MT generator itself, but
> the scaling done in Random::getInt(). This scaling is independent of the
> used source. So while the raw numbers generated by
> Random\NumberGenerator\MT19937 would be the same as before, the result
> produced by Random::getInt() with this source wouldn't be.
>
> Regards,
> Nikita
>
>
>> 2021年9月7日(火) 17:28 Nikita Popov :
>>
>>> On Thu, Sep 2, 2021 at 5:10 PM Go Kudo wrote:
>>>
>>>> Hi Internals.
>>>>
>>>> Expanded from the previous RFC and changed it to an RFC that organizes
>>>> the
>>>> whole PHP random number generator. Also, the target version has been
>>>> changed to 8.2.
>>>>
>>>> https://wiki.php.net/rfc/rng_extension
>>>> https://github.com/php/php-src/pull/7453
>>>>
>>>> Hopefully you will get some good responses.
>>>>
>>>
>>> This RFC currently tries to keep some semblance of compatibility with
>>> the existing mt_rand() algorithm by retaining the same implementation for
>>> mapping the raw random number stream into a range. However, the algorithm
>>> we use for that is not exactly state of the art and requires two full-width
>>> divisions at minimum. There are more efficient scaling algorithms based on
>>> fixed-point multiplication, which are "nearly divisionless" (
>>> https://arxiv.org/pdf/1805.10941.pdf). The variant at
>>> https://github.com/apple/swift/pull/39143 is entirely divisionless.
>>>
>>> Doing this would improve performance (though I'm not sure by how much --
>>> maybe once we add up our call overhead, it isn't important anymore?) but it
>>> would provide a different sequence than mt_rand(). Something we might want
>>> to consider.
>>>
>>> Regards,
>>> Nikita
>>>
>>
[PHP-DEV] [Pre-Vote Announcement] Move RNG functions Random Extension
The voting phase for the following RFCs will begin as soon as two weeks have passed. https://externals.io/message/115975 I don't see any particular discussion, so I'm contacting you just in case. Best regards. Go Kudo
Re: [PHP-DEV] [RFC] Random Extension 3.0
Hi Nikita, sorry for the late reply.
This is a difficult problem. For now, MT19937 is left for compatibility. In
other words, if you don't need compatibility, there is no benefit to using
it.
What about implementing both a new MT and a compatible MT? A compatible MT
would have the following signature
```php
use Random\NumberGenerator\Numbergenerator;
/* for legacy compatibility */
class MT19937 implements NumberGenerator
{
public function __construct(int $mode = MT_RAND_MT19937, int $seed) {}
}
/* a new implementation */
class MersenneTwister implements NumberGenerator
{
public function __construct(int $seed) {}
}
```
I had originally removed the MT_RAND_PHP implementation on the grounds that
legacy implementations should not be retained, but if the regular Mersenne
twister is to be retained for compatibility, I think it should be as well.
Also, maybe we should opt for a more SIMD friendly RNG.
Regards,
Go Kudo
2021年9月7日(火) 17:28 Nikita Popov :
> On Thu, Sep 2, 2021 at 5:10 PM Go Kudo wrote:
>
>> Hi Internals.
>>
>> Expanded from the previous RFC and changed it to an RFC that organizes the
>> whole PHP random number generator. Also, the target version has been
>> changed to 8.2.
>>
>> https://wiki.php.net/rfc/rng_extension
>> https://github.com/php/php-src/pull/7453
>>
>> Hopefully you will get some good responses.
>>
>
> This RFC currently tries to keep some semblance of compatibility with the
> existing mt_rand() algorithm by retaining the same implementation for
> mapping the raw random number stream into a range. However, the algorithm
> we use for that is not exactly state of the art and requires two full-width
> divisions at minimum. There are more efficient scaling algorithms based on
> fixed-point multiplication, which are "nearly divisionless" (
> https://arxiv.org/pdf/1805.10941.pdf). The variant at
> https://github.com/apple/swift/pull/39143 is entirely divisionless.
>
> Doing this would improve performance (though I'm not sure by how much --
> maybe once we add up our call overhead, it isn't important anymore?) but it
> would provide a different sequence than mt_rand(). Something we might want
> to consider.
>
> Regards,
> Nikita
>
Re: [PHP-DEV] Re: [RFC] Random Extension 3.0
Nikita, Thanks for the clarification. That's exactly what I intended to do. > can you update the RFC with more information about this functionality? I understand. Organize them and reconstruct the RFC text. I've also created an RFC about moving RNG-related functions from ext/standard to ext/random. I would appreciate it if you could check it out. https://wiki.php.net/rfc/random_ext Regards, Go Kudo 2021年9月7日(火) 2:24 Ben Ramsey : > Nikita Popov wrote on 9/6/21 03:06: > > On Sun, Sep 5, 2021 at 7:40 PM Ben Ramsey wrote: > > > >> Go Kudo wrote on 9/4/21 23:00: > >>> Indeed, it may be true that these suggestions should not be made all at > >>> once. If necessary, I would like to propose to organize the RNG > >>> implementation first. > >>> > >>> Do we still need an RFC in this case? I'm not sure I'm not fully > >> understand > >>> the criteria for an RFC. Does this internal API change count as a BC > >> Break > >>> that requires an RFC? > >> > >> Yes, since re-organizing the RNG implementation is a major language > >> change that affects extensions and other downstream projects, this > >> requires an RFC. > >> > >>>> Personally, I don't see the benefit of including this OOP API in the > >> core. > >>> > >>> On the other hand, can you tell me why you thought it was unnecessary? > >>> Was my example unrealistic? > >> > >> You also said, in reply to Dan Ackroyd: > >> > >>> Either way, I'll try to separate these suggestions if necessary, but if > >> we > >>> were to try to provide an OOP API without BC Break, what do you think > >> would > >>> be the ideal form? > >> > >> The OOP API appears to be a wrapper around the RNG functions. The only > >> thing it gains by being in the core is widespread use, but it loses a > >> lot of flexibility and maintainability, since the API will be tied to > >> PHP release cycles. > >> > >> By doing this as an OOP wrapper in userland code, you're not restricting > >> yourself to release only when PHP releases, and you can work with the > >> community (e.g., the PHP-FIG) to develop interfaces that other projects > >> might use so they can make use of their own RNGs, etc. > >> > > > > The OO API is not just a wrapper around the RNG functions -- it provides > > new functionality that cannot be efficiently implemented in userland > code. > > This is for two reasons: > > > > 1. It provides independent randomness streams, which means it's not > > possible to reuse existing functionality like mt_rand() for this purpose, > > which only provides a single global random number stream. You would have > to > > reimplement the random number generator in userland. While this is > > possible, it will generally not be efficient, because PHP does not have > > native support for unsigned modular arithmetic, which is what random > number > > generators generally need. > > > > 2. It allows using functions like shuffle() and array_rand() with an > > independent randomness stream. These functions cannot be efficiently > > implemented in userland, because userland does not have random access to > > arrays. Internals can generate a random number and use it to pick the key > > at that position, which is O(1). Userland would have to call array_keys() > > first to allow random access on keys, which is O(n). > > > > Which is why I think this is a good addition to php-src. There's three > good > > reasons to include functionality in php-src (performance, ubiquity and > FFI) > > and this falls squarely into the first category. And now that we have > > fibers and need to worry more about multiple independent execution > streams, > > we also need to worry about multiple independent randomness streams. > > > > Regards, > > Nikita > > > > Thanks for the clarification, Nikita. > > The RFC says, "The Random class is a utility class that provides > functionality using random numbers." This led me to think it was a > wrapper that did not introduce new functionality. > > I can't find any discussion in the RFC on the independent randomness > streams provided by the OOP API. Go Kudo, can you update the RFC with > more information about this functionality? > > Cheers, > Ben > >
[PHP-DEV] [RFC] Move RNG functions ext/random
Hi internals. As a result of the exchange in the following thread, it seems appropriate to separate the proposals. In line with this, we have created a proposal to first clean up the existing RNG-related functionality and move it from ext/standard to ext/random. https://wiki.php.net/rfc/random_ext Hopefully this will get a good response. Regards, Go Kudo
Re: [PHP-DEV] [RFC] Random Extension 3.0
Thanks marc. > "shuffleString()" works on byte level and so it should be "shuffleBytes()". Hmm. This may be a difficult problem related to the PHP language specification. As with `str_shuffle()`, most people will probably use it to shuffle strings. People who want to shuffle binaries are likely to understand that in PHP it is synonymous with `shuffleBytes()`. > Why are there no default values for min/max on "getInt()" - It seems unnecessary to me to pass min/max arguments My apologies. I completely forgot that I needed this. Add this along with a fix for the RNG\NumberGenerator implementation. Regards, Go Kudo 2021年9月5日(日) 5:57 Marc : > > On 9/2/21 5:10 PM, Go Kudo wrote: > > Hi Internals. > > > > Expanded from the previous RFC and changed it to an RFC that organizes > the > > whole PHP random number generator. Also, the target version has been > > changed to 8.2. > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/7453 > > > > Hopefully you will get some good responses. > > For me (user land developer with no voting power) your RFC looks pretty :) > > Beside the abstract vs interface question I have some other notes: > > On the one hand you define "getBytes()" which returns a string and on > the other hand you define "shuffleString()" - is the first one a binary > string and the other a charset dependent string? I guess here > "shuffleString()" works on byte level and so it should be "shuffleBytes()". > > Why are there no default values for min/max on "getInt()" - It seems > unnecessary to me to pass min/max arguments if you are just interested > in a random integer or passing max as well if you are interested in a > positive integer only. > > Thanks for the nice RFC! > > > Regards, > > Go Kudo > Marc >
Re: [PHP-DEV] [RFC] Random Extension 3.0
This may not have been the best way to put it. The point I was trying to make was that while BC Breaks do occur, they are very easy to solve. The impact on downstream projects will be significant, and there may be many extensions affected, since this change concerns a frequently used RNG feature. However, in other words, it may need to be sorted out as soon as possible. However, I also understand the theory that this should be done in conjunction with a major language change. Perhaps I should have suggested this for PHP 8.0. Either way, I'll try to separate these suggestions if necessary, but if we were to try to provide an OOP API without BC Break, what do you think would be the ideal form? Regards, Go Kudo 2021年9月5日(日) 4:24 Dan Ackroyd : > On Fri, 3 Sept 2021 at 18:41, Go Kudo wrote: > > > > Nikita wrote: > >> this one also moves all of the existing RNG-related functionality > >> from ext/standard to ext/random. > > > > There are several reasons for this. > > > > - The `random` extension name is already used by ext/standard and may > > interfere with the build system. > > - Due to the namespace rules for new extensions, it is inappropriate to > > rename an extension. > > - Due to history, the rand / mt_rand dependency is tricky and I thought > it > > was time to sort it out. > > - I don't think it's a good idea to have multiple header files for a > single > > domain feature. > > Have you considered how much work that is going to incur on downstream > projects? > > If there's a good reason for moving stuff around then it can be > appropriate, but the RFC phrasing of: "At the same time, the > following internal APIs will also be relocated. If you want to use > them, you can _simply_ include ext/random/random.h." sounds pretty > dismissive of causing possibly unneeded work for downstream projects. > > cheers > Dan > Ack >
Re: [PHP-DEV] Re: [RFC] Random Extension 3.0
Indeed, it may be true that these suggestions should not be made all at once. If necessary, I would like to propose to organize the RNG implementation first. Do we still need an RFC in this case? I'm not sure I'm not fully understand the criteria for an RFC. Does this internal API change count as a BC Break that requires an RFC? > Personally, I don't see the benefit of including this OOP API in the core. On the other hand, can you tell me why you thought it was unnecessary? Was my example unrealistic? I'm sorry if my English is not good and my writing seems offensive. I have no intention to do so. Regards, Go Kudo 2021年9月5日(日) 1:12 Ben Ramsey : > Go Kudo wrote on 9/2/21 10:10: > > Hi Internals. > > > > Expanded from the previous RFC and changed it to an RFC that organizes > the > > whole PHP random number generator. Also, the target version has been > > changed to 8.2. > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/7453 > > > > Hopefully you will get some good responses. > > > > Regards, > > Go Kudo > > > > > This proposal seems like two different proposals to me: > > - The first consolidates and cleans up RNG functions for internals > - The second exposes an OOP API for working with RNGs > > Personally, I don't see the benefit of including this OOP API in the > core. I don't think it provides any benefits over similar abstractions > in userland, and in fact, I think this kind of API is best suited for > iteration in userland. > > Cheers, > Ben > >
Re: [PHP-DEV] [RFC] Random Extension 3.0
Thanks nikita. > ext/standard to ext/random. Why does it do this? There are several reasons for this. - The `random` extension name is already used by ext/standard and may interfere with the build system. - Due to the namespace rules for new extensions, it is inappropriate to rename an extension. - Due to history, the rand / mt_rand dependency is tricky and I thought it was time to sort it out. - I don't think it's a good idea to have multiple header files for a single domain feature. - As stated in the Future Scope, I wanted to be in a position to eliminate module global random number state in the future. - The existing implementation and the object implementation share most of the same logic. It was necessary to merge them into one to make them common. I certainly didn't want to require a workaround for PHP 8.2 or later for extensions that depended on ext/standard respectively, but I decided that I had no choice for the above reasons. > just don't make it a requirement. I would like to modify the design. > I don't think it makes sense to switch these functions to use cryptographic randomness by default... While these may not need to be cryptographically secure, I don't think they need to be state-dependent as well. Probably no one would look at the function name and think that it depends on the module global state. php_random_int() doesn't have any state anywhere (except OS) and can generate the requested random number. I don't see any problem with these functions using php_random_int() as a random number source. Regards, Go Kudo 2021年9月4日(土) 1:02 Nikita Popov : > On Thu, Sep 2, 2021 at 5:10 PM Go Kudo wrote: > >> Hi Internals. >> >> Expanded from the previous RFC and changed it to an RFC that organizes the >> whole PHP random number generator. Also, the target version has been >> changed to 8.2. >> >> https://wiki.php.net/rfc/rng_extension >> https://github.com/php/php-src/pull/7453 >> >> Hopefully you will get some good responses. >> > > This looks pretty nice to me. A few bits of feedback: > > * Unlike the previous versions of the RFC, this one also moves all of the > existing RNG-related functionality from ext/standard to ext/random. Why > does it do this? This doesn't really seem related to the problem this RFC > is solving. > > * Regarding the abstract class Random\NumberGenerator: You currently need > the abstract class, because php_random_ng is tied to the object. An > alternative design that would allow Random\NumberGenerator to be an > interface would be to make it independent of the object, such that the > structure can be allocated in the Random constructor for userland > implementations. Of course, internal implementations could still embed it > as part of their objects -- just don't make it a requirement. > > * The future scope mentions: "Changes random source to php_random_int() a > shuffle(), str_shuffle(), and array_rand()". I don't think it makes sense > to switch these functions to use cryptographic randomness by default... > > Regards, > Nikita >
Re: [PHP-DEV] [RFC] Random Extension 3.0
> I'm still unclear how I'd write my own NumberGenerator right now. I mean, I can extend the class, but I don't have a sense for what I'd do with it for non-testing/trivial implementations. You say it's using an internal function to generate numbers, but in user space what would I do with that? And when/why would I? For example, I will create an application that does a lottery. I need to estimate the load and test it. If I actually use random numbers to draw the lots, the test results will be different each time. It is true that this example can be solved by devising a better implementation of the userland. If this is the case, you may be wondering why we implement it this way. Currently, there are several implementations of random number generators written in PHP. https://github.com/savvot/random Although not in this library, I use an in-house interface that is completely replaceable with random_int() for load testing in my production brother. Also, JIT was implemented in PHP 8.0. which has the potential to make it possible to write in PHP what would otherwise have to be implemented in C due to execution speed issues. In fact, the random number generator written in PHP for my tests shows a significant performance improvement when JIT is enabled. The ability to have a workable random number implementation in userland should be useful for future extensions. > Is the intent that I should stop using random_int()? No, it's not. random_int() is a safe and easy CSPRNG, and is recommended for future use. The advantage of using Random\NumberGenerator\Secure is that it can shuffle strings/arrays using the same random number source as random_int(). Something that is not included in this RFC but that I would like to deprecate in the future is the mt_srand (srand) function. These have internal state and are very harmful for extensions such as Swoole: Also, shuffle() and str_shuffle() are very good functions, but I don't think they should use the random number source generated by mt_srand. The user may unintentionally use an unsecured random number. I apologize for the difficulty in conveying this message. I've revised the wording. > Changes random source to php_random_int() in shuffle(), str_shuffle(), and array_rand() . 2021年9月3日(金) 23:04 Larry Garfield : > On Fri, Sep 3, 2021, at 8:55 AM, Go Kudo wrote: > > Thank you. > > > > > Why is the number generator a parent class rather than an interface? > > > > This is an implementation limitation. I could not find a way to define my > > own object handler in interface. > > As Nikita pointed out in a previous suggestion, the NumberGenerator now > > uses php_random_ng_algo_user to generate random numbers faster than > before, > > even if it is a userland implementation. > > I'm still unclear how I'd write my own NumberGenerator right now. I mean, > I can extend the class, but I don't have a sense for what I'd do with it > for non-testing/trivial implementations. You say it's using an internal > function to generate numbers, but in user space what would I do with that? > And when/why would I? > > > > You don't mention the CSPRNG functions at all. > > > > This is a mistake. I have corrected it. Thanks! > > I'm still not clear on the intent here. Is the intent that I should stop > using random_int()? And if so, replace it with... what? Do I have to > supply a specific non-default generator? That makes the usability worse, > and more likely to be gotten wrong. > > Also, in future scope you you have this sentence, which makes little sense: > > >> Replace random_bytes() with random_bytes() for random numbers used in > shuffle(), str_shuffle(), and array_rand(). > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
Re: [PHP-DEV] [RFC] Random Extension 3.0
My apologies. I had skipped one. > And either way it needs more clarity about how you'd write one for reals. Added a simple example comparing it to mt_rand. What do you think of this? 2021年9月3日(金) 3:26 Larry Garfield : > On Thu, Sep 2, 2021, at 10:10 AM, Go Kudo wrote: > > Hi Internals. > > > > Expanded from the previous RFC and changed it to an RFC that organizes > the > > whole PHP random number generator. Also, the target version has been > > changed to 8.2. > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/7453 > > > > Hopefully you will get some good responses. > > > > Regards, > > Go Kudo > > This looks pretty nice to my unskilled eye. Two questions. > > 1) Why is the number generator a parent class rather than an interface? > It seems like an interface target. And either way it needs more clarity > about how you'd write one for reals. (It returns an int, so does that mean > getBytes() just derives from a series of ints?) > > 2) You don't mention the CSPRNG functions at all. Is there any intention > to fold those into this model as well, or to leave those as is? Or is the > Secure generator implementation intended to replace those? It's unclear > here. > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
Re: [PHP-DEV] [RFC] Random Extension 3.0
Thank you. > Why is the number generator a parent class rather than an interface? This is an implementation limitation. I could not find a way to define my own object handler in interface. As Nikita pointed out in a previous suggestion, the NumberGenerator now uses php_random_ng_algo_user to generate random numbers faster than before, even if it is a userland implementation. > You don't mention the CSPRNG functions at all. This is a mistake. I have corrected it. Thanks! I will keep an eye on it throughout the next week, and if there are no additional responses, I will start voting the week after next. Regards, Go Kudo 2021年9月3日(金) 3:26 Larry Garfield : > On Thu, Sep 2, 2021, at 10:10 AM, Go Kudo wrote: > > Hi Internals. > > > > Expanded from the previous RFC and changed it to an RFC that organizes > the > > whole PHP random number generator. Also, the target version has been > > changed to 8.2. > > > > https://wiki.php.net/rfc/rng_extension > > https://github.com/php/php-src/pull/7453 > > > > Hopefully you will get some good responses. > > > > Regards, > > Go Kudo > > This looks pretty nice to my unskilled eye. Two questions. > > 1) Why is the number generator a parent class rather than an interface? > It seems like an interface target. And either way it needs more clarity > about how you'd write one for reals. (It returns an int, so does that mean > getBytes() just derives from a series of ints?) > > 2) You don't mention the CSPRNG functions at all. Is there any intention > to fold those into this model as well, or to leave those as is? Or is the > Secure generator implementation intended to replace those? It's unclear > here. > > --Larry Garfield > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
[PHP-DEV] [RFC] Random Extension 3.0
Hi Internals. Expanded from the previous RFC and changed it to an RFC that organizes the whole PHP random number generator. Also, the target version has been changed to 8.2. https://wiki.php.net/rfc/rng_extension https://github.com/php/php-src/pull/7453 Hopefully you will get some good responses. Regards, Go Kudo
[PHP-DEV] Re: [RFC] Add Random Extension (before: Add Random class)
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 : > 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. > > ```sh > # 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 >
Re: [PHP-DEV] [RFC] Add Random Extension (before: Add Random class)
Sorry, let me check one thing. The macros provided by ext/standard/php_random.h are currently being used from several places. Merging this into ext/stanrard/php_rand.h would require support for all dependent code. Also, if a third party extension includes ext/standard/php_random.h and uses the php_random_bytes_throw / php_random_bytes_silent macros, this will result in a BC break. Are you sure there is no problem doing this? Translated with www.DeepL.com/Translator (free version) Regards, Go Kudo 2021年7月7日(水) 22:10 Nikita Popov : > On Wed, Jul 7, 2021 at 3:08 PM Go Kudo wrote: > >> We'll do it that way. >> >> Thanks for fixing the memory issue. I'm not quite sure I understand the >> principle, so I will investigate. >> >> By the way, are you working on something now? If so, I'll pause the work. >> > > Nope, I just dropped by to fix the valgrind warning. Generally, you also > need to initialize these is_null variables to true, because if the > parameter is not passed, then the variables will be left at the default > value and you want to treat "not passed" the same way as "null was passed". > > Regards, > Nikita > > >> Regards, >> Go Kudo >> 2021年7月7日(水) 21:41 Nikita Popov : >> >>> On Wed, Jul 7, 2021 at 2:33 PM Go Kudo wrote: >>> >>>> Incidentally, what would be the preferred name for the ext/standard >>>> random? >>>> I was going to rename it to random_func, but I have a feeling that >>>> would be controversial. >>>> >>>> - random_func.c / php_random_func.h / RANDOM_FUNC_G / >>>> php_random_func_bytes() / php_random_func_int() >>>> - std_random.c ... >>>> - standard_random.c ... >>>> >>>> Which would be better? >>>> >>> >>> We already have another php_rand.h header, so I think you can just merge >>> them. Name of the C file shouldn't matter. >>> >>> Regards, >>> Nikita >>> >>> >>>> 2021年7月7日(水) 19:32 Nikita Popov : >>>> >>>>> On Tue, Jul 6, 2021 at 4:38 PM Go Kudo wrote: >>>>> >>>>>> > 1st >>>>>> >>>>>> This is to avoid conflicts with the implementation in ext/standard. I >>>>>> don't >>>>>> want to do it this way either, but I have to do it this way. >>>>>> Since random in ext/standard does not use namespaces, I would like to >>>>>> change the ext/standard side. >>>>>> >>>>> >>>>> To clarify, are you referring to the php_random.h header in >>>>> ext/standard? I agree with Remi that the extension should be in >>>>> ext/random, >>>>> not ext/random_ext. We can rename the ext/standard header. >>>>> >>>>> Alternatively, you could also use ext/rng, with names RNG\Random, >>>>> RNG\NumberGenerator\XorShift128Plus etc. >>>>> >>>>> Regards, >>>>> Nikita >>>>> >>>>> >>>>>> > 2nd >>>>>> >>>>>> Although it goes back quite a long time, this implementation was >>>>>> originally >>>>>> based on an extension I submitted to PECL. >>>>>> >>>>>> https://pecl.php.net/package/orng >>>>>> >>>>>> After I posted this to PECL, I found that an object scope RNG had been >>>>>> proposed in the past in the Internals ML, and there was positive >>>>>> feedback >>>>>> about it. >>>>>> >>>>>> https://externals.io/message/112525 >>>>>> >>>>>> However, the proposal never actually took place. This RFC is a >>>>>> realization >>>>>> of that proposal. >>>>>> >>>>>> Is that what you asked? >>>>>> >>>>>> Regards, >>>>>> Go Kudo >>>>>> >>>>>> 2021年7月6日(火) 22:46 Remi Collet : >>>>>> >>>>>> > 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 >>>>>> > >>>>>> > -- >>>>>> > PHP Internals - PHP Runtime Development Mailing List >>>>>> > To unsubscribe, visit: https://www.php.net/unsub.php >>>>>> > >>>>>> > >>>>>> >>>>>
