Add InternalTypes.Function flag to skip is-Function and env-cast checks on hot paths#2691
Closed
lahma wants to merge 1 commit into
Closed
Add InternalTypes.Function flag to skip is-Function and env-cast checks on hot paths#2691lahma wants to merge 1 commit into
lahma wants to merge 1 commit into
Conversation
Per-call dispatch used `callable is Function` (CastHelpers.IsInstanceOfClass) to distinguish a JS function from other callables, and environment-reference resolution cast the reference base with `(Environment)` (ChkCastClass) on every global/env variable access - both class-hierarchy checks on hot paths. Add InternalTypes.Function, set once in the Function base constructor (the single writer every function object chains through; a callable Proxy derives from ObjectInstance and correctly never gets it). Two shared helpers replace the checks with a flag test plus Unsafe.As: JsValue.AsFunctionInstanceOrNull for the call/construct dispatch and Environment.FromReferenceBase for the reference-resolution sites, the latter keeping the checked cast as the mismatch fallback so a violated invariant still throws as before. The existing AsFunctionInstance now shares the same fast test. ~2% on call-dispatch-heavy loops (interleaved wall clock); neutral elsewhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Closing: clean-machine BenchmarkDotNet A/B (after removing background load) shows this is a consistent ~2-3.7% regression on call dispatch (FunctionCalls +2.0%, MethodCalls +2.8%, Fib +3.6%), not the win an earlier noisy wall-clock read suggested. On a monomorphic call site the JIT lowers \callable is Function\ to a cheap predicted type-handle compare; the flag-test + Unsafe.As replaces that with a _type load + mask + branch, which is more work. The InternalTypes.Function flag doesn't pay off for the hot dispatch check. Not merging. |
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.
Summary
Two class-hierarchy casts run on hot call/reference paths and show up in ETW as
CastHelpers:callable is Functionin per-call dispatch (JintCallExpression,Engine.Construct) — anIsInstanceOfClasson every call, distinguishing a JS function from other callables (a callable Proxy).(Environment) reference.Basein environment-reference resolution (Engine.GetValue/PutValue,JintCallExpression, delete, binding init) — aChkCastClasson every global/env variable access.Both are now flag tests.
InternalTypes.Functionis added (free bit, 2^20) and set once in theFunctionbase constructor — the single writer every function object (script/CLR/bound functions, built-in constructors) chains through. A callableProxyderives fromObjectInstance, notFunction, so it correctly never gets the flag and still dispatches through theICallablepath (verified).Two shared helpers replace the casts with
flag test + Unsafe.As:JsValue.AsFunctionInstanceOrNull()— the flag-based equivalent ofvalue as Function, used at the call/construct dispatch sites (and the existing throwingAsFunctionInstancenow shares the same fast test).Environment.FromReferenceBase(JsValue)— keyed on theObjectEnvironmentRecordflag (single-sourced in theEnvironmentctor), keeping the checked cast as the mismatch fallback so any invariant violation still throws today'sInvalidCastException.Behavior is unchanged — the flags are proven single-writer, so
Unsafe.Asis safe. The twoEnginecall sites typed asICallable(cold C#-invoke entry points, not the interpreter per-call path) are deliberately left as-is: converting them would need an interface→JsValue cast, defeating the purpose.Benchmarks
~2% on a call/reference-dispatch-heavy loop (interleaved wall clock, branch 3.20/3.14 s vs baseline 3.24 s — both branch runs below baseline); neutral on non-call-dominated workloads. The change targets the per-call
is Functionand per-reference(Environment)casts specifically; SunSpider scripts and micro-loops that don't churn calls/references are unaffected.(Absolute BDN numbers are omitted: this machine is thermally drifting ±40% run-to-run after extended benchmarking, which swamps a ~2% effect. The interleaved wall-clock A/B/A is the reliable read; the mechanism — removing the per-call/per-reference class-casts — is the justification, in the spirit of #2673.)
Gates
newconstruction, callable-Proxy and apply-trap dispatch (Function-flag discrimination),instanceof Function, recursion, built-in constructor dispatch,with/env-reference assignment and delete — all correct🤖 Generated with Claude Code