Stop a chain of binds copying the whole bound name at every level - #4130
Merged
Merged
Conversation
SetFunctionName composed step 4's "prefix name" with a CLR string concatenation, which flattens whatever it is handed. Binding an already-bound function therefore materialized "bound bound ... f" afresh at every level and left that copy in the level's own name descriptor, so N binds retained about 3*N^2 characters: at a depth of 100,000 that is roughly 30 GB and 37 s, which is what exhausted the 7 GB Linux CI runners in sebastienros#4078's run. The prefixed name is now a JsString.Concat node over the level below -- O(1) to build, flattened once if something ever reads the text -- so the same chain allocates 33 MB and completes in about 0.3 s. Every observable name is byte-identical: JsString.Concat copies below its deferral threshold and produces the same characters above it, and the bind path never forces a flatten (the guard compares against JsString.Empty, which answers on the O(1) length, and IsString is a type-flag read). Fixes sebastienros#4129 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
lahma
added a commit
that referenced
this pull request
Sep 24, 2026
…ead of recursing (#4165) A chain of bound functions is a linked list script builds in linear time (#4130). BindFunction.IsConstructor asked BoundTargetFunction.IsConstructor and GetFunctionRealm called itself for every [[BoundTargetFunction]] and [[ProxyTarget]] link - one native frame per link with no stack probe - so on tier-0 code a chain of ~16,000 (IsConstructor) or ~12,800 (GetFunctionRealm) links on a 1 MB thread ended the process through `new f()`, `Reflect.construct`, `class extends`, `super()`, `new Proxy(f, h)`, an array species constructor or a ShadowRealm call, before any probed [[Call]]/[[Construct]] ran. Neither question observes anything on the way, so both are loops now and the answer no longer depends on whether the JIT turned the call into a jump. Follow-up to #4076 / #4078. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
lahma
added a commit
that referenced
this pull request
Sep 24, 2026
…d GetFunctionRealm instead of recursing (#4169) Backport of PR #4165 (commit 8a1aad7) from main. BindFunction.IsConstructor asked BoundTargetFunction.IsConstructor, and GetFunctionRealm called itself for every [[BoundTargetFunction]] and [[ProxyTarget]] link - one native frame per link with no stack probe - so on net8.0/net10.0, where tier-0 code is not tail-called, a chain of ~16,000 (IsConstructor) or ~12,800 (GetFunctionRealm) links on a 1 MB thread ended the process through `new f()`, `Reflect.construct`, `class extends`, `super()`, `new Proxy(f, h)`, an array species constructor or a ShadowRealm call, before any probed [[Call]]/[[Construct]] ran. Neither question observes anything on the way, so both are loops now. The StackOverflowGuard probes #4007 brought to 4.x sit in BindFunction's and JsProxy's [[Call]] and [[Construct]], not on these two walks, so an engine that opted into the guard was not protected either. Adapted for 4.x: - Jint/Native/Function/BindFunction.cs: main's walk, verbatim. The doc comment drops main's "(#4130 made building one linear)", which is not true on 4.x. - Jint/Native/Function/Function.cs: the same loop, keeping 4.x's test order (Function, then BindFunction, then JsProxy). Main tests BindFunction first because #3658 made BindFunction derive from Function there; on 4.x BindFunction and JsProxy both derive from ObjectInstance, so the order is free and main's "Step 2 before step 3" comment is replaced by one that says so. - Jint.Tests/Runtime/FunctionTests.cs: the three cases, [Test] -> [Fact]. TheRealmOfAChainOfBoundFunctionsAndProxiesIsItsInnermostTargets creates the other realm with engine._host.CreateRealm(), the pattern this file already uses, in place of main's Test262Object/$262.createRealm(), which Jint.Tests on 4.x does not have. - Jint.Tests.PublicInterface/BoundFunctionChainWalkTests.cs: new, [Test] -> [Fact]; routes, depths (200,000 bound links, 100,000 proxy+bind pairs), the 1 MB thread and the expected outcomes are main's. Each chain step also runs `delete f.name`: 4.x lacks #4130, so bind writes "bound " + the target's name eagerly and an undeleted chain retains names quadratic in its depth (measured: 370 MB of managed heap at 8,000 links; the ~16,000 links a 1 MB thread needs is past a 1 GB heap). With no own name the next bind reads Function.prototype's empty one, so the build is linear (200,000 links in ~0.5 s, ~50 MB) and the chain is the same chain of BindFunctions. Main's test has no NETFRAMEWORK carve-out and none is needed: on .NET Framework x64 the JIT tail-calls both recursions, so the cases pass there before and after the fix. Evidence (-c Release, freshly built): - Unfixed 4.x, net10.0: each BoundFunctionChainWalkTests case, run alone, kills the test host - "Stack overflow." with BindFunction.get_IsConstructor() repeated 15,998 / 16,003 times (the guarded and unguarded bound-chain cases) and Function.GetFunctionRealm repeated 12,807 times (the mixed chain); under dotnet test that surfaces as "[FATAL ERROR] Xunit.Sdk.TestPipelineException" and no results. net472: 3/3 pass (tail-called). The three FunctionTests cases pass on both TFMs before the fix (72/72 FunctionTests): they pin that the walk kept the answer. - Unfixed 4.x is reachable without an expensive chain: with StackOverflowGuard on, a script that recurses until the guard's RangeError and, in the catch at that depth, evaluates `new Proxy(f, {})` over a plain 2,000-link bind chain (24 MB, built in ~0.25 s) ends the process (get_IsConstructor repeated ~1,800 times); a plain 20,000-link proxy chain (4 MB) as an array's constructor ends it in GetFunctionRealm (12,794 frames). With the fix both answer. - With the fix: BoundFunctionChainWalkTests 3/3 and FunctionTests 72/72 on net10.0 and net472. - Full solution dotnet test -c Release: 0 failures. Jint.Tests 7616/0/4 (net10.0), 7531/0/4 (net472); Jint.Tests.PublicInterface 1877/0/9 (net10.0), 1869/0/9 (net472), public-API Verify snapshots unchanged; Jint.Tests.CommonScripts 28/0/0 on both; Jint.Tests.SourceGenerators 52/0/0. (passed/failed/skipped) - test262 (4.x pin), from that same run: 102,501 passed / 0 failed / 183 skipped, identical to the 4.x control; no load flakes in this run. - JINT_HOST_CONTRACT_VERIFICATION=1: Jint.Tests 7616/0/4 (net10.0), 7531/0/4 (net472); Jint.Tests.PublicInterface 1881/0/5 (net10.0), 1873/0/5 (net472). Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.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.
Fixes #4129. Base: 513b0b7.
Function.SetFunctionNamecomposed step 4 ofSetFunctionName with a CLR string concatenation:
string + JsValueflattens whatever it is handed, so binding an already-bound function materialized"bound bound … f"afresh at every level and left that copy in that level's own_nameDescriptor. Achain of N binds therefore retained Σ 6·i characters — about 30 GB at a depth of 100,000, which is what
took the 7 GB Linux runners down mid-test in #4078's run.
The prefixed name is now a
JsString.Concatnode over the level below: O(1) to build, flattened once ifsomething ever reads the text. The three prefixes the engine ever passes (
bound,get,set) arecached with their separating space, so the common case concatenates two values it already holds.
The bind path never forces a flatten
Checked level by level, because one forced flatten per level would have kept the whole thing quadratic:
SetFunctionName's own guard,UnwrapJsValue(nameDescriptor) != JsString.Empty— a freshBindFunctionhas no name descriptor at all, so the guard short-circuits before the comparison; and
JsString.Equalscompares
Lengthfirst, which every representation answers without materializing.FunctionPrototype.Bind'stargetName.IsString()— anInternalTypesflag read.new PropertyDescriptor(name, PropertyFlag.Configurable)— stores the value, nothing else.oi.Get(CommonProperties.Name)at the next level — hands the sameJsValueback.JsString.Concatreads both lengths, andRopeString.Lengthis a field.BindFunction.IsConstructor(BoundTargetFunction.IsConstructor, a walk down the chain per read) isnot reached during
bind:BoundFunctionCreateonly reads the target's[[Prototype]], and thelength/namecopies never ask. It stays O(depth) per read —new f()on a deep chain pays it — butthat is not on the path this issue is about, and the measurement below confirms nothing quadratic is left.
Measured
One engine per row, on this branch and on its base,
Stopwatcharound a singleEvaluateplusMemoryLimitConstraint.AllocatedBytesfor the allocation; net10.0, Release, Windows. Not a BenchmarkDotNetrun — it is the shape of the issue's own measurement, three orders of magnitude apart:
Allocation before: 9.0× for 3× the depth. After: 3.0×. The 100,000 row was not run unfixed — this box has
32 GB, and the issue already measured it.
The regression test
Jint.Tests/Runtime/BoundFunctionNameTests.csbuilds the 100,000-deep chain, then asserts the name(600,006 characters, compared against
'bound '.repeat(100000) + 'target'built in script),f.length,and that a bound function taken from the chain still answers when called.
The wall clock is not the assertion — nothing here asserts a duration. The wedge is an allocation
ceiling of 768 MB (
LimitMemory), against a healthy run's 33 MB: a per-level copy trips it inside thefirst few thousand levels, so the quadratic is reported as a failed test in under a second instead of as
an exhausted machine. Raising that ceiling by an order of magnitude would still catch it.
Against the unfixed tree it fails on every framework —
MemoryLimitExceededException : Script has allocated 805392952 but is limited to 805306368at 400 ms(net8.0), 429 ms (net10.0), 677 ms (net472). Fixed, it passes at 298 ms / 353 ms / 502 ms.
Six
[TestCase]rows beside it pin the text itself —bound f,bound bound bound f, the trailing-spaceboundof an anonymous target,get x,set x,get [Symbol.iterator]— and they pass identically onboth trees, which is the point: the characters do not change.
JsString.Concatcopies below its deferralthreshold and produces the same text above it.
Verification
dotnet build -c Release: 0 errors, 0 C# warnings.Jint.Tests: net8.0 12,593 (0 failed, 5 skipped), net10.0 12,593 (0 failed, 5 skipped), net472 8,676(0 failed, 4 skipped); 33,862 combined.
Jint.Tests.PublicInterface: 10,454 total, 0 failed, 63 skipped.JINT_HOST_CONTRACT_VERIFICATION=1: 33,862 / 0 failed and 10,454 / 0 failed.Jint.Tests.Test262: 102,720 total, 102,611 passed, 0 failed, 109 skipped.One thing deliberately left alone
ObjectInstance.SetFunctionNamecarries the sameprefix + " " + nameline. Nothing reaches its prefixbranch today — every prefixed call site (
FunctionPrototype.Bind,ClassDefinition,JintObjectExpression,ShadowRealm.CopyNameAndLength) has aFunctionreceiver and binds to theoverride above — and
ObjectInstance.cshas other work in flight, so it is untouched here.🤖 Generated with Claude Code