Store function-local numbers unboxed in environment slot bindings#2499
Merged
Conversation
…d binding
The identifier fast path stored the null TDZ sentinel as the original left
value and computed with it, surfacing a NullReferenceException instead of
the spec-required ReferenceError for code like `{ x += 1; let x; }`.
Detect the uninitialized binding and take the Reference-based path, which
produces the proper error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ndings
Same bug class as the previous commit: UpdateIdentifier read the null TDZ
sentinel and dereferenced it, surfacing a NullReferenceException instead of
the spec-required ReferenceError for code like `{ y++; let y; }`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Binding gains a raw-double representation alongside the materialized JsValue: flags are packed into a byte and an UnboxedNumber flag marks slots whose value lives in a double field instead of a heap JsNumber. Reads stay on their existing instruction stream: the unboxed check nests inside the pre-existing null/TDZ branch (HasReferenceValue == today's IsInitialized), and a materializing read converts through JsNumber.Create with write-back caching so repeated reads observe a single instance and read-many patterns never cost more than the one allocation per write that the boxed representation already paid. TryGetNumberSlot/SetNumberSlot/FindSlotIndex expose the raw representation to the numeric read-modify-write fast paths added in a follow-up commit; nothing produces unboxed state yet, so this commit is behavior-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lane
Normal-completion values of statements are spec-observable only at
script/eval/module top level; inside function bodies only Return/Throw
completions surface. EvaluationContext.CompletionValuesObservable tracks
this (maintained by JintStatementList with save/restore: FunctionBody
lists clear it, top-level lists set it, blocks inherit), and expression
statements in unobservable frames evaluate through a new
JintExpression.EvaluateAndDiscard virtual whose default matches GetValue.
The discard lane is gated by node type at both call sites (expression
statements and for-statement updates): only update and compound
assignment expressions override it, so every other statement keeps its
exact current call sequence. Generators and async functions always use
the materializing path because suspended values flow through completions.
CompletionValueTests pin the observable cases test262 does not cover:
eval('1;2;3') inside functions, indirect eval, trailing expression values,
loops/blocks/switch/try at top level, generator/async results, class
static blocks.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ents i++ / s += <expr> statements on function-local number bindings now compute on raw doubles and store unboxed via the discard lane, eliminating the per-write JsNumber allocation for values outside the interned-int cache (double accumulators, large counters). The fast path engages only when every full-semantics trigger is absent: identifier target, no operator overloading, no generator/async frame, a slot-stored mutable binding currently holding a number, and (for compound forms) a non-logical operator. The slot location is cached per node with the same validity reasoning as JintIdentifierExpression's slot-binding cache, and nodes whose binding can never be slot-stored (global/object/ dictionary environments) permanently stop attempting, so non-qualifying code does not pay a second environment walk. The right-hand side runs exactly once; non-number results complete through ComputeCompound, the per-operator computation extracted from EvaluateInternal and shared by both lanes. IEEE 754 double division and remainder match the ECMAScript algorithms exactly, so the raw-double arms need no special cases; bitwise forms use a new double-based TypeConverter.ToInt32 with a correct range fast path. FunctionLocalNumberLoopBenchmark covers the patterns this targets (accumulators, large counters, mixed kernels, write-then-call-arg); UnboxedBindingTests pin exact semantics: -0/NaN/Infinity through materialization, int32 boundary widening, TDZ/const errors, single rhs evaluation, closure visibility, generator/async fallback and observable-context materialization. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d96685c to
c84c9bc
Compare
ToInt32(JsValue)'s range guard read `doubleVal >= -(double) int.MinValue`, i.e. >= 2147483648, combined with <= int.MaxValue — unsatisfiable — so every non-Integer operand of the bitwise/shift operators took DoubleToInt32Slow. Delegate to the corrected double-based ToInt32 instead. ToUint32(double) mirrors ToInt32(double) so raw-double callers do not need to route shift counts back through the JsValue overload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The slot-location caches walked the environment chain by reference equality
only, skipping intermediate ObjectEnvironments. A `with` object that gains a
property with the cached name after the cache was populated must shadow the
slot binding, but the cached walk kept reading - and with the new discard
fast paths, writing - the function-local slot:
function f() { var i = 0;
for (var k = 0; k < 3; k++) {
var obj = (k === 1) ? { i: 100 } : {};
with (obj) { i++; } // k==1 must update obj.i
} return i; } // was 3 (obj.i untouched), now 2
The identifier read path had the same pre-existing gap (mis-reads instead of
mis-writes); the chain walk now refuses to skip over an ObjectEnvironment in
all of them, placed after the reference-equality test so hop-0 hits pay
nothing.
The cache itself (fields, bounded walk, populate/poison policy) was hand
written in three drifting copies; it now lives in one SlotLocationCache
struct used by the update and compound-assignment fast paths, with the
identifier path sharing FindSlotIndex for population. TryGetNumberSlot gains
the same (uint) bounds guard as the identifier read so a stale cache falls
back instead of throwing, and the discard-lane shift arms use the raw double
shift count instead of reaching back through the JsValue overload.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Switch-case consequents are built as statement lists with a null statement, which the completion-value logic treated as top-level, forcing values observable and disabling expression-statement elision for everything inside a switch nested in a function. Observability is now an explicit enum on the statement list (Observable / NotObservable / Inherit) and switch consequents inherit the enclosing frame's mode. The discard gating also no longer hardcodes node types at the call sites: JintExpression.HasDiscardFastPath is the capability flag, overridden by the update/compound-assignment fast paths, and JintSequenceExpression forwards the discard to its operands so comma-separated for-loop updates (`for (...; ...; i++, j--)`) reach the fast path too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 15, 2026
This was referenced Jun 24, 2026
This was referenced Jul 6, 2026
This was referenced Jul 13, 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
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.
Numeric read-modify-write statements on function-local bindings (
s += 0.25;,i++;and friends) currently allocate aJsNumberper write once values leave the interned-int cache — double accumulators and large counters allocate on every loop iteration. This PR stores such values as raw doubles inside the environment slot and skips materialization entirely when the statement result is unobservable.The first two commits fix pre-existing
NullReferenceExceptions (instead of the spec-requiredReferenceError) for compound assignment / update expressions on uninitialized (TDZ) bindings, e.g.{ x += 1; let x; }— found by this work's test coverage and continuing the theme of #2497.Design
Unboxed slot storage —
Bindingpacks its bools into a flags byte and gains a raw-double field. A slot binding can now be in a third state: unboxed number. Reads keep their existing instruction stream — the unboxed check nests inside the pre-existing null/TDZ branch — and a materializing read converts throughJsNumber.Createwith write-back caching, so repeated reads observe one instance and read-many patterns never cost more than the one allocation per write the boxed representation already paid.TryGetNumberSlotcarries the same(uint)bounds guard as the identifier read path, so a stale cache falls back instead of throwing.Discard lane — Normal-completion values of statements are spec-observable only at script/eval/module top level. Completion-value observability is an explicit per-list mode (
Observablefor top-level script/eval/module lists,NotObservablefor function bodies,Inheritfor nested blocks and switch-case consequents), maintained byJintStatementListwith save/restore onEvaluationContext.CompletionValuesObservable. Expression statements in unobservable frames evaluate through a newJintExpression.EvaluateAndDiscardvirtual whose default matchesGetValue.The lane is gated by a
HasDiscardFastPathcapability virtual rather than hardcoded type lists: update and compound assignment expressions override it, andJintSequenceExpressionforwards the discard to its operands so comma-separated for-loop updates (for (...; ...; i++, j--)) reach the fast path too. Everything else keeps its exact current call sequence; generators/async functions always use the materializing path because suspended values flow through completions.Producers — discarded
i++/i--and numeric compound assignments compute on raw doubles and store unboxed. The fast path engages only when every full-semantics trigger is absent (identifier target, no operator overloading, no generator/async frame, slot-stored mutable number binding, non-logical operator). The right-hand side runs exactly once; non-number results complete throughComputeCompound, the per-operator computation extracted fromEvaluateInternaland shared by both lanes. IEEE 754 double division/remainder match the ECMAScript algorithms exactly, so the raw arms need no special cases.Slot-location caching — the per-node cache (environment + slot index) used by the identifier read fast path and both producers now lives in a single
SlotLocationCachestruct. The chain walk validates the cached env by reference equality with an engine-identity gate, refuses to skip over anObjectEnvironment(awithobject can gain a shadowing property at any time — the check sits after the equality test so hop-0 hits pay nothing), and permanently disables itself for nodes whose binding can never be slot-stored (global/object/dictionary environments) so non-qualifying code doesn't pay a second environment walk.Bugs found and fixed along the way
with-statement shadowing: the previous per-node slot caches (and the identifier read cache that already ships inmain) walked the env chain by reference equality only, so awithobject that gained a shadowing property after the cache was populated kept being bypassed — reads returned the stale local, and the new write fast paths updated the local slot instead of the with-object property (with (obj) { i++; }). Fixed for reads, updates and compound assignments via the shared cache; covered by regression tests.ToInt32(JsValue)fast path: its range guard readdoubleVal >= -(double) int.MinValue(i.e.>= 2147483648) combined with<= int.MaxValue— unsatisfiable — so every non-Integer bitwise/shift operand engine-wide tookDoubleToInt32Slow. It now delegates to a corrected double-basedToInt32; aToUint32(double)twin keeps raw-double shift counts off theJsValuepath.Benchmarks
Paired back-to-back runs on the same machine (BenchmarkDotNet defaults,
[MemoryDiagnoser]), final branch head. NewFunctionLocalNumberLoopBenchmarkcovers the targeted patterns, which the existing suite under-covers because its loops live at script top level where bindings are global-object properties:Regression watch (same protocol):
¹ node-size growth (the slot-location cache on two expression types), visible only because this benchmark re-parses per execution.
² these two top-level 1000-iteration tight loops bounce ±3–4% between runs of identical code on this machine; the deltas sit inside that band (earlier pairs measured +1.7…+2.2%). The mechanical overhead is the for-update discard detour (~2 ns/iteration).
³ noisy classes: stopwatch-modern's own base varies ±6.5% across runs and ObjectString carries ±7–14 ms StdDev in both builds; the controlled micro equivalents of their hot shapes (ForLet, Stopwatch classic) show no regression. Unprepared Cube cases are bimodal (≈15.3 ↔ ≈18.5 ms) in both builds across process restarts and cannot resolve small deltas.
⁴ slot
Bindinggrew 16 → 24 bytes; env-churn-heavy code shows it as allocated bytes while time stays within noise.Validation
Jint.Testsgreen on net10.0 and net472, including newUnboxedBindingTests(-0/NaN/Infinity through materialization, int32 boundary widening, TDZ/const errors, sloppy-mode semantics, single rhs evaluation, closure visibility, generator/async fallback, observable-context materialization, dynamicwithshadowing for reads/updates/compound assignments, comma-sequence updates, switch bodies) andCompletionValueTests(eval/top-level completion-value semantics test262 does not cover)Jint.Tests.PublicInterfacegreen (no public API change)🤖 Generated with Claude Code