DYN-4231 Fast Path for sweep during FullGC (#11923)

saintentropy · web-flow · commit 69cb60798c01 · 2021-11-08T11:29:06.000-05:00
* Add fast path to dispose void calls in Callsite and FFi function endpoint

* Add fast path in FFI dispose calls vs generic callr

* pre-allocated arguments list and cache FunctionEndPoint for repeated calls

* cache procedure node for repeated calls of the same type

* cache CallSite and remove unnecessary calls

* Remove dynamic strings from hot path of GC

* Update comments

* PR review comments

* Add legacy tests

* move to internal

* cleanup

* fix test

Co-authored-by: Craig Long &lt;craig.long@autodesk.com&gt;
diff --git a/src/Engine/ProtoCore/DSASM/Executive.cs b/src/Engine/ProtoCore/DSASM/Executive.cs
@@ -817,6 +817,32 @@ public StackValue Callr(int blockDeclId,
             return sv;
         }
 
+        // cached values for repeated calls of the same object type and dispose function.
+        private int previousClassIndex;
+        private string previousProcedureName;
+        private CallSite callsite;
+
+        internal StackValue CallDispose(ProcedureNode fNode,
+                                StackValue svThisPtr,
+                                int classIndex)
+        {
+            if (null != Properties.executingGraphNode)
+            {
+                exe.ExecutingGraphnode = Properties.executingGraphNode;
+            }
+
+            if (callsite == null || classIndex != previousClassIndex || previousProcedureName != fNode.Name)
+            {
+                previousClassIndex = classIndex;
+                previousProcedureName = fNode.Name;
+                callsite = new CallSite(classIndex, fNode.Name, exe.FunctionTable, runtimeCore.Options.ExecutionMode);
+            }
+
+            Validity.Assert(null != callsite);
+
+            return callsite.DispatchDispose(svThisPtr, runtimeCore);
+        }
+
         private StackValue CallrForMemberFunction(int blockIndex,
                                                   int classIndex,
                                                   int procIndex,
diff --git a/src/Engine/ProtoCore/DSASM/Heap.cs b/src/Engine/ProtoCore/DSASM/Heap.cs
@@ -624,45 +624,62 @@ private bool TryFindFreeIndex(out int index)
             }
         }
 
+        //cache ClassIndex and ProcdureNode for repeated calls of the same type object.
+        private int previousClassIndex;
+        private ProcedureNode disposeProcedureNode;
+        private bool isDSObject;
+
         private void GCDisposeObject(StackValue svPtr, Executive exe)
         {
             int classIndex = svPtr.metaData.type;
-            ClassNode cn = exe.exe.classTable.ClassNodes[classIndex];
 
-            ProcedureNode pn = cn.GetDisposeMethod();
-            while (pn == null)
+            if (this.disposeProcedureNode == null || classIndex != previousClassIndex)
             {
-                if (cn.Base != Constants.kInvalidIndex)
-                {
-                    classIndex = cn.Base;
-                    cn = exe.exe.classTable.ClassNodes[classIndex];
-                    pn = cn.GetDisposeMethod();
-                }
-                else
+                previousClassIndex = classIndex;
+                ClassNode cn = exe.exe.classTable.ClassNodes[classIndex];
+
+                isDSObject = !string.IsNullOrEmpty(cn.ExternLib) && cn.ExternLib.Contains(".ds");
+
+                disposeProcedureNode = cn.GetDisposeMethod();
+                while (disposeProcedureNode == null)
                 {
-                    break;
+                    if (cn.Base != Constants.kInvalidIndex)
+                    {
+                        classIndex = cn.Base;
+                        cn = exe.exe.classTable.ClassNodes[classIndex];
+                        disposeProcedureNode = cn.GetDisposeMethod();
+                    }
+                    else
+                    {
+                        break;
+                    }
                 }
             }
 
-            if (pn != null)
+            //legacy dispose for design script objects.  This may be very rare
+            if (disposeProcedureNode != null && isDSObject)
             {
                 // TODO Jun/Jiong: Use build pointer utilities 
                 exe.rmem.Push(StackValue.BuildArrayDimension(0));
                 exe.rmem.Push(StackValue.BuildPointer(svPtr.Pointer, svPtr.metaData));
                 exe.rmem.Push(StackValue.BuildInt(1));
-                
+
                 ++exe.RuntimeCore.FunctionCallDepth;
 
                 // TODO: Need to move IsExplicitCall to DebugProps and come up with a more elegant solution for this
                 // fix for IDE-963 - pratapa
                 bool explicitCall = exe.IsExplicitCall;
                 bool tempFlag = explicitCall;
-                exe.Callr(pn.RuntimeIndex, pn.ID, classIndex, ref explicitCall);
+                exe.Callr(disposeProcedureNode.RuntimeIndex, disposeProcedureNode.ID, classIndex, ref explicitCall);
 
                 exe.IsExplicitCall = tempFlag;
 
                 --exe.RuntimeCore.FunctionCallDepth;
             }
+            else if (disposeProcedureNode != null)
+            {
+                exe.CallDispose(disposeProcedureNode, svPtr, classIndex);
+            }
         }
 
         /// <summary>
diff --git a/src/Engine/ProtoCore/Lang/CallSite.cs b/src/Engine/ProtoCore/Lang/CallSite.cs
@@ -1528,6 +1528,25 @@ private StackValue DispatchNew(
             return ret;
         }
 
+        //Pre-initialize array for repeated calls.  The StackValue is inserted vs making a new array for every call.
+        private static List<StackValue> disposeArguments = new List<StackValue>() { StackValue.Null };
+        //Cache final function endpoint for repeated calls;
+        private FunctionEndPoint finalFep;
+
+        internal StackValue DispatchDispose(StackValue stackValue, RuntimeCore runtimeCore)
+        {
+            //Cache finalFep for CallSite.  Note there is always only one dispose endpoint returned.
+            if (finalFep == null)
+            {
+                var funcGroup = FirstFunctionGroupInInheritanceChain(runtimeCore, classScope);
+                finalFep = funcGroup.FunctionEndPoints[0];
+            }
+            
+            disposeArguments[0] = stackValue;
+
+            //EXECUTE
+            return finalFep.Execute(null, disposeArguments, null, runtimeCore);
+        }
 
         private StackValue Execute(
             List<FunctionEndPoint> functionEndPoint,
diff --git a/src/Engine/ProtoCore/Lang/FFIFunctionEndPoint.cs b/src/Engine/ProtoCore/Lang/FFIFunctionEndPoint.cs
@@ -84,9 +84,7 @@ private void Init(RuntimeCore runtimeCore)
         }
 
         public override StackValue Execute(Runtime.Context c, List<StackValue> formalParameters, StackFrame stackFrame, RuntimeCore runtimeCore)
-        {   
-            StackValue svThisPtr = stackFrame.ThisPtr;
-
+        {
             if (mInterpreter == null)
             {
                 Init(runtimeCore);
@@ -97,40 +95,45 @@ public override StackValue Execute(Runtime.Context c, List<StackValue> formalPar
                 return StackValue.Null;
             }
 
-                // Check if this is a 'this' pointer function overload that was generated by the compiler
-            if (null != mFNode && mFNode.IsAutoGeneratedThisProc)
+            if (stackFrame != null)
             {
-                int thisPtrIndex = 0;
-                bool isStaticCall = svThisPtr.IsPointer && Constants.kInvalidPointer == svThisPtr.Pointer;
-                if (isStaticCall)
-                {
-                    stackFrame.ThisPtr = formalParameters[thisPtrIndex];
-                }
+                StackValue svThisPtr = stackFrame.ThisPtr;
 
-                // Comment Jun: Execute() can handle a null this pointer. 
-                // But since we don't even need to to reach there if we don't have a valid this pointer, then just return null
-                if (formalParameters[thisPtrIndex].IsNull)
-                {
-                    runtimeCore.RuntimeStatus.LogWarning(
-                        Runtime.WarningID.DereferencingNonPointer, Resources.kDeferencingNonPointer);
-                    return StackValue.Null;
-                }
-
-                // These are the op types allowed. 
-                Validity.Assert(formalParameters[thisPtrIndex].IsPointer ||
-                                formalParameters[thisPtrIndex].IsDefaultArgument);
-
-                // Make sure we to pass a pointer to unmarshal.
-                if (formalParameters[thisPtrIndex].IsPointer)
+                // Check if this is a 'this' pointer function overload that was generated by the compiler
+                if (null != mFNode && mFNode.IsAutoGeneratedThisProc)
                 {
-                    svThisPtr = formalParameters[thisPtrIndex];
+                    int thisPtrIndex = 0;
+                    bool isStaticCall = svThisPtr.IsPointer && Constants.kInvalidPointer == svThisPtr.Pointer;
+                    if (isStaticCall)
+                    {
+                        stackFrame.ThisPtr = formalParameters[thisPtrIndex];
+                    }
+
+                    // Comment Jun: Execute() can handle a null this pointer. 
+                    // But since we don't even need to to reach there if we don't have a valid this pointer, then just return null
+                    if (formalParameters[thisPtrIndex].IsNull)
+                    {
+                        runtimeCore.RuntimeStatus.LogWarning(
+                            Runtime.WarningID.DereferencingNonPointer, Resources.kDeferencingNonPointer);
+                        return StackValue.Null;
+                    }
+
+                    // These are the op types allowed. 
+                    Validity.Assert(formalParameters[thisPtrIndex].IsPointer ||
+                                    formalParameters[thisPtrIndex].IsDefaultArgument);
+
+                    // Make sure we to pass a pointer to unmarshal.
+                    if (formalParameters[thisPtrIndex].IsPointer)
+                    {
+                        svThisPtr = formalParameters[thisPtrIndex];
+                    }
+
+                    formalParameters.RemoveAt(thisPtrIndex);
                 }
 
-                formalParameters.RemoveAt(thisPtrIndex);
+                formalParameters.Add(svThisPtr);
             }
 
-            formalParameters.Add(svThisPtr);
-
             Object ret = mFunctionPointer.Execute(c, mInterpreter, formalParameters);
             StackValue op;
             if (ret == null)
diff --git a/test/Engine/ProtoTest/MultiLangTests/GCTest.cs b/test/Engine/ProtoTest/MultiLangTests/GCTest.cs
@@ -12,6 +12,42 @@ public override void Setup()
             FFITarget.DisposeCounter.Reset(0);
         }
 
+        [Test]
+        public void T00_TestGCEndofForBlk()
+        {
+            string code = @"
+import(""DisposeVerify.ds"");
+v1;
+v2;
+v3;
+[Imperative]
+{
+  
+DisposeVerify.x = 1;
+arr = [ A.A(), A.A(), A.A() ];
+[Associative]
+{
+    [Imperative]
+    {
+        for(i in arr)
+        {
+            mm = i;
+            mm2 = A.A();
+        }
+        v1 = DisposeVerify.x; // 3
+    }
+}
+v2 = DisposeVerify.x; // 4
+arr = null;
+v3 = DisposeVerify.x; // 7
+}
+__GC();
+v4 = DisposeVerify.x;
+";
+            ExecutionMirror mirror = thisTest.RunScriptSource(code, "", testCasePath);
+            thisTest.Verify("v4", 7);
+        }
+
         [Test]
         [Category("DSDefinedClass_Ported")]
         public void T01_TestGCArray()
