ext/intl: Pre-size arrays created from ICU enumerations#22483
Conversation
|
|
||
| /* Traverse it, filling the return array. */ | ||
| array_init( return_value ); | ||
| array_init_size( return_value, count > 0 ? count : 0 ); |
There was a problem hiding this comment.
I do not think you need this ternary expression count is 0 otherwise.
|
lgtm minus the nit ; here since these are enumerations, uenum_count should not be slow but it s not a huge win neither. |
There was a problem hiding this comment.
a. The min size of hashtable in Zend is 8. So array_init won't grow if the size is smaller than 8. That makes your optimization in locale_get_keyword not that much useful (it could even be slower)
b. please run a benchmark if you think this optimize codes. It is not so obvious because uenum_count can be slow either (as David mentioned). See: https://raw.githubusercontent.com/unicode-org/icu/main/icu4c/source/common/unicode/uenum.h
c. I don't love the additional complexity. (But it is good if this do speed-up things :) )
f861cc2 to
4f580c7
Compare
4f580c7 to
644aacd
Compare
|
Use falls back to an unsized array and keeps the existing iteration behavior.
Local ICU-level benchmark numbers, using PHP 8.5.6RC3 + ICU 77 through FFI: |
Use
uenum_count()to pre-size arrays built from ICU enumerations.This avoids unnecessary HashTable growth in
transliterator_list_ids(),resourcebundle_locales()andlocale_get_keywords().locale_get_keywords()also uses the keyword length reported by ICU wheninserting the associative entry, with a fallback to
strlen()for unknownlengths.
If counting fails, the previous behavior is preserved by falling back to an
unsized array initialization.