From 259d283b33239eeeb93d577069a03543636c097d Mon Sep 17 00:00:00 2001 From: Martin-Molinero Date: Fri, 3 Mar 2023 11:03:07 -0300 Subject: [PATCH 1/2] Remove managed reference tracking - Remove unrequired reference tracking causing exceptions on shutdown and a performance overhead --- src/runtime/Runtime.cs | 8 --- src/runtime/StateSerialization/RuntimeData.cs | 49 ------------------- src/runtime/Types/ClassBase.cs | 7 +-- src/runtime/Types/ClrObject.cs | 7 --- src/runtime/Types/ExtensionType.cs | 11 +---- 5 files changed, 2 insertions(+), 80 deletions(-) diff --git a/src/runtime/Runtime.cs b/src/runtime/Runtime.cs index effbe2935..8634b85d2 100644 --- a/src/runtime/Runtime.cs +++ b/src/runtime/Runtime.cs @@ -280,7 +280,6 @@ internal static void Shutdown() ClearClrModules(); RemoveClrRootModule(); - NullGCHandles(ExtensionType.loadedExtensions); ClassManager.RemoveClasses(); TypeManager.RemoveTypes(); _typesInitialized = false; @@ -319,8 +318,6 @@ internal static void Shutdown() PyEval_SaveThread(); } - ExtensionType.loadedExtensions.Clear(); - CLRObject.reflectedObjects.Clear(); } else { @@ -349,11 +346,6 @@ static bool TryCollectingGarbage(int runs, bool forceBreakLoops) { if (attempt + 1 == runs) return true; } - else if (forceBreakLoops) - { - NullGCHandles(CLRObject.reflectedObjects); - CLRObject.reflectedObjects.Clear(); - } } return false; } diff --git a/src/runtime/StateSerialization/RuntimeData.cs b/src/runtime/StateSerialization/RuntimeData.cs index 065f3718a..a60796a87 100644 --- a/src/runtime/StateSerialization/RuntimeData.cs +++ b/src/runtime/StateSerialization/RuntimeData.cs @@ -140,57 +140,9 @@ static bool CheckSerializable (object o) private static SharedObjectsState SaveRuntimeDataObjects() { var contexts = new Dictionary>(PythonReferenceComparer.Instance); - var extensionObjs = new Dictionary(PythonReferenceComparer.Instance); - // make a copy with strongly typed references to avoid concurrent modification - var extensions = ExtensionType.loadedExtensions - .Select(addr => new PyObject( - new BorrowedReference(addr), - // if we don't skip collect, finalizer might modify loadedExtensions - skipCollect: true)) - .ToArray(); - foreach (var pyObj in extensions) - { - var extension = (ExtensionType)ManagedType.GetManagedObject(pyObj)!; - Debug.Assert(CheckSerializable(extension)); - var context = extension.Save(pyObj); - if (context is not null) - { - contexts[pyObj] = context; - } - extensionObjs.Add(pyObj, extension); - } var wrappers = new Dictionary>(); var userObjects = new CLRWrapperCollection(); - // make a copy with strongly typed references to avoid concurrent modification - var reflectedObjects = CLRObject.reflectedObjects - .Select(addr => new PyObject( - new BorrowedReference(addr), - // if we don't skip collect, finalizer might modify reflectedObjects - skipCollect: true)) - .ToList(); - foreach (var pyObj in reflectedObjects) - { - // Wrapper must be the CLRObject - var clrObj = (CLRObject)ManagedType.GetManagedObject(pyObj)!; - object inst = clrObj.inst; - List mappedObjs; - if (!userObjects.TryGetValue(inst, out var item)) - { - item = new CLRMappedItem(inst); - userObjects.Add(item); - - Debug.Assert(!wrappers.ContainsKey(inst)); - mappedObjs = new List(); - wrappers.Add(inst, mappedObjs); - } - else - { - mappedObjs = wrappers[inst]; - } - item.AddRef(pyObj); - mappedObjs.Add(clrObj); - } var wrapperStorage = new Dictionary(); WrappersStorer?.Store(userObjects, wrapperStorage); @@ -215,7 +167,6 @@ private static SharedObjectsState SaveRuntimeDataObjects() return new() { InternalStores = internalStores, - Extensions = extensionObjs, Wrappers = wrapperStorage, Contexts = contexts, }; diff --git a/src/runtime/Types/ClassBase.cs b/src/runtime/Types/ClassBase.cs index 6066e5fec..83406bb1c 100644 --- a/src/runtime/Types/ClassBase.cs +++ b/src/runtime/Types/ClassBase.cs @@ -352,12 +352,7 @@ public static int tp_clear(BorrowedReference ob) Runtime.PyObject_ClearWeakRefs(ob); } - if (TryFreeGCHandle(ob)) - { - IntPtr addr = ob.DangerousGetAddress(); - bool deleted = CLRObject.reflectedObjects.Remove(addr); - Debug.Assert(deleted); - } + TryFreeGCHandle(ob); int baseClearResult = BaseUnmanagedClear(ob); if (baseClearResult != 0) diff --git a/src/runtime/Types/ClrObject.cs b/src/runtime/Types/ClrObject.cs index db6e99121..cabcca682 100644 --- a/src/runtime/Types/ClrObject.cs +++ b/src/runtime/Types/ClrObject.cs @@ -11,8 +11,6 @@ internal sealed class CLRObject : ManagedType { internal readonly object inst; - // "borrowed" references - internal static readonly HashSet reflectedObjects = new(); static NewReference Create(object ob, BorrowedReference tp) { Debug.Assert(tp != null); @@ -23,8 +21,6 @@ static NewReference Create(object ob, BorrowedReference tp) GCHandle gc = GCHandle.Alloc(self); InitGCHandle(py.Borrow(), type: tp, gc); - bool isNew = reflectedObjects.Add(py.DangerousGetAddress()); - Debug.Assert(isNew); // Fix the BaseException args (and __cause__ in case of Python 3) // slot if wrapping a CLR exception @@ -64,9 +60,6 @@ protected override void OnLoad(BorrowedReference ob, Dictionary base.OnLoad(ob, context); GCHandle gc = GCHandle.Alloc(this); SetGCHandle(ob, gc); - - bool isNew = reflectedObjects.Add(ob.DangerousGetAddress()); - Debug.Assert(isNew); } } } diff --git a/src/runtime/Types/ExtensionType.cs b/src/runtime/Types/ExtensionType.cs index 439bd3314..5eed8a500 100644 --- a/src/runtime/Types/ExtensionType.cs +++ b/src/runtime/Types/ExtensionType.cs @@ -42,16 +42,11 @@ public virtual NewReference Alloc() public PyObject AllocObject() => new PyObject(Alloc().Steal()); - // "borrowed" references - internal static readonly HashSet loadedExtensions = new(); void SetupGc (BorrowedReference ob, BorrowedReference tp) { GCHandle gc = GCHandle.Alloc(this); InitGCHandle(ob, tp, gc); - bool isNew = loadedExtensions.Add(ob.DangerousGetAddress()); - Debug.Assert(isNew); - // We have to support gc because the type machinery makes it very // hard not to - but we really don't have a need for it in most // concrete extension types, so untrack the object to save calls @@ -92,11 +87,7 @@ public static int tp_clear(BorrowedReference ob) Runtime.PyObject_ClearWeakRefs(ob); } - if (TryFreeGCHandle(ob)) - { - bool deleted = loadedExtensions.Remove(ob.DangerousGetAddress()); - Debug.Assert(deleted); - } + TryFreeGCHandle(ob); int res = ClassBase.BaseUnmanagedClear(ob); return res; From f4190fdb9131360bd029e9bd5f21196415d9fc99 Mon Sep 17 00:00:00 2001 From: Martin-Molinero Date: Fri, 3 Mar 2023 11:42:19 -0300 Subject: [PATCH 2/2] Bump version to 2.0.18 --- src/perf_tests/Python.PerformanceTests.csproj | 4 ++-- src/runtime/Properties/AssemblyInfo.cs | 4 ++-- src/runtime/Python.Runtime.csproj | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/perf_tests/Python.PerformanceTests.csproj b/src/perf_tests/Python.PerformanceTests.csproj index cc2b83e05..a05bd3f9d 100644 --- a/src/perf_tests/Python.PerformanceTests.csproj +++ b/src/perf_tests/Python.PerformanceTests.csproj @@ -13,7 +13,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + compile @@ -25,7 +25,7 @@ - + diff --git a/src/runtime/Properties/AssemblyInfo.cs b/src/runtime/Properties/AssemblyInfo.cs index b8481e7cb..4d739394c 100644 --- a/src/runtime/Properties/AssemblyInfo.cs +++ b/src/runtime/Properties/AssemblyInfo.cs @@ -4,5 +4,5 @@ [assembly: InternalsVisibleTo("Python.EmbeddingTest, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")] [assembly: InternalsVisibleTo("Python.Test, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")] -[assembly: AssemblyVersion("2.0.17")] -[assembly: AssemblyFileVersion("2.0.17")] +[assembly: AssemblyVersion("2.0.18")] +[assembly: AssemblyFileVersion("2.0.18")] diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index dc773267a..66e815afa 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -5,7 +5,7 @@ Python.Runtime Python.Runtime QuantConnect.pythonnet - 2.0.17 + 2.0.18 false LICENSE https://github.com/pythonnet/pythonnet