From 69cb60798c01ca3e1a92f2f408a7d2bbd53d1798 Mon Sep 17 00:00:00 2001 From: Craig Long Date: Mon, 8 Nov 2021 12:29:06 -0400 Subject: [PATCH] DYN-4231 Fast Path for sweep during FullGC (#11923) * 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 --- src/Engine/ProtoCore/DSASM/Executive.cs | 26 ++++++++ src/Engine/ProtoCore/DSASM/Heap.cs | 45 ++++++++----- src/Engine/ProtoCore/Lang/CallSite.cs | 19 ++++++ .../ProtoCore/Lang/FFIFunctionEndPoint.cs | 63 ++++++++++--------- .../Engine/ProtoTest/MultiLangTests/GCTest.cs | 36 +++++++++++ 5 files changed, 145 insertions(+), 44 deletions(-) diff --git a/src/Engine/ProtoCore/DSASM/Executive.cs b/src/Engine/ProtoCore/DSASM/Executive.cs index aab893d46ca..3fd942eb46a 100644 --- 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 index e711f0ee27f..dd7e65fcbe7 100644 --- 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); + } } /// diff --git a/src/Engine/ProtoCore/Lang/CallSite.cs b/src/Engine/ProtoCore/Lang/CallSite.cs index e9cc9821453..4c6e99d26cf 100644 --- 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 disposeArguments = new List() { 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, diff --git a/src/Engine/ProtoCore/Lang/FFIFunctionEndPoint.cs b/src/Engine/ProtoCore/Lang/FFIFunctionEndPoint.cs index e4b1c1286b2..7b632e56d9a 100644 --- 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 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 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 index cdb2d91bd02..5375b3519cd 100644 --- 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()