Stop a chain of binds copying the whole bound name at every level

Jint.Tests/Runtime/BoundFunctionNameTests.cs

@@ -0,0 +1,67 @@
+#nullable enable
+
+namespace Jint.Tests.Runtime;
+
+/// <summary>
+/// <see href="https://github.com/sebastienros/jint/issues/4129">#4129</see>: naming a bound function used
+/// to flatten the target's whole <c>"bound bound … f"</c> name into a fresh CLR string, so a chain of N
+/// binds cost O(N²) in both time and retained memory.
+/// </summary>
+public class BoundFunctionNameTests
+{
+    /// <summary>
+    /// The depth the issue measured: 37.3 s and roughly 30 GB of live text before the fix, which is what
+    /// shut the 7 GB Linux CI runners down mid-test.
+    /// </summary>
+    private const int ChainDepth = 100_000;
+
+    /// <summary>
+    /// The wedge ceiling, and deliberately not the assertion — no duration is asserted anywhere here.
+    /// The whole chain allocates a few tens of megabytes once each level's name is a node over the level
+    /// below; a per-level copy needs about 30 GB and trips this inside the first few thousand levels, so
+    /// the quadratic is reported as a failed test rather than as an exhausted machine. Raising this by an
+    /// order of magnitude would still catch it.
+    /// </summary>
+    private const long AllocationCeiling = 768L * 1024 * 1024;
+
+    [Test]
+    public void ADeepChainOfBoundFunctionsDoesNotCopyTheNameAtEveryLevel()
+    {
+        var engine = new Engine(options => options.LimitMemory(AllocationCeiling));
+
+        // Reading the full name at the end is legitimately O(N) once — it is 600,006 characters — and
+        // comparing it against the same text built in script is what pins the bytes as unchanged.
+        var result = engine.Evaluate($$"""
+            function target(x) { return x; }
+            var f = target;
+            var shallow;
+            for (var i = 0; i < {{ChainDepth}}; i++) {
+                f = f.bind(null);
+                if (i === 99) { shallow = f; }
+            }
+            var name = f.name;
+            [
+                name.length,
+                name === 'bound '.repeat({{ChainDepth}}) + 'target',
+                name.slice(0, 12),
+                name.slice(-12),
+                f.length,
+                typeof f,
+                shallow(42)
+            ].join('|');
+            """).AsString();
+
+        result.Should().Be("600006|true|bound bound |bound target|1|function|42");
+    }
+
+    [TestCase("(function f() {}).bind(null).name", "bound f")]
+    [TestCase("(function f() {}).bind(null).bind(null).bind(null).name", "bound bound bound f")]
+    [TestCase("(function () {}).bind(null).name", "bound ")]
+    [TestCase("Object.getOwnPropertyDescriptor({ get x() {} }, 'x').get.name", "get x")]
+    [TestCase("Object.getOwnPropertyDescriptor({ set x(v) {} }, 'x').set.name", "set x")]
+    [TestCase("Object.getOwnPropertyDescriptor({ get [Symbol.iterator]() {} }, Symbol.iterator).get.name", "get [Symbol.iterator]")]
+    public void APrefixedNameIsTheSameTextItAlwaysWas(string source, string expected)
+    {
+        new Engine().Evaluate(source).AsString().Should().Be(expected);
+    }
+}

Jint/Native/Function/Function.cs

@@ -432,12 +432,56 @@ internal void SetFunctionName(JsValue name, string? prefix = null, bool force =
 
         if (!string.IsNullOrWhiteSpace(prefix))
         {
-            name = prefix + " " + name;
+            name = PrefixName(prefix!, name);
         }
 
         _nameDescriptor = new PropertyDescriptor(name, PropertyFlag.Configurable);
     }
 
+    // The three prefixes SetFunctionName is ever called with, each already carrying the separating
+    // space, so the common case concatenates two values the engine already holds.
+    private static readonly JsString _boundPrefix = JsString.CachedCreate("bound ");
+    private static readonly JsString _getPrefix = JsString.CachedCreate("get ");
+    private static readonly JsString _setPrefix = JsString.CachedCreate("set ");
+
+    /// <summary>
+    /// Produces step 4's <c>prefix, " ", name</c> concatenation of
+    /// <see href="https://tc39.es/ecma262/#sec-setfunctionname">SetFunctionName</see>.
+    /// </summary>
+    /// <remarks>
+    /// Deferred rather than copied, because the result can itself be the next call's <c>name</c>. A CLR
+    /// <c>string</c> concatenation flattens whatever it is handed, so binding an already-bound function
+    /// materialized <c>"bound bound … f"</c> afresh at every level and left that copy in the level's own
+    /// name descriptor: N binds retained Σ 6·i characters, about 30 GB at a depth of 100,000
+    /// (<see href="https://github.com/sebastienros/jint/issues/4129">#4129</see>).
+    /// <see cref="JsString.Concat"/> builds a node over the level below instead — O(1) per level,
+    /// flattened once if something ever reads the text — and yields the same characters either way.
+    /// </remarks>
+    private static JsString PrefixName(string prefix, JsValue name)
+    {
+        if (name is not JsString jsName)
+        {
+            return JsString.Create(prefix + " " + name);
+        }
+
+        var prefixWithSeparator = prefix switch
+        {
+            "bound" => _boundPrefix,
+            "get" => _getPrefix,
+            "set" => _setPrefix,
+            _ => JsString.Create(prefix + " "),
+        };
+
+        if ((long) prefixWithSeparator.Length + jsName.Length > JsString.MaxLength)
+        {
+            // Concat's caller owes it a result that fits. Only a host-supplied string reaches that
+            // length, and for it the flat concatenation is what this path has always produced.
+            return JsString.Create(prefix + " " + name);
+        }
+
+        return JsString.Concat(prefixWithSeparator, jsName);
+    }
+
     /// <summary>
     /// https://tc39.es/ecma262/#sec-ordinarycreatefromconstructor
     /// </summary>