From ba89f9b14248e74b385dc69af8ffbe60b54b028d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 28 Dec 2023 16:46:06 +0100 Subject: [PATCH] random: Optimize data flow for the `generate` function of native engines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 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. --- UPGRADING.INTERNALS | 4 ++++ ext/random/engine_combinedlcg.c | 8 ++++--- ext/random/engine_mt19937.c | 21 ++++++++-------- ext/random/engine_pcgoneseq128xslrr64.c | 9 ++++--- ext/random/engine_secure.c | 8 ++++--- ext/random/engine_user.c | 20 ++++++++++------ ext/random/engine_xoshiro256starstar.c | 8 ++++--- ext/random/php_random.h | 9 ++++--- ext/random/random.c | 32 ++++++++++++------------- ext/random/randomizer.c | 27 ++++++++++----------- 10 files changed, 82 insertions(+), 64 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index bc959eb5038c2..eeab501f756ed 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -47,6 +47,10 @@ PHP 8.4 INTERNALS UPGRADE NOTES - The macro RAND_RANGE_BADSCALING() has been removed. The implementation should either be inlined and undefined behavior fixed or it should be replaced by a non-biased scaler. + - The generate member of a php_random_algo is now expected to return + the new php_random_result struct, replacing the last_generated_size + member of the php_random_status struct and the generate_size member of + the php_random_algo struct. c. ext/xsl - The function php_xsl_create_object() was removed as it was not used diff --git a/ext/random/engine_combinedlcg.c b/ext/random/engine_combinedlcg.c index 2fa26860ec2ce..4abb0bea48e6b 100644 --- a/ext/random/engine_combinedlcg.c +++ b/ext/random/engine_combinedlcg.c @@ -40,7 +40,7 @@ static void seed(php_random_status *status, uint64_t seed) s->state[1] = seed >> 32; } -static uint64_t generate(php_random_status *status) +static php_random_result generate(php_random_status *status) { php_random_status_state_combinedlcg *s = status->state; int32_t q, z; @@ -53,7 +53,10 @@ static uint64_t generate(php_random_status *status) z += 2147483562; } - return (uint64_t) z; + return (php_random_result){ + .size = sizeof(uint32_t), + .result = (uint64_t) z, + }; } 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) } const php_random_algo php_random_algo_combinedlcg = { - sizeof(uint32_t), sizeof(php_random_status_state_combinedlcg), seed, generate, diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c index e16c9f6722fc3..377526550a6e5 100644 --- a/ext/random/engine_mt19937.c +++ b/ext/random/engine_mt19937.c @@ -143,7 +143,7 @@ static void seed(php_random_status *status, uint64_t seed) mt19937_seed_state(status->state, seed); } -static uint64_t generate(php_random_status *status) +static php_random_result generate(php_random_status *status) { php_random_status_state_mt19937 *s = status->state; uint32_t s1; @@ -157,7 +157,10 @@ static uint64_t generate(php_random_status *status) s1 ^= (s1 << 7) & 0x9d2c5680U; s1 ^= (s1 << 15) & 0xefc60000U; - return (uint64_t) (s1 ^ (s1 >> 18)); + return (php_random_result){ + .size = sizeof(uint32_t), + .result = (uint64_t) (s1 ^ (s1 >> 18)), + }; } 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) } const php_random_algo php_random_algo_mt19937 = { - sizeof(uint32_t), sizeof(php_random_status_state_mt19937), seed, generate, @@ -288,25 +290,22 @@ PHP_METHOD(Random_Engine_Mt19937, __construct) PHP_METHOD(Random_Engine_Mt19937, generate) { php_random_engine *engine = Z_RANDOM_ENGINE_P(ZEND_THIS); - uint64_t generated; - size_t size; zend_string *bytes; ZEND_PARSE_PARAMETERS_NONE(); - generated = engine->algo->generate(engine->status); - size = engine->status->last_generated_size; + php_random_result generated = engine->algo->generate(engine->status); if (EG(exception)) { RETURN_THROWS(); } - bytes = zend_string_alloc(size, false); + bytes = zend_string_alloc(generated.size, false); /* Endianness safe copy */ - for (size_t i = 0; i < size; i++) { - ZSTR_VAL(bytes)[i] = (generated >> (i * 8)) & 0xff; + for (size_t i = 0; i < generated.size; i++) { + ZSTR_VAL(bytes)[i] = (generated.result >> (i * 8)) & 0xff; } - ZSTR_VAL(bytes)[size] = '\0'; + ZSTR_VAL(bytes)[generated.size] = '\0'; RETURN_STR(bytes); } diff --git a/ext/random/engine_pcgoneseq128xslrr64.c b/ext/random/engine_pcgoneseq128xslrr64.c index 243e296bbe890..885fd9bf0aab3 100644 --- a/ext/random/engine_pcgoneseq128xslrr64.c +++ b/ext/random/engine_pcgoneseq128xslrr64.c @@ -47,12 +47,16 @@ static void seed(php_random_status *status, uint64_t seed) seed128(status, php_random_uint128_constant(0ULL, seed)); } -static uint64_t generate(php_random_status *status) +static php_random_result generate(php_random_status *status) { php_random_status_state_pcgoneseq128xslrr64 *s = status->state; step(s); - return php_random_pcgoneseq128xslrr64_rotr64(s->state); + + return (php_random_result){ + .size = sizeof(uint64_t), + .result = php_random_pcgoneseq128xslrr64_rotr64(s->state), + }; } 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) } const php_random_algo php_random_algo_pcgoneseq128xslrr64 = { - sizeof(uint64_t), sizeof(php_random_status_state_pcgoneseq128xslrr64), seed, generate, diff --git a/ext/random/engine_secure.c b/ext/random/engine_secure.c index 8cc5d9cb6f249..90ed4c0ed8504 100644 --- a/ext/random/engine_secure.c +++ b/ext/random/engine_secure.c @@ -24,13 +24,16 @@ #include "Zend/zend_exceptions.h" -static uint64_t generate(php_random_status *status) +static php_random_result generate(php_random_status *status) { zend_ulong r = 0; php_random_bytes_throw(&r, sizeof(zend_ulong)); - return r; + return (php_random_result){ + .size = sizeof(zend_ulong), + .result = r, + }; } 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) } const php_random_algo php_random_algo_secure = { - sizeof(zend_ulong), 0, NULL, generate, diff --git a/ext/random/engine_user.c b/ext/random/engine_user.c index b45924d3bb7da..9d7521a29bf92 100644 --- a/ext/random/engine_user.c +++ b/ext/random/engine_user.c @@ -21,7 +21,7 @@ #include "php.h" #include "php_random.h" -static uint64_t generate(php_random_status *status) +static php_random_result generate(php_random_status *status) { php_random_status_state_user *s = status->state; uint64_t result = 0; @@ -31,17 +31,18 @@ static uint64_t generate(php_random_status *status) zend_call_known_instance_method_with_0_params(s->generate_method, s->object, &retval); if (EG(exception)) { - return 0; + return (php_random_result){ + .size = sizeof(uint64_t), + .result = 0, + }; } - /* Store generated size in a state */ size = Z_STRLEN(retval); /* Guard for over 64-bit results */ if (size > sizeof(uint64_t)) { size = sizeof(uint64_t); } - status->last_generated_size = size; if (size > 0) { /* Endianness safe copy */ @@ -50,12 +51,18 @@ static uint64_t generate(php_random_status *status) } } else { zend_throw_error(random_ce_Random_BrokenRandomEngineError, "A random engine must return a non-empty string"); - return 0; + return (php_random_result){ + .size = sizeof(uint64_t), + .result = 0, + }; } zval_ptr_dtor(&retval); - return result; + return (php_random_result){ + .size = size, + .result = result, + }; } 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) } const php_random_algo php_random_algo_user = { - 0, sizeof(php_random_status_state_user), NULL, generate, diff --git a/ext/random/engine_xoshiro256starstar.c b/ext/random/engine_xoshiro256starstar.c index 52617adb10565..78cafe341c32e 100644 --- a/ext/random/engine_xoshiro256starstar.c +++ b/ext/random/engine_xoshiro256starstar.c @@ -103,9 +103,12 @@ static void seed(php_random_status *status, uint64_t seed) seed256(status, s[0], s[1], s[2], s[3]); } -static uint64_t generate(php_random_status *status) +static php_random_result generate(php_random_status *status) { - return generate_state(status->state); + return (php_random_result){ + .size = sizeof(uint64_t), + .result = generate_state(status->state), + }; } 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) } const php_random_algo php_random_algo_xoshiro256starstar = { - sizeof(uint64_t), sizeof(php_random_status_state_xoshiro256starstar), seed, generate, diff --git a/ext/random/php_random.h b/ext/random/php_random.h index bcae40e7a0740..9fc29951f383b 100644 --- a/ext/random/php_random.h +++ b/ext/random/php_random.h @@ -191,7 +191,6 @@ static inline zend_result php_random_int_silent(zend_long min, zend_long max, ze } typedef struct _php_random_status_ { - size_t last_generated_size; void *state; } php_random_status; @@ -218,11 +217,15 @@ typedef struct _php_random_status_state_user { zend_function *generate_method; } php_random_status_state_user; +typedef struct _php_random_result { + const uint64_t result; + const size_t size; +} php_random_result; + typedef struct _php_random_algo { - const size_t generate_size; const size_t state_size; void (*seed)(php_random_status *status, uint64_t seed); - uint64_t (*generate)(php_random_status *status); + php_random_result (*generate)(php_random_status *status); zend_long (*range)(php_random_status *status, zend_long min, zend_long max); bool (*serialize)(php_random_status *status, HashTable *data); bool (*unserialize)(php_random_status *status, HashTable *data); diff --git a/ext/random/random.c b/ext/random/random.c index f34d55f340727..7935e1794fdb2 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -82,9 +82,9 @@ PHPAPI uint32_t php_random_range32(const php_random_algo *algo, php_random_statu result = 0; total_size = 0; do { - uint32_t r = algo->generate(status); - result = result | (r << (total_size * 8)); - total_size += status->last_generated_size; + php_random_result r = algo->generate(status); + result = result | (((uint32_t) r.result) << (total_size * 8)); + total_size += r.size; if (EG(exception)) { return 0; } @@ -117,9 +117,9 @@ PHPAPI uint32_t php_random_range32(const php_random_algo *algo, php_random_statu result = 0; total_size = 0; do { - uint32_t r = algo->generate(status); - result = result | (r << (total_size * 8)); - total_size += status->last_generated_size; + php_random_result r = algo->generate(status); + result = result | (((uint32_t) r.result) << (total_size * 8)); + total_size += r.size; if (EG(exception)) { return 0; } @@ -138,9 +138,9 @@ PHPAPI uint64_t php_random_range64(const php_random_algo *algo, php_random_statu result = 0; total_size = 0; do { - uint64_t r = algo->generate(status); - result = result | (r << (total_size * 8)); - total_size += status->last_generated_size; + php_random_result r = algo->generate(status); + result = result | (r.result << (total_size * 8)); + total_size += r.size; if (EG(exception)) { return 0; } @@ -173,9 +173,9 @@ PHPAPI uint64_t php_random_range64(const php_random_algo *algo, php_random_statu result = 0; total_size = 0; do { - uint64_t r = algo->generate(status); - result = result | (r << (total_size * 8)); - total_size += status->last_generated_size; + php_random_result r = algo->generate(status); + result = result | (r.result << (total_size * 8)); + total_size += r.size; if (EG(exception)) { return 0; } @@ -229,7 +229,6 @@ PHPAPI php_random_status *php_random_status_alloc(const php_random_algo *algo, c { php_random_status *status = pecalloc(1, sizeof(php_random_status), persistent); - status->last_generated_size = algo->generate_size; status->state = algo->state_size > 0 ? pecalloc(1, algo->state_size, persistent) : NULL; return status; @@ -237,7 +236,6 @@ PHPAPI php_random_status *php_random_status_alloc(const php_random_algo *algo, c PHPAPI php_random_status *php_random_status_copy(const php_random_algo *algo, php_random_status *old_status, php_random_status *new_status) { - new_status->last_generated_size = old_status->last_generated_size; new_status->state = memcpy(new_status->state, old_status->state, algo->state_size); return new_status; @@ -400,7 +398,7 @@ PHPAPI double php_combined_lcg(void) RANDOM_G(combined_lcg_seeded) = true; } - return php_random_algo_combinedlcg.generate(status) * 4.656613e-10; + return php_random_algo_combinedlcg.generate(status).result * 4.656613e-10; } /* }}} */ @@ -415,7 +413,7 @@ PHPAPI void php_mt_srand(uint32_t seed) /* {{{ php_mt_rand */ PHPAPI uint32_t php_mt_rand(void) { - return (uint32_t) php_random_algo_mt19937.generate(php_random_default_status()); + return (uint32_t) php_random_algo_mt19937.generate(php_random_default_status()).result; } /* }}} */ @@ -437,7 +435,7 @@ PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max) return php_mt_rand_range(min, max); } - uint64_t r = php_random_algo_mt19937.generate(php_random_default_status()) >> 1; + uint64_t r = php_random_algo_mt19937.generate(php_random_default_status()).result >> 1; /* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering * (max - min) > ZEND_LONG_MAX. diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index fe9ad5fc35a9c..54571365d4e75 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -101,9 +101,9 @@ PHP_METHOD(Random_Randomizer, nextFloat) result = 0; total_size = 0; do { - uint64_t r = randomizer->algo->generate(randomizer->status); - result = result | (r << (total_size * 8)); - total_size += randomizer->status->last_generated_size; + php_random_result r = randomizer->algo->generate(randomizer->status); + result = result | (r.result << (total_size * 8)); + total_size += r.size; if (EG(exception)) { RETURN_THROWS(); } @@ -208,20 +208,19 @@ PHP_METHOD(Random_Randomizer, getFloat) PHP_METHOD(Random_Randomizer, nextInt) { php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS); - uint64_t result; ZEND_PARSE_PARAMETERS_NONE(); - result = randomizer->algo->generate(randomizer->status); + php_random_result result = randomizer->algo->generate(randomizer->status); if (EG(exception)) { RETURN_THROWS(); } - if (randomizer->status->last_generated_size > sizeof(zend_long)) { + if (result.size > sizeof(zend_long)) { zend_throw_exception(random_ce_Random_RandomException, "Generated value exceeds size of int", 0); RETURN_THROWS(); } - RETURN_LONG((zend_long) (result >> 1)); + RETURN_LONG((zend_long) (result.result >> 1)); } /* }}} */ @@ -246,7 +245,7 @@ PHP_METHOD(Random_Randomizer, getInt) randomizer->algo->range == php_random_algo_mt19937.range && ((php_random_status_state_mt19937 *) randomizer->status->state)->mode != MT_RAND_MT19937 )) { - uint64_t r = php_random_algo_mt19937.generate(randomizer->status) >> 1; + uint64_t r = php_random_algo_mt19937.generate(randomizer->status).result >> 1; /* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering * (max - min) > ZEND_LONG_MAX. @@ -286,13 +285,13 @@ PHP_METHOD(Random_Randomizer, getBytes) retval = zend_string_alloc(length, 0); while (total_size < length) { - uint64_t result = randomizer->algo->generate(randomizer->status); + php_random_result result = randomizer->algo->generate(randomizer->status); if (EG(exception)) { zend_string_free(retval); RETURN_THROWS(); } - for (size_t i = 0; i < randomizer->status->last_generated_size; i++) { - ZSTR_VAL(retval)[total_size++] = (result >> (i * 8)) & 0xff; + for (size_t i = 0; i < result.size; i++) { + ZSTR_VAL(retval)[total_size++] = (result.result >> (i * 8)) & 0xff; if (total_size >= length) { break; } @@ -425,14 +424,14 @@ PHP_METHOD(Random_Randomizer, getBytesFromString) int failures = 0; while (total_size < length) { - uint64_t result = randomizer->algo->generate(randomizer->status); + php_random_result result = randomizer->algo->generate(randomizer->status); if (EG(exception)) { zend_string_free(retval); RETURN_THROWS(); } - for (size_t i = 0; i < randomizer->status->last_generated_size; i++) { - uint64_t offset = (result >> (i * 8)) & mask; + for (size_t i = 0; i < result.size; i++) { + uint64_t offset = (result.result >> (i * 8)) & mask; if (offset > max_offset) { if (++failures > PHP_RANDOM_RANGE_ATTEMPTS) {