Repository navigation
Strings: a long + defers its copy, so s = s + x is linear like s += x - #3386
Merged
Merged
Conversation
… += x` `s += x` and `s = s + x` mean the same thing and did not cost the same thing. The compound form builds into `JsString.ConcatenatedString`, which is `StringBuilder`-backed and amortised linear; a plain `+` went through `ApplyAdditionToPrimitives`, which coerced both operands to `string` and returned `JsString.Create(string.Concat(left, right))` — so every iteration of an accumulator loop copied the whole left operand. Prepending (`s = x + s`) had no fast path at all, because the compound form cannot express it. Reported as sebastienros#3350; the shape had never been in the benchmark suite, which is why a rope evaluated against it in July 2026 came out a NO-GO for what it measured. `+` cannot simply reuse `ConcatenatedString`: that class is mutated in place, and `+=` may do so only because the assignment replaces the receiver. A non-assignment `+` leaves both operands reachable from wherever they were read, so the deferred form it can use has to be immutable — which is also what makes it work for prepend. `JsString.RopeString` is that form: two `JsString` operands and a total length, flattened once on the first read that needs characters and memoized, with both references released at that point so a flattened value stops retaining its tree. `JsString.Concat` builds one when the result reaches 512 characters and concatenates flat below that, and snapshots an operand that is a `ConcatenatedString` so a later `+=` on it cannot change content behind the node's reader. `Length` — and so truthiness and the length comparison string equality performs first — is answered from the node; everything else flattens, `this[int]` included and deliberately, since descending the tree per character would be a new quadratic. Depth is not capped: a cap would re-copy the accumulator every N operations, which is the quadratic again with a smaller constant. The stack overflow a cap would have prevented is removed at its source instead — the flatten walk is iterative over a heap array, and descends right-first so the append shape needs one entry. The flattened `a + b + c` chain gets the same treatment, so `s = s + a + b` is linear too, and the length guard still runs on the summed lengths before anything is built. `+=` is untouched. Also adds the four missing benchmark lanes — AssignAppendSmallChunks, AssignPrependSmallChunks, AssignChainThree, AssignAppendThenScan — which are the shapes the suite could not see. They are not measured here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
This was referenced Aug 26, 2026
lahma
added a commit
that referenced
this pull request
Sep 1, 2026
…on it never takes (#3571) #3386 gave a long `+` a deferred copy, and SunSpider's date-format-xparb row paid 3.9% for it. The cutoff that was supposed to keep short concatenations out of that trade was already there and already working — `JsString.MinDeferredConcatenationLength`, 512 characters, honoured by both the pairwise `+` and the flattened chain — and xparb's chains are ten to thirty-five characters, so they were on the eager side of it the whole time. What they paid for was not the deferral. It was the shape of the eager path. Two things had moved onto it. The chain lane coerced every operand with `TypeConverter.ToJsString` before the cutoff was read, which allocates a `JsString` wrapper for every operand that is not already a string — a number, which is most of what a formatted date is made of — and the eager join then unwrapped every one of them again. And `ConcatMany` built a `string[]` to hand to `string.Concat` on top of the `JsString[]` the operands already lived in, so a chain allocated two arrays where the pre-#3386 lane allocated one. For xparb's fifteen-operand long-format chain that is a second 144-byte array plus a wrapper per numeric operand, all of it dead before the result was copied out: the doubled variable-size allocations and the +23% allocation events the profile in #3527 measured under the chain node. An operand is now carried in whichever form its coercion already produced — the `JsString` it was, whose representation has to survive because flattening it is the copy the deferred lane exists to avoid, or the plain text a non-string primitive coerced to. Both answer their length without materializing anything, which is all the cutoff is decided on, so nothing is given up by holding the cheaper form until the decision is made. Below the cutoff the result is then joined through a `ValueStringBuilder` over a cutoff-sized stack buffer, which the assembly's `SkipLocalsInit` leaves unzeroed, so there is no second array and no wrapper. Above it the fold is unchanged. The pairwise `+` gets the same treatment from the other end: two operands that are already strings — a `+` between two string expressions — now take a branch that coerces nothing at all, and the mixed pair (`n + "x"`) coerces to text and builds the wrapper only if it goes on to defer. `+=` and `String.prototype.concat` are untouched and exempt: both build `JsString.ConcatenatedString`, the mutable builder, and never enter the deferred representation at all. Bytes per evaluation of an eleven-operand short chain, as a delta of a thread-local allocation counter: 576 to 272 on net10.0 and net8.0, 712 to 296 on net472. `AShortChainDoesNotPayForTheDeferredRepresentation` pins it at 400, between the two with a third of the distance on either side; the asymptotic guarantee #3386 exists for is pinned as before by `AccumulatingWithPlusIsNoLongerQuadratic`, now including the five-operand chain shapes that reach `ConcatMany`. Fixes #3527. Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
Merged
lahma
added a commit
to lahma/jint
that referenced
this pull request
Oct 5, 2026
…y, so `s = s + x` is linear like `s += x` Backport of PR sebastienros#3386 (commit 9d80990) from main. `s += x` and `s = s + x` mean the same thing and did not cost the same thing. The compound form builds into `JsString.ConcatenatedString`, which is `StringBuilder`-backed and amortised linear; a plain `+` coerced both operands to `string` and returned `JsString.Create(string.Concat(left, right))`, so every iteration of an accumulator loop copied the whole left operand, and prepending (`s = x + s`) had no fast path at all. `JsString.RopeString` is an immutable deferred form: two operands and a total length, flattened once on the first read that needs characters, memoized, and with both operand references released at that point. `JsString.Concat` builds one when the result reaches 512 characters and concatenates flat below that. `Length` - and so truthiness and the length comparison string equality performs first - is answered from the node; everything else flattens, `this[int]` included, since descending per character would be a new quadratic. The flatten walk is iterative over a heap array and descends right-first, so depth is not capped and cannot overflow the stack. The flattened `a + b + c` chain gets the same decision, so `s = s + a + b` is linear too, and the MaxLength guard still runs on the summed lengths before anything is built. `+=` is untouched. Also adds the four Assign* lanes to StringConcatLargeBenchmark. Adapted for 4.x: - JsString.cs, class remarks (conflict): main rewrote the subclassing paragraphs around LazyJsString, which is v5's sebastienros#3340 and absent here. Kept 4.x's two paragraphs and their contract; only the list of Jint's own representations gains the deferred one. - JsValueExtensions.cs (conflict; the file is Jint/JsValueExtensions.cs on this branch): the "InternalTypes.String is set only by" list gains RopeString, without main's LazyJsString; 4.x's `IsString` cref is kept. - LazyJsString.cs (absent): main's hunk there is a comment. It adds RopeString to the list of Jint's own lazy strings that are built on JsString directly rather than on LazyJsString, which is what lets LazyJsString's declared- length verification run with no out-of-assembly gate. The invariant behind it is one a node depends on: it answers its own Length as the sum of its operands' Lengths, sizes the flatten buffer from that sum and writes each operand's text at an offset computed from it, so every operand's Length has to be the length of its text. On main a host string is a LazyJsString whose declared length host-contract verification checks against what it produces. On 4.x the same role is played by a host's direct JsString subclass - the documented subclassing contract, pinned by LazyHostStringTests and by RavenApiUsageTests' CustomString - whose Length is its own unverified claim. So main's `Immutable` (snapshot a ConcatenatedString, hold everything else) becomes `IsRetainable`: a node holds only Jint's own immutable representations - an exact JsString, a SlicedString or a RopeString - and a ConcatenatedString or a host subclass is snapshotted at the `+`, with the node's length summed from what it actually holds. That keeps a host's ToString() at the `+`, where 4.x has always called it, and keeps a host whose Length disagrees with its text producing that text. The two tests added to Jint.Tests.PublicInterface's LazyHostStringTests pin both; run against main's rule verbatim, all three cases fail - the host is not materialized at the `+` (count 0, expected 1), a host reporting 5 for "ab" flattens to "\0\0\0ab...", and one reporting 3 for "abcd" throws ArgumentOutOfRangeException out of RopeString.CopyInto. The accumulator a loop builds is always Jint's own after its first deferred `+`, so the snapshot does not reintroduce a copy per iteration. - README.md (conflict): main's hunk edits the "Lazy strings" section, which this branch's README does not have, and this README never described `s = s + x` as quadratic or recommended `+=` over `+`; dropped. - docs/v5-migration.md: absent on this branch; dropped. - StringConcatenationTests: this branch's GCPolyfills has no AllocatedBytesForCurrentThreadIsSupported (main's sebastienros#3036); the guard asks TryGetAllocatedBytesForCurrentThread instead. The file was already xUnit on main at sebastienros#3386. Evidence - the ported tests against unfixed 4.x (engine files at upstream/4.x plus a never-committed compile stub declaring RopeString and the cutoff constant), net10.0 and net472 alike: in StringConcatenationTests, StringRepresentationKeyTests and StringLengthLimitTests 32 fail and 55 pass. Three of the failures are the asymptotic claim itself - AccumulatingWithPlusIsNoLongerQuadratic allocates 640,758,656 B (`s = s + chunk`), 640,744,128 B (`s = chunk + s`) and 1,281,226,960 B (`s = s + chunk + chunk`) for 8,000 iterations against a 16 MB bound; the other 29 stop at the "is a RopeString" premise assertion. The 55 passes pin what must not change (characters, order, keys, the existing rows of both older classes). AVeryDeepTreeFlattensWithoutRecursing (4x10^10 characters copied at 200,000 iterations without the fix) and ConcatenationPastTheLimitIsACatchableRangeError (about 1.3 GB of flat strings without the fix) were not run against unfixed code on this shared machine. With the package all of them pass on both frameworks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs
lahma
added a commit
to lahma/jint
that referenced
this pull request
Oct 5, 2026
…er pays for a deferred representation it never takes Backport of PR sebastienros#3571 (commit ef706d7) from main. sebastienros#3386 cost SunSpider's date-format-xparb 3.9% on main (sebastienros#3527), and not because of the deferral: xparb's chains are ten to thirty-five characters, far below the 512-character cutoff. What moved onto the eager path was its shape. The chain lane coerced every operand with TypeConverter.ToJsString before the cutoff was read, allocating a JsString wrapper per non-string operand, and ConcatMany built a string[] on top of the JsString[] the operands already lived in. An operand is now carried in whichever form its coercion produced - the JsString it already was, or the plain text a non-string primitive coerced to - both of which answer their length without materializing anything, and below the cutoff the result is joined through a ValueStringBuilder over a cutoff-sized stack buffer: no second array and no wrapper. Above the cutoff the fold through JsString.Concat is unchanged. The pairwise `+` gets the same treatment: two string operands take a branch that coerces nothing, and the mixed pair (`n + "x"`) builds the wrapper only if it goes on to defer. TypeConverter.ToStringNonString becomes internal for that. `+=` and String.prototype.concat are untouched. Adapted for 4.x: - Engine hunks (JsString.cs, JintBinaryExpression.cs, TypeConverter.cs) apply verbatim; ValueStringBuilder and the assembly's SkipLocalsInit are already on this branch. - StringConcatenationTests: transcribed from NUnit to xUnit v3 ([Test] to [Fact], [TestCase] to [Theory] with [InlineData]); the allocation guard asks TryGetAllocatedBytesForCurrentThread, as in the previous commit. Evidence: bytes per evaluation of the eleven-operand short chain (y + '-' + m + '-' + d + ' ' + y + ':' + m + ':' + d), measured as a delta of the thread-local allocation counter over 20,000 evaluations - unfixed 4.x 272 on net10.0 and 408 on net472; with this package 272 and 296, the same after-figures main measured. So a short chain now costs no more than it did on 4.x before sebastienros#3386, and 27% less on net472. The ported tests against unfixed 4.x: the eleven new cases fail 3 on net10.0 and 4 on net472 - AccumulatingWithPlusIsNoLongerQuadratic's two five-operand rows (2,562,225,160 and 2,562,134,112 B for 8,000 iterations against a 16 MB bound), APairWithOneCoercedOperandTakesBothSidesOfTheCutoff at its "is a RopeString" premise, and on net472 AShortChainDoesNotPayForTheDeferredRepresentation at 408 B against its 400 B ceiling. The seven AChainMixingCoercedAndStringOperandsProducesTheSameCharacters rows pass, pinning characters that must not change. With the package all pass on both frameworks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs
lahma
added a commit
to lahma/jint
that referenced
this pull request
Oct 5, 2026
…y, so `s = s + x` is linear like `s += x` Backport of PR sebastienros#3386 (commit 9d80990) from main. `s += x` and `s = s + x` mean the same thing and did not cost the same thing. The compound form builds into `JsString.ConcatenatedString`, which is `StringBuilder`-backed and amortised linear; a plain `+` coerced both operands to `string` and returned `JsString.Create(string.Concat(left, right))`, so every iteration of an accumulator loop copied the whole left operand, and prepending (`s = x + s`) had no fast path at all. `JsString.RopeString` is an immutable deferred form: two operands and a total length, flattened once on the first read that needs characters, memoized, and with both operand references released at that point. `JsString.Concat` builds one when the result reaches 512 characters and concatenates flat below that. `Length` - and so truthiness and the length comparison string equality performs first - is answered from the node; everything else flattens, `this[int]` included, since descending per character would be a new quadratic. The flatten walk is iterative over a heap array and descends right-first, so depth is not capped and cannot overflow the stack. The flattened `a + b + c` chain gets the same decision, so `s = s + a + b` is linear too, and the MaxLength guard still runs on the summed lengths before anything is built. `+=` is untouched. Also adds the four Assign* lanes to StringConcatLargeBenchmark. Adapted for 4.x: - JsString.cs, class remarks (conflict): main rewrote the subclassing paragraphs around LazyJsString, which is v5's sebastienros#3340 and absent here. Kept 4.x's two paragraphs and their contract; only the list of Jint's own representations gains the deferred one. - JsValueExtensions.cs (conflict; the file is Jint/JsValueExtensions.cs on this branch): the "InternalTypes.String is set only by" list gains RopeString, without main's LazyJsString; 4.x's `IsString` cref is kept. - LazyJsString.cs (absent): main's hunk there is a comment. It adds RopeString to the list of Jint's own lazy strings that are built on JsString directly rather than on LazyJsString, which is what lets LazyJsString's declared- length verification run with no out-of-assembly gate. The invariant behind it is one a node depends on: it answers its own Length as the sum of its operands' Lengths, sizes the flatten buffer from that sum and writes each operand's text at an offset computed from it, so every operand's Length has to be the length of its text. On main a host string is a LazyJsString whose declared length host-contract verification checks against what it produces. On 4.x the same role is played by a host's direct JsString subclass - the documented subclassing contract, pinned by LazyHostStringTests and by RavenApiUsageTests' CustomString - whose Length is its own unverified claim. So main's `Immutable` (snapshot a ConcatenatedString, hold everything else) becomes `IsRetainable`: a node holds only Jint's own immutable representations - an exact JsString, a SlicedString or a RopeString - and a ConcatenatedString or a host subclass is snapshotted at the `+`, with the node's length summed from what it actually holds. That keeps a host's ToString() at the `+`, where 4.x has always called it, and keeps a host whose Length disagrees with its text producing that text. The two tests added to Jint.Tests.PublicInterface's LazyHostStringTests pin both; run against main's rule verbatim, all three cases fail - the host is not materialized at the `+` (count 0, expected 1), a host reporting 5 for "ab" flattens to "\0\0\0ab...", and one reporting 3 for "abcd" throws ArgumentOutOfRangeException out of RopeString.CopyInto. The accumulator a loop builds is always Jint's own after its first deferred `+`, so the snapshot does not reintroduce a copy per iteration. - README.md (conflict): main's hunk edits the "Lazy strings" section, which this branch's README does not have, and this README never described `s = s + x` as quadratic or recommended `+=` over `+`; dropped. - docs/v5-migration.md: absent on this branch; dropped. - StringConcatenationTests: this branch's GCPolyfills has no AllocatedBytesForCurrentThreadIsSupported (main's sebastienros#3036); the guard asks TryGetAllocatedBytesForCurrentThread instead. The file was already xUnit on main at sebastienros#3386. Evidence - the ported tests against unfixed 4.x (engine files at upstream/4.x plus a never-committed compile stub declaring RopeString and the cutoff constant), net10.0 and net472 alike: in StringConcatenationTests, StringRepresentationKeyTests and StringLengthLimitTests 32 fail and 55 pass. Three of the failures are the asymptotic claim itself - AccumulatingWithPlusIsNoLongerQuadratic allocates 640,758,656 B (`s = s + chunk`), 640,744,128 B (`s = chunk + s`) and 1,281,226,960 B (`s = s + chunk + chunk`) for 8,000 iterations against a 16 MB bound; the other 29 stop at the "is a RopeString" premise assertion. The 55 passes pin what must not change (characters, order, keys, the existing rows of both older classes). AVeryDeepTreeFlattensWithoutRecursing (4x10^10 characters copied at 200,000 iterations without the fix) and ConcatenationPastTheLimitIsACatchableRangeError (about 1.3 GB of flat strings without the fix) were not run against unfixed code on this shared machine. With the package all of them pass on both frameworks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs
lahma
added a commit
to lahma/jint
that referenced
this pull request
Oct 5, 2026
…er pays for a deferred representation it never takes Backport of PR sebastienros#3571 (commit ef706d7) from main. sebastienros#3386 cost SunSpider's date-format-xparb 3.9% on main (sebastienros#3527), and not because of the deferral: xparb's chains are ten to thirty-five characters, far below the 512-character cutoff. What moved onto the eager path was its shape. The chain lane coerced every operand with TypeConverter.ToJsString before the cutoff was read, allocating a JsString wrapper per non-string operand, and ConcatMany built a string[] on top of the JsString[] the operands already lived in. An operand is now carried in whichever form its coercion produced - the JsString it already was, or the plain text a non-string primitive coerced to - both of which answer their length without materializing anything, and below the cutoff the result is joined through a ValueStringBuilder over a cutoff-sized stack buffer: no second array and no wrapper. Above the cutoff the fold through JsString.Concat is unchanged. The pairwise `+` gets the same treatment: two string operands take a branch that coerces nothing, and the mixed pair (`n + "x"`) builds the wrapper only if it goes on to defer. TypeConverter.ToStringNonString becomes internal for that. `+=` and String.prototype.concat are untouched. Adapted for 4.x: - Engine hunks (JsString.cs, JintBinaryExpression.cs, TypeConverter.cs) apply verbatim; ValueStringBuilder and the assembly's SkipLocalsInit are already on this branch. - StringConcatenationTests: transcribed from NUnit to xUnit v3 ([Test] to [Fact], [TestCase] to [Theory] with [InlineData]); the allocation guard asks TryGetAllocatedBytesForCurrentThread, as in the previous commit. Evidence: bytes per evaluation of the eleven-operand short chain (y + '-' + m + '-' + d + ' ' + y + ':' + m + ':' + d), measured as a delta of the thread-local allocation counter over 20,000 evaluations - unfixed 4.x 272 on net10.0 and 408 on net472; with this package 272 and 296, the same after-figures main measured. So a short chain now costs no more than it did on 4.x before sebastienros#3386, and 27% less on net472. The ported tests against unfixed 4.x: the eleven new cases fail 3 on net10.0 and 4 on net472 - AccumulatingWithPlusIsNoLongerQuadratic's two five-operand rows (2,562,225,160 and 2,562,134,112 B for 8,000 iterations against a 16 MB bound), APairWithOneCoercedOperandTakesBothSidesOfTheCutoff at its "is a RopeString" premise, and on net472 AShortChainDoesNotPayForTheDeferredRepresentation at 408 B against its 400 B ceiling. The seven AChainMixingCoercedAndStringOperandsProducesTheSameCharacters rows pass, pinning characters that must not change. With the package all pass on both frameworks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs
lahma
added a commit
that referenced
this pull request
Oct 5, 2026
…uilding, safe across engines and charged to LimitMemory (#4160) * Backport #3386 to 4.x: Strings: a long `+` defers its copy, so `s = s + x` is linear like `s += x` Backport of PR #3386 (commit 9d80990) from main. `s += x` and `s = s + x` mean the same thing and did not cost the same thing. The compound form builds into `JsString.ConcatenatedString`, which is `StringBuilder`-backed and amortised linear; a plain `+` coerced both operands to `string` and returned `JsString.Create(string.Concat(left, right))`, so every iteration of an accumulator loop copied the whole left operand, and prepending (`s = x + s`) had no fast path at all. `JsString.RopeString` is an immutable deferred form: two operands and a total length, flattened once on the first read that needs characters, memoized, and with both operand references released at that point. `JsString.Concat` builds one when the result reaches 512 characters and concatenates flat below that. `Length` - and so truthiness and the length comparison string equality performs first - is answered from the node; everything else flattens, `this[int]` included, since descending per character would be a new quadratic. The flatten walk is iterative over a heap array and descends right-first, so depth is not capped and cannot overflow the stack. The flattened `a + b + c` chain gets the same decision, so `s = s + a + b` is linear too, and the MaxLength guard still runs on the summed lengths before anything is built. `+=` is untouched. Also adds the four Assign* lanes to StringConcatLargeBenchmark. Adapted for 4.x: - JsString.cs, class remarks (conflict): main rewrote the subclassing paragraphs around LazyJsString, which is v5's #3340 and absent here. Kept 4.x's two paragraphs and their contract; only the list of Jint's own representations gains the deferred one. - JsValueExtensions.cs (conflict; the file is Jint/JsValueExtensions.cs on this branch): the "InternalTypes.String is set only by" list gains RopeString, without main's LazyJsString; 4.x's `IsString` cref is kept. - LazyJsString.cs (absent): main's hunk there is a comment. It adds RopeString to the list of Jint's own lazy strings that are built on JsString directly rather than on LazyJsString, which is what lets LazyJsString's declared- length verification run with no out-of-assembly gate. The invariant behind it is one a node depends on: it answers its own Length as the sum of its operands' Lengths, sizes the flatten buffer from that sum and writes each operand's text at an offset computed from it, so every operand's Length has to be the length of its text. On main a host string is a LazyJsString whose declared length host-contract verification checks against what it produces. On 4.x the same role is played by a host's direct JsString subclass - the documented subclassing contract, pinned by LazyHostStringTests and by RavenApiUsageTests' CustomString - whose Length is its own unverified claim. So main's `Immutable` (snapshot a ConcatenatedString, hold everything else) becomes `IsRetainable`: a node holds only Jint's own immutable representations - an exact JsString, a SlicedString or a RopeString - and a ConcatenatedString or a host subclass is snapshotted at the `+`, with the node's length summed from what it actually holds. That keeps a host's ToString() at the `+`, where 4.x has always called it, and keeps a host whose Length disagrees with its text producing that text. The two tests added to Jint.Tests.PublicInterface's LazyHostStringTests pin both; run against main's rule verbatim, all three cases fail - the host is not materialized at the `+` (count 0, expected 1), a host reporting 5 for "ab" flattens to "\0\0\0ab...", and one reporting 3 for "abcd" throws ArgumentOutOfRangeException out of RopeString.CopyInto. The accumulator a loop builds is always Jint's own after its first deferred `+`, so the snapshot does not reintroduce a copy per iteration. - README.md (conflict): main's hunk edits the "Lazy strings" section, which this branch's README does not have, and this README never described `s = s + x` as quadratic or recommended `+=` over `+`; dropped. - docs/v5-migration.md: absent on this branch; dropped. - StringConcatenationTests: this branch's GCPolyfills has no AllocatedBytesForCurrentThreadIsSupported (main's #3036); the guard asks TryGetAllocatedBytesForCurrentThread instead. The file was already xUnit on main at #3386. Evidence - the ported tests against unfixed 4.x (engine files at upstream/4.x plus a never-committed compile stub declaring RopeString and the cutoff constant), net10.0 and net472 alike: in StringConcatenationTests, StringRepresentationKeyTests and StringLengthLimitTests 32 fail and 55 pass. Three of the failures are the asymptotic claim itself - AccumulatingWithPlusIsNoLongerQuadratic allocates 640,758,656 B (`s = s + chunk`), 640,744,128 B (`s = chunk + s`) and 1,281,226,960 B (`s = s + chunk + chunk`) for 8,000 iterations against a 16 MB bound; the other 29 stop at the "is a RopeString" premise assertion. The 55 passes pin what must not change (characters, order, keys, the existing rows of both older classes). AVeryDeepTreeFlattensWithoutRecursing (4x10^10 characters copied at 200,000 iterations without the fix) and ConcatenationPastTheLimitIsACatchableRangeError (about 1.3 GB of flat strings without the fix) were not run against unfixed code on this shared machine. With the package all of them pass on both frameworks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs * Backport #3571 to 4.x: Strings: a short `+` chain no longer pays for a deferred representation it never takes Backport of PR #3571 (commit ef706d7) from main. #3386 cost SunSpider's date-format-xparb 3.9% on main (#3527), and not because of the deferral: xparb's chains are ten to thirty-five characters, far below the 512-character cutoff. What moved onto the eager path was its shape. The chain lane coerced every operand with TypeConverter.ToJsString before the cutoff was read, allocating a JsString wrapper per non-string operand, and ConcatMany built a string[] on top of the JsString[] the operands already lived in. An operand is now carried in whichever form its coercion produced - the JsString it already was, or the plain text a non-string primitive coerced to - both of which answer their length without materializing anything, and below the cutoff the result is joined through a ValueStringBuilder over a cutoff-sized stack buffer: no second array and no wrapper. Above the cutoff the fold through JsString.Concat is unchanged. The pairwise `+` gets the same treatment: two string operands take a branch that coerces nothing, and the mixed pair (`n + "x"`) builds the wrapper only if it goes on to defer. TypeConverter.ToStringNonString becomes internal for that. `+=` and String.prototype.concat are untouched. Adapted for 4.x: - Engine hunks (JsString.cs, JintBinaryExpression.cs, TypeConverter.cs) apply verbatim; ValueStringBuilder and the assembly's SkipLocalsInit are already on this branch. - StringConcatenationTests: transcribed from NUnit to xUnit v3 ([Test] to [Fact], [TestCase] to [Theory] with [InlineData]); the allocation guard asks TryGetAllocatedBytesForCurrentThread, as in the previous commit. Evidence: bytes per evaluation of the eleven-operand short chain (y + '-' + m + '-' + d + ' ' + y + ':' + m + ':' + d), measured as a delta of the thread-local allocation counter over 20,000 evaluations - unfixed 4.x 272 on net10.0 and 408 on net472; with this package 272 and 296, the same after-figures main measured. So a short chain now costs no more than it did on 4.x before #3386, and 27% less on net472. The ported tests against unfixed 4.x: the eleven new cases fail 3 on net10.0 and 4 on net472 - AccumulatingWithPlusIsNoLongerQuadratic's two five-operand rows (2,562,225,160 and 2,562,134,112 B for 8,000 iterations against a 16 MB bound), APairWithOneCoercedOperandTakesBothSidesOfTheCutoff at its "is a RopeString" premise, and on net472 AShortChainDoesNotPayForTheDeferredRepresentation at 408 B against its 400 B ceiling. The seven AChainMixingCoercedAndStringOperandsProducesTheSameCharacters rows pass, pinning characters that must not change. With the package all pass on both frameworks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs * Backport #4130 to 4.x: Build a bound function's name as a rope so a chain of binds stays linear Backport of PR #4130 (commit 5eac242) from main. 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 (#4129). 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 - and every observable name is byte-identical. Adapted for 4.x: - On this branch BindFunction derives from ObjectInstance, not Function (main made it a Function in #3658, which is not here), so Function.prototype.bind names the function it creates through ObjectInstance.SetFunctionName, which main's hunk does not touch and which still did `prefix + " " + name`. With main's Function.cs hunk alone the bind chain stayed exactly as quadratic (measured: 9.5, 36.9, 145.3, 576.7 MB allocated for 1,250, 2,500, 5,000 and 10,000 levels). Function.PrefixName becomes internal rather than private and ObjectInstance.SetFunctionName calls it, so both overloads defer. Main's Function.cs hunk applies verbatim otherwise; it still covers the get/set accessor names and ShadowRealm's wrapped functions, which are Functions. - BoundFunctionNameTests: transcribed from NUnit to xUnit v3. The wedge ceiling is 256 MB instead of main's 768 MB: with the fix the whole script allocates 53.0 MB on net10.0 and 55.3 MB on net472, and against the defect the per-level copies are all retained, so main's ceiling would let a run against unfixed code hold about 770 MB of live text before it tripped. 256 MB trips at level 6,649 (net10.0) / 6,648 (net472) with a peak working set of 291 / 284 MB. Added ABoundNameLongEnoughToDeferIsANodeOverTheLevelBelow, which asserts the premise directly - a 100-level bound name is a RopeString - because on this branch the route bind takes is the one main's hunk misses. Evidence - the bind chain, measured as allocated and retained bytes around `for (i < N) f = f.bind(null)` on net10.0: depth unfixed 4.x allocated / retained with this package 1,250 9.5 MB / 9.4 MB 0.6 MB / 0.5 MB 2,500 36.9 MB / 36.7 MB 1.1 MB / 1.0 MB 5,000 145.3 MB / 145.0 MB 2.2 MB / 1.9 MB 100,000 (not run: about 60 GB) 45.5 MB / 36.7 MB, 98 ms Unfixed quadruples per doubling; fixed doubles. net472 unfixed: 9.7, 37.1, 145.6 MB for the same three depths. The ported tests against unfixed 4.x fail 2 of 8 on both frameworks - ADeepChainOfBoundFunctionsDoesNotCopyTheNameAtEveryLevel with MemoryLimitExceededException at 256 MB, and the new premise test at "is a RopeString" - and the six APrefixedNameIsTheSameTextItAlwaysWas rows pass, pinning the names that must not change. With the package all 8 pass on both. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs * Backport #4164 to 4.x: Fall back to a rope's published text when a concurrent flatten has released its operands RopeString.Flatten memoized its text and then released both operands with plain writes, while CopyInto tested the memo and then dereferenced the operands. A second thread finishing its own flatten of the same node between those two reads left the walk holding a null operand, and the next ToString() threw NullReferenceException. On 4.x, too, preparation folds a literal-plus-literal into one constant on the shared Prepared<Script>, so every engine running a preparation reads the same RopeString. The engine change applies as is. The tests are transcribed from NUnit to xUnit v3: the two tests are async and wait on a local two-minute wedge ceiling through Task.WhenAny (4.x has no TestBudgets, and xUnit1031 rejects a blocking Task.WaitAll). (cherry picked from commit 78e9bb0) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Backport #4173 to 4.x: Charge a deferred string concatenation to LimitMemory when it is built A long `a + b` returns a RopeString, whose characters are allocated by whoever flattens it, and that can be a host's ToString() after every constraint is disarmed. JsString.Concat now charges the memory limit on its deferring branches: the shorter operand, which keeps `s = s + x` linear, or the whole length, refused before the node exists, when the flat form alone exceeds the budget. A constant fold evaluates with an engine-less context and is not charged. Adapted for 4.x's MemoryLimitConstraint, which predates main's operation state and async segments: it measures the thread's allocation counter against a per-entry baseline. The charge is kept in a _deferredBytes total that Reset() clears with the baseline and Check() adds to the measured allocation. A result whose flat form alone exceeds the budget calls Check() and then throws regardless, because Check() measures nothing on a thread other than the one the entry started on. The engine finds its constraint once at construction (Engine._memoryLimitConstraint). 4.x's ConcatSnapshotting branch, which main does not have, is charged too, for the lengths the node actually holds. Function.PrefixName is called from ObjectInstance.SetFunctionName on 4.x as well, and both pass the engine's evaluation context. Not ported: main's AllocatedBytes doc line and the TheCharactersADeferredConcatenationAppendsAreReportedAsAllocated test (MemoryLimitConstraint.AllocatedBytes is not public API on 4.x), the docs/guide and migration-guide hunks (absent on 4.x; the README's constraints section gets the paragraph instead) and the Jint/Constraints/AGENTS.md hunk (absent on 4.x). Tests transcribed from NUnit to xUnit v3. Adapted from 7d9cdcc Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- 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.
s += xands = s + xmean the same thing and did not cost the same thing. The compound form builds intoJsString.ConcatenatedString, which isStringBuilder-backed and amortised linear; a plain+went throughApplyAdditionToPrimitives, which coerced both operands tostringand returnedJsString.Create(string.Concat(left, right)), so every iteration of an accumulator loop copied the whole left operand. Prepending (s = x + s) had no fast path at all, because+=cannot express it.Fixes #3350.
The node
+cannot simply reuseConcatenatedString: it is mutated in place, and+=may do that only because the assignment replaces the receiver. A non-assignment+leaves both operands reachable from wherever they were read, so the deferred form it can use has to be immutable — which is also the property that makes it work for prepend, the shape+=can never reach.JsString.RopeString(internal, nested besideConcatenatedStringandSlicedString) is that form: twoJsStringoperands and a total length.JsString.Concatbuilds one once the result reaches 512 characters and concatenates flat below that. The threshold is a knob for the small-string case, not for the fix: a loop accumulating in n-character pieces copies for the first512/niterations and defers from then on, so the entire quadratic term is bounded by 512² characters however long the loop runs.The five things that are ways to get this wrong:
Lengthwithout flattening;ToString()flattening onceLengthis a stored field, sostr.length, truthiness and the length comparisonEquals(JsString)performs first are all free.ToString()is_value ?? Flatten(), andFlattenmemoizes into_valueand releases both child references, so a flattened value stops retaining the tree it was built from — one node per iteration, for an accumulator loop.GetHashCodeconsistent with the flat forms_valueonly when it is eithernullor the exact flat text, which is precisely the invariant this class keeps (the oneConcatenatedStringdoes not, which is why it has to override them). Pinned both before and after flattening, and as aMap/Set/property key on both sides of a lookup.ToString()Lengthandthis[int]are the only two the class declares.this[int]flattens rather than descending: descending is O(depth), so acharCodeAtscan over a freshly accumulated value would be a new quadratic — the very shape being removed. Flattening costs the one copy the old code performed on every concatenation anyway, and the memo makes every later read O(1).Contains,IndexOf,StartsWith,EndsWith,Substring,Equals(string),TryGetIterator,Append,EnsureCapacityandToObjectall route throughToString()already and are correct unchanged.s = s + x, a left-leaning spine) never holds more than one pending entry; the prepend shape is the one that pays for the array. Tested at 200,000 nodes in both leanings.MaxLengthenforcementintlength arithmetic be unchecked.One more hazard the issue did not name, and the reason
Immutableexists: an operand may itself be aConcatenatedString, which a later+=grows in place. A node that merely held the reference would change content behind whoever read the concatenation, so such an operand is snapshotted on the way in — a wrapper around the string the builder has already flattened, not a character copy.AppendingToAnOperandAfterwardsDoesNotChangeTheResultpins it.The flattened
a + b + cchain (AdditionChainExpression) gets the same decision, through three helpers shared by the direct and the resumed paths, sos = s + a + bis linear too — otherwise it would have been the one shape that stayed quadratic. Below the threshold it still concatenates in the single allocation that node exists to produce, and the coercion moved fromTypeConverter.ToStringtoTypeConverter.ToJsString, which produces the same characters without materializing an operand's representation.+=is untouched. Not one line ofJintAssignmentExpressionchanged, andAppend/EnsureCapacityare not overridden on the node — a+=against a deferred value flattens once and lands back on the builder lane, which is what keeps repeated small appends on the representation that is best for them.Evidence
Asymptotics, measured by allocation rather than wall-clock — the quadratic is the copying, so allocated bytes say the same thing as a stopwatch without depending on what else the machine is doing.
AccumulatingWithPlusIsNoLongerQuadraticbuilds at 4,000 and 8,000 iterations of a 10-character chunk and asserts the ratio is under 3 (linear ≈ 2, quadratic ≈ 4) plus an absolute 16 MB ceiling. Measured on this branch against the same test with the deferral threshold set toint.MaxValue, which is exactly the old behaviour:s = s + xs = x + ss = s + x + yEvery other new test also fails against that same disabled-threshold build, so none of them is asserting a tautology.
Jint.Repl, Release, the issue's own repro (indicative only — REPL timings, not gate-quality numbers):s = s + xwent 463 ms → 10 ms at 20,000 iterations and 1,626 ms → 20 ms at 40,000, i.e. from 3.5× for 2× the work to 2.0×.s = x + sands = s + x + yscale the same way.A side effect worth its own row:
s = s + snow doubles the length by referencing the same value twice, so theMaxLengthguard for+is reachable through 29 nodes and no characters at all. That path used to need the half gigabyte it exists to prevent, andStringLengthLimitTestssaid so in prose; it now has a real test.Benchmarks — added, not run
Four lanes in
StringConcatLargeBenchmark, which had none of this shape (every accumulating lane there usess += chunk, andConcatLargePairrebuilds a fresh pair each iteration so its left operand never grows — which is why the July 2026 rope evaluation came out a NO-GO for what it measured):AssignAppendSmallChunks—s = s + chunk16× 4,096AssignPrependSmallChunks—s = chunk16 + s× 4,096AssignChainThree—s = s + chunk16 + chunk16× 2,048AssignAppendThenScan— the first one plus acharCodeAtscan of the resultMeasurement is outstanding. This machine runs agents concurrently, so nothing here was benchmarked and no timing figure in this PR comes from BenchmarkDotNet.
JINT_BENCH_MODE=gateon an idle machine, run serially, paired againstmain:The pairings that carry the reading:
AssignAppendSmallChunksvsAppendSmallChunks— the same 4,096 × 16 characters with one operator changed. This is the headline: the difference between them is the whole remaining cost of the+path.AssignAppendThenScanvsAssignAppendSmallChunks— the same build plus a read-back, so their difference is what flattening costs. The other three lanes finish ons.length, which a deferred value answers without ever producing characters, so this is the one a deferred representation cannot cheat.ChainSmallThreeandChainSmallSix, branch vsmain— the guard rows. Short chains take the flat path and must not get slower; the only change they see is that the operand lengths are now read through a virtualLengthrather than off the coercedstring.AppendSmallChunks,AppendLargeChunks,BuildLargeThenScan, branch vsmain— the+=rows. They must be flat: no code on their path changed, but they now shareJsString.Length/ToString()call sites with one more subclass, and a lost guarded devirtualization would show up here or nowhere.ConcatLargePairandChainLargeThreeshould improve outright (big64k + big64kfollowed by.lengthno longer copies 128 KB); they are the least interesting rows here.Verification
dotnet build -c Release— clean, 0 warnings, all five target frameworks.Jint.Tests— 10,745 passed on net10.0 and net8.0, 7,369 on net472 (which is what exercises the#if NETFRAMEWORKhalf of the flatten), 0 failed. Also green withJINT_HOST_CONTRACT_VERIFICATION=1.Jint.Tests.PublicInterface— 2,998 passed on net10.0, 2,369 on net472, 0 failed. No public API added, soUndocumentedPublicApi.txt(634) is untouched;RopeStringisinternaland nested, like its two siblings.Jint.Tests.CommonScripts— 28/28 on both frameworks.Jint.Tests.Test262— 102,495 passed, 0 failed, 189 skipped. Unchanged.Documentation
docs/v5-migration.md§4.27 — a+may now hand a host aJsStringsubclass (it always could, for+=, so only an exact-type test is affected), and the result keeps its operands alive until something reads its text.README.md— theLazyJsStringsection listed concatenation among the operations that materialize a lazy host string. A long one no longer does; it holds the value as an operand.Jint.Benchmark/StringConcatLargeBenchmark.cs,Jint.Tests/Runtime/StringRepresentationKeyTests.csandJint.Tests/Runtime/StringLengthLimitTests.csall carried prose that this change makes wrong; each is corrected in place rather than left to mislead the next reader.Nothing dropped
One judgement call worth flagging for review:
Flatten()carries a#if NETFRAMEWORK || NETSTANDARD2_0block rather than aPolyfills.csentry, against the repository's polyfill-downwards rule. What those targets lack is notstring.CreatebutSystem.Buffers.SpanAction<T, TArg>, its callback type — a "polyfill" declaring a delegate of its own would be inventing API rather than backfilling it. The downlevel arm fills achar[]and copies it into the string; the modern arm writes the characters into the string as it is allocated, and that single copy is what keeps a flattened concatenation no more expensive than thestring.Concatit replaced.