Skip to content

Zend guard recursion protection #11812

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

Closed
wants to merge 8 commits into from
Closed

Conversation

bukka
Copy link
Member

@bukka bukka commented Jul 28, 2023

This PR introduces a new way of recursion protection in JSON, var_dump and friends. This is alternative solution for what was done in #10020 and specifically implements the solution suggested by Dmitry in #10020 (comment) . More info can be find in that PR so I will not repeat it all here. As mentioned there, it fixes issue in master for __debugInfo and also improves perf for jsonSerializable in some cases.

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

I think this approach should work.
@iluuu1994 could you please also check the patch.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

I see a tiny slowdown in Symfony Demo of <0.01%. I'm not sure if that's enough to get to the bottom of the direct cause.

@bukka
Copy link
Member Author

bukka commented Aug 1, 2023

In terms of the benchmark, it might be a bit tricky to measure because current master is buggy in this behaviour and there is a regression (e.g. you can no longer do var_export in __debugInfo - not a huge deal but still regression as you could do that in 8.2). So you would need to revert all previous commits done by Tyson and measure against that instead. This is a regression fix with addition of that JSON improvemets.

@bukka bukka force-pushed the zend_guard_recursion branch from 530e122 to 82b328f Compare August 1, 2023 10:53
@bukka bukka force-pushed the zend_guard_recursion branch from 82b328f to fc718a8 Compare August 24, 2023 10:10
@bukka bukka closed this in 53aa53f Aug 24, 2023
jorgsowa pushed a commit to jorgsowa/php-src that referenced this pull request Aug 24, 2023
This PR introduces a new way of recursion protection in JSON, var_dump
and friends. It fixes issue in master for __debugInfo and also improves
perf for jsonSerializable in some cases. More info can be found in
phpGH-10020.

Closes phpGH-11812
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants