Replace flag-proven casts with Unsafe.As on hot arithmetic paths#2673
Merged
Conversation
ETW profiling showed CastHelpers.ChkCastClass at ~0.7% on hot interpreter paths. The JIT cannot correlate an InternalTypes flag test with the runtime type, so a plain (T) cast after such a check still emits a castclass. Apply the established Unsafe.As-after- flag-check idiom (see ObjectInstance shape-mode paths) to the enumerated hot arithmetic sites: - JintBinaryExpression: (JsNumber) operand casts in the Plus/Minus/Times/Divide double lanes, each directly dominated by left/right._type == InternalTypes.Number checks; (JsBoolean) casts on Compare(...) results in the four relational classes, guarded by the not-undefined check (Compare's return set is closed: JsBoolean.True/False or undefined on every path). - JintExpression.CompareNumber: (JsNumber) casts; sole caller Compare has proven both operands via IsNumber(), documented with a Debug.Assert precondition. - JintUnaryExpression.EvaluateMinus: (JsNumber) cast after the post-ToNumeric IsNumber() check. - JsValueExtensions.AsNumber: cast after the method's own IsNumber() guard (throw helper is [DoesNotReturn]), with a Debug.Assert beside it. AsInteger is intentionally left unchanged: it has no in-method guard and IConvertible.ToInt32 (JsValue.Convertible.cs) invokes it unguarded on arbitrary JsValues, so Unsafe.As there would turn a user-visible InvalidCastException into type confusion. The Exponentiate (JsNumber) casts are likewise skipped: their guard (AreNonBigIntOperands) only proves not-BigInt; JsNumber-ness rests on callers' ToNumeric contract rather than a local flag check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lahma
force-pushed
the
perf-unsafe-as-arith
branch
from
July 13, 2026 13:59
5eee5d9 to
d9c7882
Compare
lahma
enabled auto-merge (squash)
July 13, 2026 13:59
This was referenced Jul 15, 2026
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
1 participant
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
Idiom-consistency cleanup: the codebase's established pattern for flag-proven downcasts is
Unsafe.As<T>after anInternalTypestest (~20 sites in ObjectInstance / JintMemberExpression), but the hot arithmetic paths still used plain(T)casts that emitCastHelpers.ChkCastClass. This applies the idiom to the sites where the dominating branch provably establishes the type:(JsNumber)pairs in the Plus/Minus/Times/Divide double lanes (_type == InternalTypes.Numberon both operands)(JsBoolean)onCompare(...)results in the relational classes — the conversion is justified by a closed audit of Compare's return set (every path returnsJsBoolean.True/FalseorUndefined; comment added at each site)(JsNumber)inJintExpression.CompareNumber(sole caller guardsIsNumber()on both args;Debug.Assertprecondition added) andJintUnaryExpression.EvaluateMinusJsValueExtensions.AsNumber— its ownIsNumber()guard dominates the cast, so misuse still throws the same ArgumentExceptionDeliberately skipped (not provably safe):
Exponentiate's(JsNumber)casts — guarded only byAreNonBigIntOperands, which proves not BigInt; JsNumber-ness rests on an inter-proceduralToNumericcontract, not a same-variable checkJsValueExtensions.AsInteger— 34 of 35 call sites are flag-proven, but the publicIConvertible.ToInt32implementation (JsValue.Convertible.cs) calls it unguarded; today that's anInvalidCastException, withUnsafe.Asit would be type confusion. Left unchanged.Impact
Benchmarks on the arithmetic set (stopwatch, math-cordic, string-fasta, crypto-md5) are neutral — within the ±2% machine drift band, as expected for a change whose entire budget was 0.7% of profile self time (much of which the recent lane work already bypassed). ETW confirms the mechanism:
ChkCastClassself time 48.7 → 35.7 ms on the mixed SunSpider driver for identical work.Gates
🤖 Generated with Claude Code