8000 perf: extend table on env startup instead of letting zend_hash_copy do it by henderkes · Pull Request #2272 · php/frankenphp · GitHub
[go: up one dir, main page]

Skip to content

perf: extend table on env startup instead of letting zend_hash_copy do it#2272

Merged
henderkes merged 2 commits intomainfrom
perf/extend_array
Mar 12, 2026
Merged

perf: extend table on env startup instead of letting zend_hash_copy do it#2272
henderkes merged 2 commits intomainfrom
perf/extend_array

Conversation

@henderkes
Copy link
Contributor

this can potentially save us a few internal calls to zend_hash_do_resize while it loops over the source table (main_thread_env)

8 -> 16 -> 32 -> 64 -> 128 becomes 8 -> target_size (rounded to power of 2) 128.

@henderkes henderkes requested a review from AlliBalliBaba March 12, 2026 09:05
frankenphp.c Outdated
< 8000 div class="f6 py-2 tmp-px-3 border-bottom d-flex flex-justify-between">
Comment on lines 963 to 968
Copy link
Contributor

Choose a reason for hiding this comment

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

zend_hash_extend also happens in frankenphp_register_server_vars. You could probably move the zend_hash_copy to frankenphp_register_server_vars, so extending only happens once.

(go_register_server_variables just calls frankenphp_register_server_vars with the expected hash size)

@AlliBalliBaba
Copy link
Contributor

Not sure why the preload sometimes segfaults, maybe zend_hash_extend is unsafe on the main thread?

@henderkes
Copy link
Contributor Author

Not sure why the preload sometimes segfaults, maybe zend_hash_extend is unsafe on the main thread?

I don't think it's related to this PR as the segfaults are happening on multiple branches.

@AlliBalliBaba
Copy link
Contributor

Oh you're right, seems to be related to PHP 8.2. The 'build docker images' workflow wasn't triggered on the branch that added the test.

Copy link
Contributor
@AlliBalliBaba AlliBalliBaba left a comment

Choose a reason for hiding this comment

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

Not sure why opcache_preload is broken on PHP 8.2, will have a look. Probably a good thing that the test was added.

@henderkes
Copy link
Contributor Author

Yeah I'm longing for 2027 to drop 8.2 and debian buster/rhel 8 support.

@dunglas dunglas changed the title extend table on env startup instead of letting zend_hash_copy do it perf: extend table on env startup instead of letting zend_hash_copy do it Mar 12, 2026
@henderkes henderkes merged commit 2895273 into main Mar 12, 2026
88 of 95 checks passed
@henderkes henderkes deleted the perf/extend_array branch March 12, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0