Skip to content

random: Optimize data flow for the generate function of native engines #13043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 9, 2024
Merged

random: Optimize data flow for the generate function of native engines #13043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions UPGRADING.INTERNALS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions ext/random/engine_combinedlcg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 10 additions & 11 deletions ext/random/engine_mt19937.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
9 changes: 6 additions & 3 deletions ext/random/engine_pcgoneseq128xslrr64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions ext/random/engine_secure.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
20 changes: 13 additions & 7 deletions ext/random/engine_user.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 */
Expand All @@ -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)
Expand All @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions ext/random/engine_xoshiro256starstar.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 6 additions & 3 deletions ext/random/php_random.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The order of all initializers is inversed, so maybe inverse it here too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've initially had it reversed, but this order minimizes the differences in the generated assembly, because .result stays in rax (with .size being in rdx, instead of vice versa).

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);
Expand Down
32 changes: 15 additions & 17 deletions ext/random/random.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -229,15 +229,13 @@ 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;
}

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;
Expand Down Expand Up @@ -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;
}
/* }}} */

Expand All @@ -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;
}
/* }}} */

Expand All @@ -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.
Expand Down
Loading