Quellcode durchsuchen

Allow garbage collector to collect unused memory (#2196)

* Allow garbage collector to collect unused memory
* Added polyfill for Array.Fill for full framework
* Polyfill also used for dotnet standard 2.0
tomatosalat0 vor 1 Monat
Ursprung
Commit
44144972d1

+ 65 - 0
Jint.Tests/Runtime/GarbageCollectionTests.cs

@@ -0,0 +1,65 @@
+namespace Jint.Tests.Runtime;
+
+// This needs to run without any parallelization because it uses
+// garbage collector metrics which cannot be isolated.
+[CollectionDefinition(nameof(GarbageCollectionTests), DisableParallelization = true)]
+[Collection(nameof(GarbageCollectionTests))]
+public class GarbageCollectionTests
+{
+    [Fact]
+    public void InternalCachingDoesNotPreventGarbageCollection()
+    {
+        // This test ensures that memory allocated within functions
+        // can be garbage collected by the .NET runtime. To test that,
+        // the "allocate" functions allocates a big chunk of memory,
+        // which is not used anywhere. So the GC should have no problem
+        // releasing that memory after the "allocate" function leaves.
+
+        // Arrange
+        var engine = new Engine();
+        const string script =
+            """
+            function allocate(runAllocation) {
+                if (runAllocation) {
+                    // Allocate ~200 MB of data (not 100 because .NET uses UTF-16 for strings)
+                    var test = Array.from({ length: 100 })
+                        .map(() => ' '.repeat(1 * 1024 * 1024));
+                }
+                return 2;
+            }
+            """;
+        engine.Evaluate(script);
+
+        // Create a baseline for memory usage.
+        engine.Evaluate("allocate(false);");
+        var usedMemoryBytesBaseline = CurrentlyUsedMemory();
+
+        // Act
+        engine.Evaluate("allocate(true);");
+
+        // Assert
+        var usedMemoryBytesAfterJsScript = CurrentlyUsedMemory();
+        var epsilon = 10 * 1024 * 1024; // allowing up to 10 MB of other allocations should be enough to prevent false positives
+        Assert.True(
+            usedMemoryBytesAfterJsScript - usedMemoryBytesBaseline < epsilon,
+            userMessage: $"""
+                          The garbage collector did not free the allocated but unreachable 200 MB from the script.;
+                          Before Call : {BytesToString(usedMemoryBytesBaseline)}
+                          After Call  : {BytesToString(usedMemoryBytesAfterJsScript)}
+                          ---
+                          Acceptable  : {BytesToString(usedMemoryBytesBaseline + epsilon)}
+                          """);
+        return;
+
+        static string BytesToString(long bytes)
+            => $"{(bytes / 1024.0 / 1024.0),6:0.0} MB";
+
+        static long CurrentlyUsedMemory()
+        {
+            // Just try to ensure that everything possible gets collected.
+            GC.Collect(2, GCCollectionMode.Forced, blocking: true);
+            var currentlyUsed = GC.GetTotalMemory(forceFullCollection: true);
+            return currentlyUsed;
+        }
+    }
+}

+ 45 - 3
Jint/Collections/RefStack.cs

@@ -49,8 +49,12 @@ internal sealed class RefStack<T> : IEnumerable<T> where T : struct
         return false;
         return false;
     }
     }
 
 
+    /// <summary>
+    /// Returns the last added item from the stack and decreases the stack size by one.
+    /// </summary>
+    /// <exception cref="InvalidOperationException">Is thrown if the stack is empty.</exception>
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public ref readonly T Pop()
+    public T Pop()
     {
     {
         if (_size == 0)
         if (_size == 0)
         {
         {
@@ -58,7 +62,34 @@ internal sealed class RefStack<T> : IEnumerable<T> where T : struct
         }
         }
 
 
         _size--;
         _size--;
-        return ref _array[_size];
+        // Create a copy of the value because it will get overridden
+        // in the statement.
+        var result = _array[_size];
+        // Ensure that the item can get garbage collected if possible.
+        _array[_size] = default;
+        return result;
+    }
+
+    /// <summary>
+    /// Same as <see cref="Pop"/> but without copying the value.
+    /// </summary>
+    /// <remarks>
+    /// Use this method when the return value is not used and
+    /// <typeparamref name="T"/> is a value type. In that case, this methods
+    /// avoids one value type copy compared to <see cref="Pop"/>.
+    /// </remarks>
+    /// <exception cref="InvalidOperationException">Is thrown if the stack is empty.</exception>
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public void PopAndDiscard()
+    {
+        if (_size == 0)
+        {
+            Throw.InvalidOperationException("stack is empty");
+        }
+
+        _size--;
+        // Ensure that the item can get garbage collected if possible.
+        _array[_size] = default;
     }
     }
 
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -113,7 +144,18 @@ internal sealed class RefStack<T> : IEnumerable<T> where T : struct
 
 
     public void Clear()
     public void Clear()
     {
     {
-        _size = 0;
+        if (_size > 0)
+        {
+#if NETFRAMEWORK || NETSTANDARD2_0
+            for (var i = 0; i < _size; i++)
+            {
+                _array[i] = default;
+            }
+#else
+            Array.Fill(_array, default, 0, _size);
+#endif
+            _size = 0;
+        }
     }
     }
 
 
     public Enumerator GetEnumerator()
     public Enumerator GetEnumerator()

+ 10 - 0
Jint/Pooling/JsValueArrayPool.cs

@@ -61,16 +61,26 @@ internal sealed class JsValueArrayPool
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public void ReturnArray(JsValue[] array)
     public void ReturnArray(JsValue[] array)
     {
     {
+        // Ensure that the array contents are cleared
+        // to allow garbage collecting the values of the
+        // array if possible. Only the array itself
+        // should be cached.
         if (array.Length == 1)
         if (array.Length == 1)
         {
         {
+            array[0] = null!;
             _poolArray1.Free(array);
             _poolArray1.Free(array);
         }
         }
         else if (array.Length == 2)
         else if (array.Length == 2)
         {
         {
+            array[0] = null!;
+            array[1] = null!;
             _poolArray2.Free(array);
             _poolArray2.Free(array);
         }
         }
         else if (array.Length == 3)
         else if (array.Length == 3)
         {
         {
+            array[0] = null!;
+            array[1] = null!;
+            array[2] = null!;
             _poolArray3.Free(array);
             _poolArray3.Free(array);
         }
         }
     }
     }

+ 1 - 1
Jint/Runtime/CallStack/JintCallStack.cs

@@ -80,7 +80,7 @@ internal sealed class JintCallStack
 
 
     public CallStackElement Pop()
     public CallStackElement Pop()
     {
     {
-        ref readonly var item = ref _stack.Pop();
+        var item = _stack.Pop();
         if (_statistics is not null)
         if (_statistics is not null)
         {
         {
             if (_statistics[item] == 0)
             if (_statistics[item] == 0)

+ 2 - 2
Jint/Runtime/ExecutionContextStack.cs

@@ -58,7 +58,7 @@ internal sealed class ExecutionContextStack
     public void Push(in ExecutionContext context) => _stack.Push(in context);
     public void Push(in ExecutionContext context) => _stack.Push(in context);
 
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public ref readonly ExecutionContext Pop() => ref _stack.Pop();
+    public void Pop() => _stack.PopAndDiscard();
 
 
     public IScriptOrModule? GetActiveScriptOrModule()
     public IScriptOrModule? GetActiveScriptOrModule()
     {
     {
@@ -91,4 +91,4 @@ internal sealed class ExecutionContextStack
 
 
         return null;
         return null;
     }
     }
-}
+}