-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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.
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
Fix GH-13193: Significant performance degradation in 'foreach' starting from PHP 8.2.13 (caused by garbage collection) #13265
Conversation
…rting from PHP 8.2.13 (caused by garbage collection)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks correct to me.
Zend/zend_gc.c
Outdated
|
||
ZEND_API int zend_gc_collect_cycles(void) | ||
{ | ||
int total_count = 0; | ||
bool should_rerun_gc = 0; | ||
bool did_rerun_gc = 0; | ||
|
||
zend_gc_remove_root_tmpvars(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be moved into the if
below? Preferably after if (GC_G(gc_active)) {
so that this doesn't run twice when GC isn't even active.
Edit: It would only run once since the check returns. Might be fine if it can remove some elements from the buffer I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It make sense to wrap this call with if (GC(active) && GC_G(num_roots))
.
Moving the call down is not a good idea because of rerun_gc
label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No further comments, looks good thanks.
Indeed may make sense to wrap the call.
No description provided.