Skip to content

optimize unpack() for named fields #6958

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

Jump to bottom
Merged
merged 3 commits into from
May 14, 2021
Merged

optimize unpack() for named fields #6958

merged 3 commits into from
May 14, 2021

Conversation

kaja47
Copy link
Contributor

@kaja47 kaja47 commented May 7, 2021 •
edited
Loading

  • unpack function bypasses costly snprintf call possible (ie. case of named fields without repetition).
  • field names are allocated by zend_string_init_fast and added to the resulting hashmap by zend_symtable_update. This way there's no internal call to strlen() as in add_assoc_*.
  • numerical suffixes for small number of repetitions are hardcoded for additional speedups

As a result this PR speeds up calls like unpack('Va/Vb/Vc/Vd/Ve', $str) almost 3x.

@kaja47
optimize unpack() for named fields
7c95b78 
- unpack function bypasses costly snprintf call if possible (ie. named
  field without repetition).
- Elements are added via add_assoc_*_ex functions with explicit key length.
  This way there's no need for internal strlen call.
nikic
nikic reviewed May 7, 2021
View reviewed changes
@ramsey ramsey added the Feature label May 7, 2021
@ramsey ramsey added this to the PHP 8.1 milestone May 8, 2021
nikic
nikic reviewed May 13, 2021
View reviewed changes
@nikic nikic merged commit 21422e8 into php:master May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikic nikic nikic left review comments

Assignees
No one assigned
Labels
Feature Waiting on Author
Projects
None yet
Milestone
PHP 8.1
Development

Successfully merging this pull request may close these issues.
4 participants
@kaja47 @nikic @ramsey @krakjoe