Perf: cache function-body intervals in PropertyHooks (101x on lib/VM.php) (#23056) - #23059
Merged
Merged
Conversation
…php) (#23056) isInsideFunctionBody() decides whether `$var = …` is a local inside a method body or a hooked property declaration. findNextPropertyHookDecl probes every `$var` occurrence, and each probe walked backwards to byte 0 AND took substr($body, 0, $i) on the way — O(vars x body) with an O(body) copy per probe. It was 32% of the gen-0 rebuild profile once the php-cfg simplifier hotspot was removed (#23056): 212 of 671 samples across isInsideFunctionBody / offsetInNonCodeContext / isPromotedConstructorParamVar. Same remedy as isOffsetInComment right beside it: scan the body once into sorted disjoint intervals, then binary search. Measured on lib/VM.php: probes legacy cached 3000 0.78s 0.38s 19778 37.51s 0.37s <- all 19778 $var occurrences in the file Legacy grows ~quadratically (6.6x the probes cost 48x the time); cached is flat because the one-time build dominates and lookups are O(log intervals). The build is cheaper than the scan it replaces because braces are ~0.2x as common as `$` probes (lib/VM.php 3538 vs 19778, lib/Compiler.php 10361 vs 51153), so the per-brace prefix work happens ~5x less often, and once per body rather than once per probe. Semantics are preserved exactly, including the subtle part: only the INNERMOST enclosing brace counts, so a nested block inside a method reads as NOT inside a function body. The interval builder replicates the backward scan's boundary rule — the state established by the brace at p governs offsets [p+1, nextBracePos]. Raw byte scan like the original, so braces inside strings and comments confuse both equally; behaviour is preserved, not improved. Verified by differential test rather than by inspection — this runs during source preprocessing, so a behaviour change alters how every file in the project parses: phpunit --filter PropertyHooksFunctionBodyCacheTest OK (7 tests, 3916 assertions) comparing cached against legacy at every $var offset across lib/VM.php, lib/Compiler.php, lib/JIT/Context.php, PropertyHooks.php and PackJitHelper.php, at every brace-adjacent offset, and across class/method/nested-block/closure/return-type/by-ref shapes. PHP_COMPILER_FUNCTION_BODY_SCAN_LEGACY=1 restores the old scan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Maintainer verify (2026-07-25): |
This was referenced Jul 25, 2026
PurHur
added a commit
that referenced
this pull request
Jul 25, 2026
…3075) Pillar 1: north-star5-verify --fast step 4f-m failed (4470c186… vs live 34cc1d83…) after PropertyHooks/clone-with/Simplifier merges. Stamp with BOOTSTRAP_GEN0_ALLOW_UNVERIFIED_STAMP=1 so provenance=unverified-restamp is explicit (#22966). Honest verified-fresh rebuild remains #22717. Co-authored-by: Cursor <cursoragent@cursor.com>
4 tasks
PurHur
added a commit
that referenced
this pull request
Jul 25, 2026
…ift (#23075) (#23077) * Trust: restamp gen-0 fingerprint after #23059/#23066/#23070 drift (#23075) Pillar 1: north-star5-verify --fast step 4f-m failed (4470c186… vs live 34cc1d83…) after PropertyHooks/clone-with/Simplifier merges. Stamp with BOOTSTRAP_GEN0_ALLOW_UNVERIFIED_STAMP=1 so provenance=unverified-restamp is explicit (#22966). Honest verified-fresh rebuild remains #22717. Co-authored-by: Cursor <cursoragent@cursor.com> * Trust: restamp gen-0 fingerprint after #23076 CURLFile drift (#23075) Master moved (#23064/#23076) while the prior restamp was in flight; live fingerprint is now 94f60af8… (was 34cc1d83…). Keep provenance=unverified-restamp. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: PurHur <PurHur@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of the two quadratic scans identified in #23056.
Why
isInsideFunctionBody()decides whether$var = …is a local inside a method body or a hooked property declaration.findNextPropertyHookDeclprobes every$varoccurrence, and each probe walked backwards to byte 0 and tooksubstr($body, 0, $i)on the way — O(vars × body) with an O(body) copy per probe.It was 32% of the gen-0 rebuild profile once the php-cfg simplifier hotspot was removed (212 of 671 samples across
isInsideFunctionBody/offsetInNonCodeContext/isPromotedConstructorParamVar).What
The same remedy already applied to
isOffsetInCommentimmediately beside it: scan the body once into sorted disjoint intervals, then binary search.Measured on
lib/VM.php:$varin the file)Legacy grows ~quadratically — 6.6× the probes costs 48× the time. Cached is flat: the one-time build dominates and lookups are
O(log intervals).The build is cheaper than the scan it replaces because braces are ~0.2× as common as
$probes (lib/VM.php3538 vs 19778;lib/Compiler.php10361 vs 51153), so the per-brace prefix work happens ~5× less often — and once per body rather than once per probe. I checked that ratio before writing it, because if braces outnumbered probes the "fix" would have been slower.Correctness
This runs during source preprocessing, so a behaviour change alters how every file in the project parses. Speed is worthless if the answer moves, so it is verified by differential test rather than by inspection:
comparing cached against legacy at every
$varoffset acrosslib/VM.php,lib/Compiler.php,lib/JIT/Context.php,PropertyHooks.phpandPackJitHelper.php; at every brace-adjacent offset (where an off-by-one would hide); and across class-body / method-body / nested-block / closure / return-type / by-ref / no-brace shapes.Semantics are preserved exactly, including the subtle part: only the innermost enclosing brace counts, so a nested block inside a method reads as not inside a function body. The interval builder replicates the backward scan's boundary rule — the state established by the brace at
pgoverns offsets[p+1, nextBracePos]. It is a raw byte scan like the original, so braces inside strings and comments confuse both equally; behaviour is preserved, not improved.PHP_COMPILER_FUNCTION_BODY_SCAN_LEGACY=1restores the old scan.Not in scope
The other half of #23056 —
PHPCFG_SIMPLIFIER_USECHAIN(~3× on bootstrap compiles) — needs codegen-equivalence validation before it can be switched on for compile, since replacement order is observable in phi operand types. That one is a bigger piece of work and is deliberately untouched here.🤖 Generated with Claude Code