From dce8372a4dd48cf8bcd020638a1edac1ac8751bc Mon Sep 17 00:00:00 2001
From: Marko Lahma <marko.lahma@gmail.com>
Date: Tue, 21 Jul 2026 10:10:30 +0300
Subject: [PATCH] Keep slice-of-slice and split segments zero-copy

Two zero-copy String optimizations building on the existing SlicedString view:

- Add JsString.CreateSliced(JsString, int, int) that rebases a slice of an
  un-materialized SlicedString directly onto its original backing string,
  evaluating the retention policy against that original source so chained views
  cannot compound the pinning of a large backing string. A materialized view or
  any other receiver falls back to the flat path. Slice/Substring/Substr now
  pass the JsString receiver instead of s.ToString(), so a slice-of-slice never
  materializes the intermediate view. The copy fallback now routes through
  Create so short results still hit the cached single-char / empty instances.

- Split now routes each segment and the tail through CreateSliced (policy
  decides copy vs view against the source length), and gains a dedicated
  split("") fast path that builds an exact-capacity array from cached single-char
  JsStrings, skipping the intermediate segment list. Segment list element type
  changed from string to JsString accordingly.

Custom JsString subclasses keep the existing plain path (no new materialization).
Adds a SliceOfSlice benchmark and unit tests for slice-of-slice correctness,
split("") on ASCII/non-ASCII, split tail/segment correctness, and the retention
policy still copying small results.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 Jint.Benchmark/StringSliceBenchmarks.cs      |  21 +++
 Jint.Tests/Runtime/StringTests.cs            | 153 +++++++++++++++++++
 Jint/Native/JsString.cs                      |  34 ++++-
 Jint/Native/String/StringExecutionContext.cs |   4 +-
 Jint/Native/String/StringPrototype.cs        |  73 ++++-----
 5 files changed, 246 insertions(+), 39 deletions(-)

diff --git a/Jint.Benchmark/StringSliceBenchmarks.cs b/Jint.Benchmark/StringSliceBenchmarks.cs
index 382ab52b01..ab9a0b2125 100644
--- a/Jint.Benchmark/StringSliceBenchmarks.cs
+++ b/Jint.Benchmark/StringSliceBenchmarks.cs
@@ -14,6 +14,8 @@ namespace Jint.Benchmark;
 /// SearchOnSlice exercises indexOf/startsWith/endsWith/includes on a fresh large slice each
 /// iteration — the case where the inherited base search methods materialize the whole substring
 /// on every call. With zero-copy span search overrides this drops to ~0 allocation.
+/// SliceOfSlice slices a large view of a view: without receiver unwrapping the second slice
+/// materializes the intermediate view; with it, it rebases straight onto the backing string.
 /// </summary>
 [MemoryDiagnoser]
 [HideColumns("Error", "Gen0", "Gen1", "Gen2")]
@@ -26,6 +28,7 @@ public class StringSliceBenchmarks
     private Prepared<Script> _sliceThenRead;
     private Prepared<Script> _searchOnSlice;
     private Prepared<Script> _searchOnFlat;
+    private Prepared<Script> _sliceOfSlice;
 
     [GlobalSetup]
     public void Setup()
@@ -109,6 +112,20 @@ public void Setup()
             })();
             """, strict: true);
 
+        // Slice-of-slice: the first slice returns a large zero-copy view; the second slices that view.
+        // Without receiver unwrapping the second slice materializes the intermediate view first; with
+        // it, the second slice rebases straight onto the original backing string.
+        _sliceOfSlice = Engine.PrepareScript("""
+            (function() {
+                var ret = null;
+                for (var i = 0; i < 5000; i++) {
+                    var outer = str.slice(0, 100000);
+                    ret = outer.slice(1000, 60000);
+                }
+                return ret.length;
+            })();
+            """, strict: true);
+
         _engine = new Engine(static options => options.Strict());
         // Build the shared ~128K char base string once (dromaeo-style doubling).
         _engine.Execute("""
@@ -123,6 +140,7 @@ public void Setup()
         _engine.Evaluate(_sliceThenRead);
         _engine.Evaluate(_searchOnSlice);
         _engine.Evaluate(_searchOnFlat);
+        _engine.Evaluate(_sliceOfSlice);
     }
 
     [Benchmark]
@@ -142,4 +160,7 @@ public void Setup()
 
     [Benchmark]
     public JsValue SearchOnFlat() => _engine.Evaluate(_searchOnFlat);
+
+    [Benchmark]
+    public JsValue SliceOfSlice() => _engine.Evaluate(_sliceOfSlice);
 }
diff --git a/Jint.Tests/Runtime/StringTests.cs b/Jint.Tests/Runtime/StringTests.cs
index 0d4ff68fc5..5caa727bd8 100644
--- a/Jint.Tests/Runtime/StringTests.cs
+++ b/Jint.Tests/Runtime/StringTests.cs
@@ -348,6 +348,159 @@ public void PlantedStringPrototypeIndexOrLengthFunctionIsNotCalledOnStringReceiv
         Assert.Equal("a|TypeError", engine.Evaluate(script).AsString());
     }
 
+    [Fact]
+    public void SliceOfSliceMatchesMaterializedExpectation()
+    {
+        // Build a ~128K backing string; slice/substring/substr produce a zero-copy view over it, and a
+        // second slice of that (still un-materialized) view must rebase onto the original backing string
+        // rather than materializing the intermediate. Each chained result is compared against an
+        // identical *flat* string produced via a JSON round-trip (a separate, fully materialized path).
+        const string script = @"
+var seed = 'aB3$xQ9pLm0_kEwZ';
+var s = seed;
+while (s.length < 131072) s += s;         // 131072 chars
+
+// slice-of-slice (view then slice), substring-of-slice, substr-of-slice
+var v1 = s.slice(0, 100000);              // large -> view
+var a = v1.slice(1000, 60000);            // rebased slice-of-slice
+var b = v1.substring(2000, 50000);        // rebased substring-of-slice
+var c = v1.substr(3000, 40000);           // rebased substr-of-slice
+
+// materialized expectations from the original backing string
+var ea = JSON.parse(JSON.stringify(s.slice(1000, 60000)));
+var eb = JSON.parse(JSON.stringify(s.substring(2000, 50000)));
+var ec = JSON.parse(JSON.stringify(s.substr(3000, 40000)));
+
+var ok =
+    a.length === ea.length && a === ea &&
+    b.length === eb.length && b === eb &&
+    c.length === ec.length && c === ec &&
+    // boundary spot checks so a shared bug can't pass silently
+    a.charAt(0) === s.charAt(1000) &&
+    a.charAt(a.length - 1) === s.charAt(59999) &&
+    b.charAt(0) === s.charAt(2000);
+
+// slice of an ALREADY-materialized view must also stay correct (treated as flat)
+var v2 = s.slice(0, 100000);
+var forced = v2 + '';                      // materialize v2's backing substring
+var d = v2.slice(500, 70000);
+var ed = JSON.parse(JSON.stringify(s.slice(500, 70000)));
+
+ok && d.length === ed.length && d === ed ? 'ok' : 'fail';
+";
+        Assert.Equal("ok", _engine.Evaluate(script).AsString());
+    }
+
+    [Fact]
+    public void SliceOfSliceStaysZeroCopyForLargeResult()
+    {
+        // A moderate slice of a large view whose unused remainder stays within the retention budget is
+        // kept as a zero-copy view (SlicedString), rebased onto the original backing string.
+        _engine.Execute(@"
+var seed = 'aB3$xQ9pLm0_kEwZ';
+var s = seed;
+while (s.length < 131072) s += s;
+var v1 = s.slice(0, 100000);");
+
+        Assert.Contains("Sliced", _engine.Evaluate("v1").GetType().Name);
+        Assert.Contains("Sliced", _engine.Evaluate("v1.slice(1000, 60000)").GetType().Name);
+    }
+
+    [Fact]
+    public void ChainedSliceOfViewCopiesAgainstOriginalSource()
+    {
+        // Evaluating the retention policy against the ORIGINAL backing string (not the intermediate
+        // view) prevents a chained view from pinning a much larger source: a 200K slice of a 256K view
+        // over a 512K backing string copies, because the ~312K unused remainder of the backing string
+        // exceeds the retention budget. Value stays correct regardless.
+        _engine.Execute(@"
+var seed = 'aB3$xQ9pLm0_kEwZ';
+var s = seed;
+while (s.length < 524288) s += s;         // 512K
+var v1 = s.slice(0, 262144);              // half -> view
+var small = v1.slice(0, 200000);          // rebased; copies against the 512K source");
+
+        // v1 is a view, but the chained slice is copied (flat JsString), not a compounded view.
+        Assert.Contains("Sliced", _engine.Evaluate("v1").GetType().Name);
+        Assert.DoesNotContain("Sliced", _engine.Evaluate("small").GetType().Name);
+
+        // and the value is still exactly s[0..200000]
+        Assert.Equal("ok", _engine.Evaluate(
+            "small.length === 200000 && small === JSON.parse(JSON.stringify(s.slice(0, 200000))) ? 'ok' : 'fail'").AsString());
+    }
+
+    [Fact]
+    public void SplitEmptySeparatorAscii()
+    {
+        Assert.Equal(@"[""h"",""e"",""l"",""l"",""o""]", _engine.Evaluate(@"JSON.stringify('hello'.split(''))").AsString());
+        Assert.Equal(5, _engine.Evaluate("'hello'.split('').length").AsNumber());
+        // limit truncates
+        Assert.Equal(@"[""h"",""e"",""l""]", _engine.Evaluate(@"JSON.stringify('hello'.split('', 3))").AsString());
+        // empty string splits to an empty array
+        Assert.Equal(0, _engine.Evaluate("''.split('').length").AsNumber());
+        // cached single-char instances still compare equal by value
+        Assert.True(_engine.Evaluate("'aba'.split('')[0] === 'a' && 'aba'.split('')[2] === 'a'").AsBoolean());
+    }
+
+    [Fact]
+    public void SplitEmptySeparatorNonAscii()
+    {
+        // BMP chars above the ASCII cache (é = U+00E9, Greek U+03B1..) must round-trip correctly.
+        Assert.Equal(@"[""c"",""a"",""f"",""é""]", _engine.Evaluate(@"JSON.stringify('café'.split(''))").AsString());
+        Assert.Equal(@"[""α"",""β"",""γ""]", _engine.Evaluate(@"JSON.stringify('αβγ'.split(''))").AsString());
+        Assert.Equal(3, _engine.Evaluate("'αβγ'.split('').length").AsNumber());
+
+        // split('') splits by UTF-16 code unit: an astral char (surrogate pair) becomes two elements.
+        Assert.Equal(2, _engine.Evaluate("'😀'.split('').length").AsNumber());
+        Assert.True(_engine.Evaluate(
+            "var p = '😀'.split(''); p[0] === '\uD83D' && p[1] === '\uDE00'").AsBoolean());
+    }
+
+    [Fact]
+    public void SplitTailAndSegmentsAreCorrect()
+    {
+        // Small segments and tail
+        Assert.Equal(@"[""a"",""b"",""cdefghij""]", _engine.Evaluate(@"JSON.stringify('a,b,cdefghij'.split(','))").AsString());
+        // consecutive separators produce empty segments
+        Assert.Equal(@"[""a"","""",""b""]", _engine.Evaluate(@"JSON.stringify('a,,b'.split(','))").AsString());
+        // multi-char separator
+        Assert.Equal(@"[""a"",""b"",""c""]", _engine.Evaluate(@"JSON.stringify('a<>b<>c'.split('<>'))").AsString());
+        // trailing separator yields a trailing empty segment
+        Assert.Equal(@"[""a"",""b"",""""]", _engine.Evaluate(@"JSON.stringify('a,b,'.split(','))").AsString());
+
+        // Large tail segment: the final piece of a large string, routed through the retention policy,
+        // must still equal the exact backing substring.
+        const string script = @"
+var seed = 'aB3$xQ9pLm0_kEwZ';
+var s = seed;
+while (s.length < 131072) s += s;
+var parts = ('HEADER|' + s).split('|');   // ['HEADER', s]
+var tail = parts[1];
+parts.length === 2 &&
+    parts[0] === 'HEADER' &&
+    tail.length === s.length &&
+    tail === JSON.parse(JSON.stringify(s)) ? 'ok' : 'fail';
+";
+        Assert.Equal("ok", _engine.Evaluate(script).AsString());
+    }
+
+    [Fact]
+    public void SplitPolicyCopiesSmallSegmentsAndViewsLargeOnes()
+    {
+        _engine.Execute(@"
+var seed = 'aB3$xQ9pLm0_kEwZ';
+var s = seed;
+while (s.length < 131072) s += s;
+var small = 'a,b,c'.split(',');
+var big = (s + '|tail').split('|');       // [s, 'tail']: first segment is ~the whole source -> view");
+
+        // small segments are copied (not views)
+        Assert.DoesNotContain("Sliced", _engine.Evaluate("small[0]").GetType().Name);
+        // a large-enough segment of a large string is kept as a zero-copy view
+        Assert.Contains("Sliced", _engine.Evaluate("big[0]").GetType().Name);
+        Assert.True(_engine.Evaluate("big.length === 2 && big[0] === s && big[1] === 'tail'").AsBoolean());
+    }
+
     public static TheoryData<string, string> GetLithuaniaTestsData()
     {
         return new StringTetsLithuaniaData().TestData();
diff --git a/Jint/Native/JsString.cs b/Jint/Native/JsString.cs
index 3f4d78abc3..ae4710d266 100644
--- a/Jint/Native/JsString.cs
+++ b/Jint/Native/JsString.cs
@@ -229,7 +229,33 @@ internal static JsString CreateSliced(string source, int start, int length)
             return new SlicedString(source, start, length);
         }
 
-        return new JsString(source.Substring(start, length));
+        // Copy fallback goes through Create so short results (e.g. single-char or empty split
+        // segments) still hit the cached single-char / empty instances. slice/substring/substr
+        // never reach here with length < 2 (they short-circuit first), so Create is equivalent to
+        // new JsString for those callers.
+        return Create(source.Substring(start, length));
+    }
+
+    /// <summary>
+    /// Creates a string for a substring of <paramref name="source"/>. When <paramref name="source"/>
+    /// is a not-yet-materialized <see cref="SlicedString"/>, the slice is rebased directly onto the
+    /// original backing string so a slice-of-slice never materializes the intermediate view, and the
+    /// retention policy is evaluated against that original backing string so chained views cannot
+    /// compound the pinning of a large source. A materialized view or any other receiver is treated
+    /// as flat.
+    /// </summary>
+    internal static JsString CreateSliced(JsString source, int start, int length)
+    {
+        // A view still carrying its lazy (unmaterialized) value is rebased onto its own backing
+        // string: offset past the view's own start and let the policy weigh retention against the
+        // original source. Once a view has materialized its flat value, reusing that string is
+        // cheaper than pinning the (possibly much larger) original backing string, so fall through.
+        if (source is SlicedString { _value: null } sliced)
+        {
+            return CreateSliced(sliced._source, sliced._start + start, length);
+        }
+
+        return CreateSliced(source.ToString(), start, length);
     }
 
     internal static JsString Create(int value)
@@ -505,8 +531,10 @@ internal override JsValue DoClone()
     /// </summary>
     internal sealed class SlicedString : JsString
     {
-        private readonly string _source;
-        private readonly int _start;
+        // internal (not private) so the enclosing JsString.CreateSliced can rebase a slice-of-slice
+        // directly onto this view's backing string without materializing the intermediate.
+        internal readonly string _source;
+        internal readonly int _start;
         private readonly int _length;
 
         internal SlicedString(string source, int start, int length)
diff --git a/Jint/Native/String/StringExecutionContext.cs b/Jint/Native/String/StringExecutionContext.cs
index e08eb224ee..e2f0b385db 100644
--- a/Jint/Native/String/StringExecutionContext.cs
+++ b/Jint/Native/String/StringExecutionContext.cs
@@ -9,13 +9,13 @@ internal sealed class StringExecutionContext
 {
     private static readonly ThreadLocal<StringExecutionContext> _executionContext = new ThreadLocal<StringExecutionContext>(() => new StringExecutionContext());
 
-    private List<string>? _splitSegmentList;
+    private List<JsString>? _splitSegmentList;
 
     private StringExecutionContext()
     {
     }
 
-    public List<string> SplitSegmentList => _splitSegmentList ??= new List<string>();
+    public List<JsString> SplitSegmentList => _splitSegmentList ??= new List<JsString>();
 
     public static StringExecutionContext Current => _executionContext.Value!;
 }
diff --git a/Jint/Native/String/StringPrototype.cs b/Jint/Native/String/StringPrototype.cs
index d2ee521786..1bc8348171 100644
--- a/Jint/Native/String/StringPrototype.cs
+++ b/Jint/Native/String/StringPrototype.cs
@@ -906,7 +906,7 @@ private static int ToIntegerSupportInfinityUnlikely(JsValue numberVal)
     [RequireObjectCoercible]
     private static JsValue Substring(JsValue thisObject, JsCallArguments arguments)
     {
-        var s = TypeConverter.ToString(thisObject);
+        var s = TypeConverter.ToJsString(thisObject);
         var start = TypeConverter.ToNumber(arguments.At(0));
         var end = TypeConverter.ToNumber(arguments.At(1));
 
@@ -951,7 +951,7 @@ private static JsValue Substring(JsValue thisObject, JsCallArguments arguments)
     [RequireObjectCoercible]
     private static JsValue Substr(JsValue thisObject, JsCallArguments arguments)
     {
-        var s = TypeConverter.ToString(thisObject);
+        var s = TypeConverter.ToJsString(thisObject);
         var start = TypeConverter.ToInteger(arguments.At(0));
         var length = arguments.At(1).IsUndefined()
             ? double.PositiveInfinity
@@ -1104,53 +1104,56 @@ private JsValue Split(JsValue thisObject, JsCallArguments arguments)
 
     internal static JsValue SplitWithStringSeparator(Engine engine, Realm realm, JsValue separator, string s, uint lim)
     {
-        var segments = StringExecutionContext.Current.SplitSegmentList;
-        segments.Clear();
         var sep = TypeConverter.ToString(separator);
 
-        if (sep == string.Empty)
+        if (sep.Length == 0)
         {
-            if (s.Length > segments.Capacity)
-            {
-                segments.Capacity = s.Length;
-            }
-
-            for (var i = 0; i < s.Length; i++)
+            // split("") yields the individual UTF-16 code units. Build the result array directly at
+            // exact capacity from the cached single-char JsStrings (JsString.Create(char)), skipping
+            // the intermediate segment list and its per-char string allocations for cached chars.
+            var count = (uint) System.Math.Min((long) s.Length, lim);
+            var emptyResult = realm.Intrinsics.Array.ArrayCreate(count);
+            for (var i = 0; i < count; i++)
             {
                 if (i > 0 && i % ConstraintCheckInterval == 0)
                 {
                     engine.Constraints.Check();
                 }
 
-                segments.Add(TypeConverter.ToString(s[i]));
+                emptyResult.SetIndexValue((uint) i, JsString.Create(s[i]), updateLength: false);
             }
+
+            emptyResult.SetLength(count);
+            return emptyResult;
         }
-        else
+
+        var segments = StringExecutionContext.Current.SplitSegmentList;
+        segments.Clear();
+
+        // Direct scan instead of string.Split: avoids the throwaway result array
+        // string.Split allocates on every call, and stops scanning once lim
+        // segments have been collected. Each segment goes through CreateSliced so the
+        // retention policy decides copy vs zero-copy view against the source length.
+        var start = 0;
+        while ((uint) segments.Count < lim)
         {
-            // Direct scan instead of string.Split: avoids the throwaway result array
-            // string.Split allocates on every call, and stops scanning once lim
-            // segments have been collected.
-            var start = 0;
-            while ((uint) segments.Count < lim)
+            if (segments.Count > 0 && segments.Count % ConstraintCheckInterval == 0)
             {
-                if (segments.Count > 0 && segments.Count % ConstraintCheckInterval == 0)
-                {
-                    engine.Constraints.Check();
-                }
-
-                var index = sep.Length == 1
-                    ? s.IndexOf(sep[0], start)
-                    : s.IndexOf(sep, start, StringComparison.Ordinal);
+                engine.Constraints.Check();
+            }
 
-                if (index < 0)
-                {
-                    segments.Add(start == 0 ? s : s.Substring(start));
-                    break;
-                }
+            var index = sep.Length == 1
+                ? s.IndexOf(sep[0], start)
+                : s.IndexOf(sep, start, StringComparison.Ordinal);
 
-                segments.Add(s.Substring(start, index - start));
-                start = index + sep.Length;
+            if (index < 0)
+            {
+                segments.Add(JsString.CreateSliced(s, start, s.Length - start));
+                break;
             }
+
+            segments.Add(JsString.CreateSliced(s, start, index - start));
+            start = index + sep.Length;
         }
 
         var length = (uint) System.Math.Min(segments.Count, lim);
@@ -1245,7 +1248,9 @@ private static JsValue Slice(JsValue thisObject, JsCallArguments arguments)
             return s;
         }
 
-        return JsString.CreateSliced(s.ToString(), from, span);
+        // Pass the JsString receiver (not s.ToString()) so a slice of an unmaterialized view rebases
+        // onto the original backing string instead of materializing the intermediate slice.
+        return JsString.CreateSliced(s, from, span);
     }
 
     [JsFunction(Length = 1)]