Compress interned string table offsets and increase maximum supported buffer size #13676
Conversation
This file contains 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
… buffer size The interned string buffer is organized as a header + a hash table + a zend_string arena. Hash slots point to the arena, but are represented as 32bit offsets from the buffer, which limits the maximum buffer size to about 4GiB. However zend_strings are 8-byte aligned in the buffer, so we can compress the 3 lower bits. This allows to increase the maximum supported interned string buffer size from 4095 MiB to 16367 MiB.
arnaud-lb
commented
Mar 11, 2024
| ZEND_MM_ALIGNED_SIZE_EX(_ZSTR_HEADER_SIZE + ZSTR_LEN(s) + 5, 8) | ||
| ZEND_MM_ALIGNED_SIZE_EX(_ZSTR_STRUCT_SIZE(ZSTR_LEN(s)) + sizeof(zend_string_table_pos_t), ZEND_STRING_TABLE_POS_ALIGNMENT) |
There was a problem hiding this comment.
5 here accounted for the null byte after ZSTR_LEN(s) + a slot for the collision chain.
|
The patch looks good.
|
Girgias
reviewed
Mar 12, 2024
| /* nTableMask must not overflow (uint32_t) */ \ | ||
| UINT32_MAX / (32 * 1024 * sizeof(zend_string_table_pos_t)) \ | ||
| ), \ | ||
| /* SHM allocation must not overlow (size_t) */ \ |
There was a problem hiding this comment.
Nit:
Suggested change
| /* SHM allocation must not overlow (size_t) */ \ | |
| /* SHM allocation must not overflow (size_t) */ \ |
|
@dstogov this is related to #13598. I'm assuming a shared hosting use-case Symfony demo benchmarks: |
dstogov
approved these changes
Mar 13, 2024
|
Yea, like you wrote, this will solve a lot of "problems" (use cases) on shared webhostings. Thank you for this PR. Btw, I am little surprised no one else reported this issue... Maybe nobody cares :)) But our company openservis.cz cares, so we are trying to make best performance for our customers, so, we cares :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The interned string buffer is organized as a header + a hash table + a zend_string arena. Hash slots point to the arena, but are represented as 32bit offsets from the buffer, which limits the maximum buffer size to about 4GiB. However zend_strings are 8-byte aligned in the buffer, so we can compress the 3 lower bits. This allows to increase the maximum supported interned string buffer size from 4095 MiB to 32767 MiB.