Skip to content

Refactor Proxy trap dispatch into a shared skeleton with lazy argument construction - #2674

Merged
lahma merged 1 commit into
sebastienros:mainfrom
lahma:proxy-dispatch-refactor
Jul 13, 2026
Merged

lahma merged 1 commit into
sebastienros:mainfrom
lahma:proxy-dispatch-refactor

Conversation

@lahma

@lahma lahma commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Restructures JsProxy so all 13 internal methods follow one uniform shape:

var trap = GetTrap(TrapX);          // revocation check + spec GetMethod, fetched fresh per op
var target = _target;               // [[ProxyTarget]] captured once, per spec
if (trap is null)
{
    return target.X(...);           // absent trap forwards, allocation-free
}
var result = CallTrap(trap, ...);   // arguments built only on this path
ValidateXTrapResult(target, ...);   // invariant checks, logic unchanged, handler-agnostic

What this buys:

  • Trap-less forwarding no longer allocates. Argument arrays, the apply/construct CreateArrayFromList array, and the defineProperty descriptor object were built before discovering the handler lacks the trap. The spec orders trap fetch before argument materialization, so building lazily is unobservable.
  • Spec fix — revoke-during-trap crash. The invariant validators used to re-read _target after the trap returned, so a trap revoking its own proxy (r.revoke() inside a set/deleteProperty/has trap) hit a NullReferenceException. Per spec, [[ProxyTarget]] is read once at the top of each internal method; validators now receive the captured target. Node 24/V8-verified regression tests added.
  • Message alignment: the revoked [[Get]] error now says Cannot perform 'get' on a proxy... (trap name) instead of the property name — character-identical to V8.
  • The invariant blocks live in Validate*TrapResult methods so trap acquisition and validation are visibly separated. This is also groundwork for a follow-up that lets CLR code implement traps directly (ProxyHandler), which will branch inside the same funnel and reuse the same validators.

Verification: full test262 suite 99,429 passed / 0 failed; ProxyTests 43/43 (3 new revoke-during-trap tests); full Jint.Tests + Jint.Tests.PublicInterface green.

ProxyBenchmark (#2666) before/after, DefaultJob:

Lane Time Allocated
ForwardGet −4.3% 4,689 KB → 1.3 KB
ForwardSet −27.4% 45,775 KB → 5,931 KB
ApplyForward −30.5% 25,143 KB → 5,611 KB
TrapSet −16.5% flat
ConstructTrap −12.7% flat
TrapGet / TrapHas / OwnKeys* / ApplyTrap −2..−7% flat or lower
RevocableCreate / RevokedTypeof flat (multi-launch verified) flat

🤖 Generated with Claude Code

https://claude.ai/code/session_01EsHuKuE4UihKZp5HYapDHE

…t construction

Every internal method now follows the same shape: GetTrap (revocation
check + spec GetMethod, fetched fresh per operation as required),
forward to target when the trap is absent, otherwise CallTrap and run
the post-trap invariant validation. The invariant blocks move into
handler-agnostic Validate*TrapResult methods, unchanged in logic.

Trap argument arrays, the apply/construct CreateArrayFromList array and
the defineProperty descriptor object are now built only when the trap
exists - the spec orders trap fetch before argument materialization, so
this is unobservable and makes trap-less forwarding allocation-free.

The validators receive the target captured before the trap call, per
spec ([[ProxyTarget]] is read once at the top of each internal method).
This fixes a latent NullReferenceException when a trap revokes its own
proxy mid-operation; V8-verified regression tests added. The revoked
[[Get]] error message now names the trap ('get') instead of the
property, matching V8 character for character.

Full test262 suite passes (99,429/0). ProxyBenchmark before/after:

| Lane           | Time    | Allocated              |
|--------------- |--------:|-----------------------:|
| ForwardGet     |   -4.3% | 4,689 KB -> 1.3 KB     |
| ForwardSet     |  -27.4% | 45,775 KB -> 5,931 KB  |
| ApplyForward   |  -30.5% | 25,143 KB -> 5,611 KB  |
| TrapSet        |  -16.5% | flat                   |
| ConstructTrap  |  -12.7% | flat                   |
| TrapGet/Has, OwnKeys, ApplyTrap | -2..-7% | flat or lower |

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EsHuKuE4UihKZp5HYapDHE
@lahma
lahma enabled auto-merge (squash) July 13, 2026 17:36
@lahma
lahma merged commit 6736f5e into sebastienros:main Jul 13, 2026
7 of 8 checks passed
@lahma
lahma deleted the proxy-dispatch-refactor branch August 23, 2026 10:00
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.

1 participant