Skip to content

Commit 162e1dc

Browse files
authored
random: Optimize data flow for the generate function of native engines (#13043)
Instead of returning the generated `uint64_t` and providing the size (i.e. the number of bytes of the generated value) out-of-band via the `last_generated_size` member of the `php_random_status` struct, the `generate` function is now expected to return a new `php_random_result` struct containing both the `size` and the `result`. This has two benefits, one for the developer: It's no longer possible to forget setting `last_generated_size` to the correct value, because it now happens at the time of returning from the function. and the other benefit is for performance: The `php_random_result` struct will be returned as a register pair, thus the `size` will be directly available without reloading it from main memory. Checking a simplified version of `php_random_range64()` on Compiler Explorer (“Godbolt”) with clang 17 shows a single change in the resulting assembly showcasing the improvement (https://godbolt.org/z/G4WjdYxqx): - add rbp, qword ptr [r14] + add rbp, rdx Empirical testing confirms a measurable performance increase for the `Randomizer::getBytes()` method: <?php $e = new Random\Engine\Xoshiro256StarStar(0); $r = new Random\Randomizer($e); var_dump(strlen($r->getBytes(100000000))); goes from 250ms (before the change) to 220ms (after the change). While generating 100 MB of random data certainly is not the most common use case, it confirms the theoretical improvement in practice.
1 parent d778c24 commit 162e1dc

10 files changed

+82
-64
lines changed

UPGRADING.INTERNALS

+4
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ PHP 8.4 INTERNALS UPGRADE NOTES
4747
- The macro RAND_RANGE_BADSCALING() has been removed. The implementation
4848
should either be inlined and undefined behavior fixed or it should be
4949
replaced by a non-biased scaler.
50+
- The generate member of a php_random_algo is now expected to return
51+
the new php_random_result struct, replacing the last_generated_size
52+
member of the php_random_status struct and the generate_size member of
53+
the php_random_algo struct.
5054

5155
c. ext/xsl
5256
- The function php_xsl_create_object() was removed as it was not used

ext/random/engine_combinedlcg.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ static void seed(php_random_status *status, uint64_t seed)
4040
s->state[1] = seed >> 32;
4141
}
4242

43-
static uint64_t generate(php_random_status *status)
43+
static php_random_result generate(php_random_status *status)
4444
{
4545
php_random_status_state_combinedlcg *s = status->state;
4646
int32_t q, z;
@@ -53,7 +53,10 @@ static uint64_t generate(php_random_status *status)
5353
z += 2147483562;
5454
}
5555

56-
return (uint64_t) z;
56+
return (php_random_result){
57+
.size = sizeof(uint32_t),
58+
.result = (uint64_t) z,
59+
};
5760
}
5861

5962
static zend_long range(php_random_status *status, zend_long min, zend_long max)
@@ -93,7 +96,6 @@ static bool unserialize(php_random_status *status, HashTable *data)
9396
}
9497

9598
const php_random_algo php_random_algo_combinedlcg = {
96-
sizeof(uint32_t),
9799
sizeof(php_random_status_state_combinedlcg),
98100
seed,
99101
generate,

ext/random/engine_mt19937.c

+10-11
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ static void seed(php_random_status *status, uint64_t seed)
143143
mt19937_seed_state(status->state, seed);
144144
}
145145

146-
static uint64_t generate(php_random_status *status)
146+
static php_random_result generate(php_random_status *status)
147147
{
148148
php_random_status_state_mt19937 *s = status->state;
149149
uint32_t s1;
@@ -157,7 +157,10 @@ static uint64_t generate(php_random_status *status)
157157
s1 ^= (s1 << 7) & 0x9d2c5680U;
158158
s1 ^= (s1 << 15) & 0xefc60000U;
159159

160-
return (uint64_t) (s1 ^ (s1 >> 18));
160+
return (php_random_result){
161+
.size = sizeof(uint32_t),
162+
.result = (uint64_t) (s1 ^ (s1 >> 18)),
163+
};
161164
}
162165

163166
static zend_long range(php_random_status *status, zend_long min, zend_long max)
@@ -223,7 +226,6 @@ static bool unserialize(php_random_status *status, HashTable *data)
223226
}
224227

225228
const php_random_algo php_random_algo_mt19937 = {
226-
sizeof(uint32_t),
227229
sizeof(php_random_status_state_mt19937),
228230
seed,
229231
generate,
@@ -288,25 +290,22 @@ PHP_METHOD(Random_Engine_Mt19937, __construct)
288290
PHP_METHOD(Random_Engine_Mt19937, generate)
289291
{
290292
php_random_engine *engine = Z_RANDOM_ENGINE_P(ZEND_THIS);
291-
uint64_t generated;
292-
size_t size;
293293
zend_string *bytes;
294294

295295
ZEND_PARSE_PARAMETERS_NONE();
296296

297-
generated = engine->algo->generate(engine->status);
298-
size = engine->status->last_generated_size;
297+
php_random_result generated = engine->algo->generate(engine->status);
299298
if (EG(exception)) {
300299
RETURN_THROWS();
301300
}
302301

303-
bytes = zend_string_alloc(size, false);
302+
bytes = zend_string_alloc(generated.size, false);
304303

305304
/* Endianness safe copy */
306-
for (size_t i = 0; i < size; i++) {
307-
ZSTR_VAL(bytes)[i] = (generated >> (i * 8)) & 0xff;
305+
for (size_t i = 0; i < generated.size; i++) {
306+
ZSTR_VAL(bytes)[i] = (generated.result >> (i * 8)) & 0xff;
308307
}
309-
ZSTR_VAL(bytes)[size] = '\0';
308+
ZSTR_VAL(bytes)[generated.size] = '\0';
310309

311310
RETURN_STR(bytes);
312311
}

ext/random/engine_pcgoneseq128xslrr64.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,16 @@ static void seed(php_random_status *status, uint64_t seed)
4747
seed128(status, php_random_uint128_constant(0ULL, seed));
4848
}
4949

50-
static uint64_t generate(php_random_status *status)
50+
static php_random_result generate(php_random_status *status)
5151
{
5252
php_random_status_state_pcgoneseq128xslrr64 *s = status->state;
5353

5454
step(s);
55-
return php_random_pcgoneseq128xslrr64_rotr64(s->state);
55+
56+
return (php_random_result){
57+
.size = sizeof(uint64_t),
58+
.result = php_random_pcgoneseq128xslrr64_rotr64(s->state),
59+
};
5660
}
5761

5862
static zend_long range(php_random_status *status, zend_long min, zend_long max)
@@ -103,7 +107,6 @@ static bool unserialize(php_random_status *status, HashTable *data)
103107
}
104108

105109
const php_random_algo php_random_algo_pcgoneseq128xslrr64 = {
106-
sizeof(uint64_t),
107110
sizeof(php_random_status_state_pcgoneseq128xslrr64),
108111
seed,
109112
generate,

ext/random/engine_secure.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,16 @@
2424

2525
#include "Zend/zend_exceptions.h"
2626

27-
static uint64_t generate(php_random_status *status)
27+
static php_random_result generate(php_random_status *status)
2828
{
2929
zend_ulong r = 0;
3030

3131
php_random_bytes_throw(&r, sizeof(zend_ulong));
3232

33-
return r;
33+
return (php_random_result){
34+
.size = sizeof(zend_ulong),
35+
.result = r,
36+
};
3437
}
3538

3639
static zend_long range(php_random_status *status, zend_long min, zend_long max)
@@ -43,7 +46,6 @@ static zend_long range(php_random_status *status, zend_long min, zend_long max)
4346
}
4447

4548
const php_random_algo php_random_algo_secure = {
46-
sizeof(zend_ulong),
4749
0,
4850
NULL,
4951
generate,

ext/random/engine_user.c

+13-7
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include "php.h"
2222
#include "php_random.h"
2323

24-
static uint64_t generate(php_random_status *status)
24+
static php_random_result generate(php_random_status *status)
2525
{
2626
php_random_status_state_user *s = status->state;
2727
uint64_t result = 0;
@@ -31,17 +31,18 @@ static uint64_t generate(php_random_status *status)
3131
zend_call_known_instance_method_with_0_params(s->generate_method, s->object, &retval);
3232

3333
if (EG(exception)) {
34-
return 0;
34+
return (php_random_result){
35+
.size = sizeof(uint64_t),
36+
.result = 0,
37+
};
3538
}
3639

37-
/* Store generated size in a state */
3840
size = Z_STRLEN(retval);
3941

4042
/* Guard for over 64-bit results */
4143
if (size > sizeof(uint64_t)) {
4244
size = sizeof(uint64_t);
4345
}
44-
status->last_generated_size = size;
4546

4647
if (size > 0) {
4748
/* Endianness safe copy */
@@ -50,12 +51,18 @@ static uint64_t generate(php_random_status *status)
5051
}
5152
} else {
5253
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "A random engine must return a non-empty string");
53-
return 0;
54+
return (php_random_result){
55+
.size = sizeof(uint64_t),
56+
.result = 0,
57+
};
5458
}
5559

5660
zval_ptr_dtor(&retval);
5761

58-
return result;
62+
return (php_random_result){
63+
.size = size,
64+
.result = result,
65+
};
5966
}
6067

6168
static zend_long range(php_random_status *status, zend_long min, zend_long max)
@@ -64,7 +71,6 @@ static zend_long range(php_random_status *status, zend_long min, zend_long max)
6471
}
6572

6673
const php_random_algo php_random_algo_user = {
67-
0,
6874
sizeof(php_random_status_state_user),
6975
NULL,
7076
generate,

ext/random/engine_xoshiro256starstar.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,12 @@ static void seed(php_random_status *status, uint64_t seed)
103103
seed256(status, s[0], s[1], s[2], s[3]);
104104
}
105105

106-
static uint64_t generate(php_random_status *status)
106+
static php_random_result generate(php_random_status *status)
107107
{
108-
return generate_state(status->state);
108+
return (php_random_result){
109+
.size = sizeof(uint64_t),
110+
.result = generate_state(status->state),
111+
};
109112
}
110113

111114
static zend_long range(php_random_status *status, zend_long min, zend_long max)
@@ -150,7 +153,6 @@ static bool unserialize(php_random_status *status, HashTable *data)
150153
}
151154

152155
const php_random_algo php_random_algo_xoshiro256starstar = {
153-
sizeof(uint64_t),
154156
sizeof(php_random_status_state_xoshiro256starstar),
155157
seed,
156158
generate,

ext/random/php_random.h

+6-3
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ static inline zend_result php_random_int_silent(zend_long min, zend_long max, ze
191191
}
192192

193193
typedef struct _php_random_status_ {
194-
size_t last_generated_size;
195194
void *state;
196195
} php_random_status;
197196

@@ -218,11 +217,15 @@ typedef struct _php_random_status_state_user {
218217
zend_function *generate_method;
219218
} php_random_status_state_user;
220219

220+
typedef struct _php_random_result {
221+
const uint64_t result;
222+
const size_t size;
223+
} php_random_result;
224+
221225
typedef struct _php_random_algo {
222-
const size_t generate_size;
223226
const size_t state_size;
224227
void (*seed)(php_random_status *status, uint64_t seed);
225-
uint64_t (*generate)(php_random_status *status);
228+
php_random_result (*generate)(php_random_status *status);
226229
zend_long (*range)(php_random_status *status, zend_long min, zend_long max);
227230
bool (*serialize)(php_random_status *status, HashTable *data);
228231
bool (*unserialize)(php_random_status *status, HashTable *data);

ext/random/random.c

+15-17
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ PHPAPI uint32_t php_random_range32(const php_random_algo *algo, php_random_statu
8282
result = 0;
8383
total_size = 0;
8484
do {
85-
uint32_t r = algo->generate(status);
86-
result = result | (r << (total_size * 8));
87-
total_size += status->last_generated_size;
85+
php_random_result r = algo->generate(status);
86+
result = result | (((uint32_t) r.result) << (total_size * 8));
87+
total_size += r.size;
8888
if (EG(exception)) {
8989
return 0;
9090
}
@@ -117,9 +117,9 @@ PHPAPI uint32_t php_random_range32(const php_random_algo *algo, php_random_statu
117117
result = 0;
118118
total_size = 0;
119119
do {
120-
uint32_t r = algo->generate(status);
121-
result = result | (r << (total_size * 8));
122-
total_size += status->last_generated_size;
120+
php_random_result r = algo->generate(status);
121+
result = result | (((uint32_t) r.result) << (total_size * 8));
122+
total_size += r.size;
123123
if (EG(exception)) {
124124
return 0;
125125
}
@@ -138,9 +138,9 @@ PHPAPI uint64_t php_random_range64(const php_random_algo *algo, php_random_statu
138138
result = 0;
139139
total_size = 0;
140140
do {
141-
uint64_t r = algo->generate(status);
142-
result = result | (r << (total_size * 8));
143-
total_size += status->last_generated_size;
141+
php_random_result r = algo->generate(status);
142+
result = result | (r.result << (total_size * 8));
143+
total_size += r.size;
144144
if (EG(exception)) {
145145
return 0;
146146
}
@@ -173,9 +173,9 @@ PHPAPI uint64_t php_random_range64(const php_random_algo *algo, php_random_statu
173173
result = 0;
174174
total_size = 0;
175175
do {
176-
uint64_t r = algo->generate(status);
177-
result = result | (r << (total_size * 8));
178-
total_size += status->last_generated_size;
176+
php_random_result r = algo->generate(status);
177+
result = result | (r.result << (total_size * 8));
178+
total_size += r.size;
179179
if (EG(exception)) {
180180
return 0;
181181
}
@@ -229,15 +229,13 @@ PHPAPI php_random_status *php_random_status_alloc(const php_random_algo *algo, c
229229
{
230230
php_random_status *status = pecalloc(1, sizeof(php_random_status), persistent);
231231

232-
status->last_generated_size = algo->generate_size;
233232
status->state = algo->state_size > 0 ? pecalloc(1, algo->state_size, persistent) : NULL;
234233

235234
return status;
236235
}
237236

238237
PHPAPI php_random_status *php_random_status_copy(const php_random_algo *algo, php_random_status *old_status, php_random_status *new_status)
239238
{
240-
new_status->last_generated_size = old_status->last_generated_size;
241239
new_status->state = memcpy(new_status->state, old_status->state, algo->state_size);
242240

243241
return new_status;
@@ -400,7 +398,7 @@ PHPAPI double php_combined_lcg(void)
400398
RANDOM_G(combined_lcg_seeded) = true;
401399
}
402400

403-
return php_random_algo_combinedlcg.generate(status) * 4.656613e-10;
401+
return php_random_algo_combinedlcg.generate(status).result * 4.656613e-10;
404402
}
405403
/* }}} */
406404

@@ -415,7 +413,7 @@ PHPAPI void php_mt_srand(uint32_t seed)
415413
/* {{{ php_mt_rand */
416414
PHPAPI uint32_t php_mt_rand(void)
417415
{
418-
return (uint32_t) php_random_algo_mt19937.generate(php_random_default_status());
416+
return (uint32_t) php_random_algo_mt19937.generate(php_random_default_status()).result;
419417
}
420418
/* }}} */
421419

@@ -437,7 +435,7 @@ PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max)
437435
return php_mt_rand_range(min, max);
438436
}
439437

440-
uint64_t r = php_random_algo_mt19937.generate(php_random_default_status()) >> 1;
438+
uint64_t r = php_random_algo_mt19937.generate(php_random_default_status()).result >> 1;
441439

442440
/* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering
443441
* (max - min) > ZEND_LONG_MAX.

0 commit comments

Comments
 (0)