Uh oh!
There was an error while loading. Please reload this page.
session: Stop using php_combined_lcg() - #13568
Merged
Merged
Conversation
The previous ordering resulted in a needlessly large number of holes and split
several `zval`s across cache line boundaries. Do the bare minimum of reordering
to keep related members grouped, but reducing the struct size by 32 bytes and
keeping `zval`s within a single cache line.
Before:
struct _php_session_rfc1867_progress {
size_t sname_len; /* 0 8 */
zval sid; /* 8 16 */
smart_str key; /* 24 16 */
zend_long update_step; /* 40 8 */
zend_long next_update; /* 48 8 */
double next_update_time; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
_Bool cancel_upload; /* 64 1 */
_Bool apply_trans_sid; /* 65 1 */
/* XXX 6 bytes hole, try to pack */
size_t content_length; /* 72 8 */
zval data; /* 80 16 */
zval * post_bytes_processed; /* 96 8 */
zval files; /* 104 16 */
zval current_file; /* 120 16 */
/* --- cacheline 2 boundary (128 bytes) was 8 bytes ago --- */
zval * current_file_bytes_processed; /* 136 8 */
/* size: 144, cachelines: 3, members: 14 */
/* sum members: 138, holes: 1, sum holes: 6 */
/* last cacheline: 16 bytes */
};
struct _php_ps_globals {
char * save_path; /* 0 8 */
char * session_name; /* 8 8 */
zend_string * id; /* 16 8 */
char * extern_referer_chk; /* 24 8 */
char * cache_limiter; /* 32 8 */
zend_long cookie_lifetime; /* 40 8 */
char * cookie_path; /* 48 8 */
char * cookie_domain; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
_Bool cookie_secure; /* 64 1 */
_Bool cookie_httponly; /* 65 1 */
/* XXX 6 bytes hole, try to pack */
char * cookie_samesite; /* 72 8 */
const ps_module * mod; /* 80 8 */
const ps_module * default_mod; /* 88 8 */
void * mod_data; /* 96 8 */
php_session_status session_status; /* 104 4 */
/* XXX 4 bytes hole, try to pack */
zend_string * session_started_filename; /* 112 8 */
uint32_t session_started_lineno; /* 120 4 */
/* XXX 4 bytes hole, try to pack */
/* --- cacheline 2 boundary (128 bytes) --- */
zend_long gc_probability; /* 128 8 */
zend_long gc_divisor; /* 136 8 */
zend_long gc_maxlifetime; /* 144 8 */
int module_number; /* 152 4 */
/* XXX 4 bytes hole, try to pack */
zend_long cache_expire; /* 160 8 */
struct {
zval ps_open; /* 168 16 */
zval ps_close; /* 184 16 */
/* --- cacheline 3 boundary (192 bytes) was 8 bytes ago --- */
zval ps_read; /* 200 16 */
zval ps_write; /* 216 16 */
zval ps_destroy; /* 232 16 */
zval ps_gc; /* 248 16 */
/* --- cacheline 4 boundary (256 bytes) was 8 bytes ago --- */
zval ps_create_sid; /* 264 16 */
zval ps_validate_sid; /* 280 16 */
zval ps_update_timestamp; /* 296 16 */
} mod_user_names; /* 168 144 */
_Bool mod_user_implemented; /* 312 1 */
_Bool mod_user_is_open; /* 313 1 */
/* XXX 6 bytes hole, try to pack */
/* --- cacheline 5 boundary (320 bytes) --- */
zend_string * mod_user_class_name; /* 320 8 */
const struct ps_serializer_struct * serializer; /* 328 8 */
zval http_session_vars; /* 336 16 */
_Bool auto_start; /* 352 1 */
_Bool use_cookies; /* 353 1 */
_Bool use_only_cookies; /* 354 1 */
_Bool use_trans_sid; /* 355 1 */
/* XXX 4 bytes hole, try to pack */
zend_long sid_length; /* 360 8 */
zend_long sid_bits_per_character; /* 368 8 */
_Bool send_cookie; /* 376 1 */
_Bool define_sid; /* 377 1 */
/* XXX 6 bytes hole, try to pack */
/* --- cacheline 6 boundary (384 bytes) --- */
php_session_rfc1867_progress * rfc1867_progress; /* 384 8 */
_Bool rfc1867_enabled; /* 392 1 */
_Bool rfc1867_cleanup; /* 393 1 */
/* XXX 6 bytes hole, try to pack */
char * rfc1867_prefix; /* 400 8 */
char * rfc1867_name; /* 408 8 */
zend_long rfc1867_freq; /* 416 8 */
double rfc1867_min_freq; /* 424 8 */
_Bool use_strict_mode; /* 432 1 */
_Bool lazy_write; /* 433 1 */
_Bool in_save_handler; /* 434 1 */
_Bool set_handler; /* 435 1 */
/* XXX 4 bytes hole, try to pack */
zend_string * session_vars; /* 440 8 */
/* size: 448, cachelines: 7, members: 48 */
/* sum members: 404, holes: 9, sum holes: 44 */
};
After:
struct _php_session_rfc1867_progress {
size_t sname_len; /* 0 8 */
zval sid; /* 8 16 */
smart_str key; /* 24 16 */
zend_long update_step; /* 40 8 */
zend_long next_update; /* 48 8 */
double next_update_time; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
_Bool cancel_upload; /* 64 1 */
_Bool apply_trans_sid; /* 65 1 */
/* XXX 6 bytes hole, try to pack */
size_t content_length; /* 72 8 */
zval data; /* 80 16 */
zval files; /* 96 16 */
zval * post_bytes_processed; /* 112 8 */
zval * current_file_bytes_processed; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
zval current_file; /* 128 16 */
/* size: 144, cachelines: 3, members: 14 */
/* sum members: 138, holes: 1, sum holes: 6 */
/* last cacheline: 16 bytes */
};
struct _php_ps_globals {
char * save_path; /* 0 8 */
char * session_name; /* 8 8 */
zend_string * id; /* 16 8 */
char * extern_referer_chk; /* 24 8 */
char * cache_limiter; /* 32 8 */
zend_long cookie_lifetime; /* 40 8 */
char * cookie_path; /* 48 8 */
char * cookie_domain; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
char * cookie_samesite; /* 64 8 */
_Bool cookie_secure; /* 72 1 */
_Bool cookie_httponly; /* 73 1 */
/* XXX 6 bytes hole, try to pack */
const ps_module * mod; /* 80 8 */
const ps_module * default_mod; /* 88 8 */
void * mod_data; /* 96 8 */
php_session_status session_status; /* 104 4 */
/* XXX 4 bytes hole, try to pack */
zend_string * session_started_filename; /* 112 8 */
uint32_t session_started_lineno; /* 120 4 */
int module_number; /* 124 4 */
/* --- cacheline 2 boundary (128 bytes) --- */
zend_long gc_probability; /* 128 8 */
zend_long gc_divisor; /* 136 8 */
zend_long gc_maxlifetime; /* 144 8 */
zend_long cache_expire; /* 152 8 */
struct {
zval ps_open; /* 160 16 */
zval ps_close; /* 176 16 */
/* --- cacheline 3 boundary (192 bytes) --- */
zval ps_read; /* 192 16 */
zval ps_write; /* 208 16 */
zval ps_destroy; /* 224 16 */
zval ps_gc; /* 240 16 */
/* --- cacheline 4 boundary (256 bytes) --- */
zval ps_create_sid; /* 256 16 */
zval ps_validate_sid; /* 272 16 */
zval ps_update_timestamp; /* 288 16 */
} mod_user_names; /* 160 144 */
zend_string * mod_user_class_name; /* 304 8 */
_Bool mod_user_implemented; /* 312 1 */
_Bool mod_user_is_open; /* 313 1 */
_Bool auto_start; /* 314 1 */
_Bool use_cookies; /* 315 1 */
_Bool use_only_cookies; /* 316 1 */
_Bool use_trans_sid; /* 317 1 */
_Bool send_cookie; /* 318 1 */
_Bool define_sid; /* 319 1 */
/* --- cacheline 5 boundary (320 bytes) --- */
const struct ps_serializer_struct * serializer; /* 320 8 */
zval http_session_vars; /* 328 16 */
zend_long sid_length; /* 344 8 */
zend_long sid_bits_per_character; /* 352 8 */
php_session_rfc1867_progress * rfc1867_progress; /* 360 8 */
char * rfc1867_prefix; /* 368 8 */
char * rfc1867_name; /* 376 8 */
/* --- cacheline 6 boundary (384 bytes) --- */
zend_long rfc1867_freq; /* 384 8 */
double rfc1867_min_freq; /* 392 8 */
_Bool rfc1867_enabled; /* 400 1 */
_Bool rfc1867_cleanup; /* 401 1 */
_Bool use_strict_mode; /* 402 1 */
_Bool lazy_write; /* 403 1 */
_Bool in_save_handler; /* 404 1 */
_Bool set_handler; /* 405 1 */
/* XXX 2 bytes hole, try to pack */
zend_string * session_vars; /* 408 8 */
/* size: 416, cachelines: 7, members: 48 */
/* sum members: 404, holes: 3, sum holes: 12 */
/* last cacheline: 32 bytes */
};The CombinedLCG is a terrible RNG with a questionable API and should ideally not be used anymore. While in the case of ext/session it is only used for probabilistic garbage collection where the quality of the RNG is not of particular importance, there are better choices. Replace the RNG used for garbage collection by an ext/session specific instance of PcgOneseq128XslRr64. Its 16 Byte state nicely fits into the memory freed up by the previous reordering of the session globals struct, even allowing to the storage of the php_random_algo_with_state struct, making using the RNG a little nicer. Instead multiplying the float returned by the CombinedLCG by the GC Divisor to obtain an integer between 0 and the divisor we can just use `php_random_range` to directly generate an appropriate integer, completely avoiding the floating point maths, making it easier to verify the code for correctness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.