From 1de7096b42bbb6f147d3dfc2e4b07b6fa972e2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 13 Dec 2020 10:57:30 -0500 Subject: [PATCH 1/5] Use WeakReference instead of WeakReference --- .../IronPython/Runtime/Types/PythonType.cs | 54 +++++++++---------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/core/IronPython/Runtime/Types/PythonType.cs b/src/core/IronPython/Runtime/Types/PythonType.cs index e8c78ade8..a64b404b6 100644 --- a/src/core/IronPython/Runtime/Types/PythonType.cs +++ b/src/core/IronPython/Runtime/Types/PythonType.cs @@ -39,7 +39,7 @@ public partial class PythonType : IPythonMembersList, IDynamicMetaObjectProvider private PythonTypeAttributes _attrs; // attributes of the type private int _flags; // CPython-like flags on the type private int _version = GetNextVersion(); // version of the type - private List _subtypes; // all of the subtypes of the PythonType + private List> _subtypes; // all of the subtypes of the PythonType private PythonContext _pythonContext; // the context the type was created from, or null for system types. private bool? _objectNew, _objectInit; // true if the type doesn't override __new__ / __init__ from object. internal Dictionary _cachedGets; // cached gets on user defined type instances @@ -76,7 +76,6 @@ public partial class PythonType : IPythonMembersList, IDynamicMetaObjectProvider [MultiRuntimeAware] private static int MasterVersion = 1; private static readonly CommonDictionaryStorage _pythonTypes = new CommonDictionaryStorage(); - private static readonly WeakReference[] _emptyWeakRef = Array.Empty(); private static readonly object _subtypesLock = new object(); internal static readonly Func DefaultMakeException = (message, innerException) => new Exception(message, innerException); internal static readonly Func DefaultMakeExceptionNoInnerException = (message) => new Exception(message); @@ -681,17 +680,15 @@ public void __setattr__(CodeContext/*!*/ context, string name, object value) { public PythonList __subclasses__(CodeContext/*!*/ context) { PythonList ret = new PythonList(); - IList subtypes = SubTypes; + var subtypes = SubTypes; if (subtypes != null) { PythonContext pc = context.LanguageContext; - foreach (WeakReference wr in subtypes) { - if (wr.IsAlive) { - PythonType pt = (PythonType)wr.Target; - + foreach (var wr in subtypes) { + if (wr.TryGetTarget(out PythonType pt)) { if (pt.PythonContext == null || pt.PythonContext == pc) { - ret.AddNoLock(wr.Target); + ret.AddNoLock(pt); } } } @@ -1271,8 +1268,8 @@ internal void AddSlot(string name, PythonTypeSlot slot) { private void ClearObjectNewInSubclasses(PythonType pt) { lock (_subtypesLock) { if (pt._subtypes != null) { - foreach (WeakReference wr in pt._subtypes) { - if (wr.Target is PythonType type) { + foreach (var wr in pt._subtypes) { + if (wr.TryGetTarget(out PythonType type)) { type._objectNew = null; ClearObjectNewInSubclasses(type); @@ -1285,8 +1282,8 @@ private void ClearObjectNewInSubclasses(PythonType pt) { private void ClearObjectInitInSubclasses(PythonType pt) { lock (_subtypesLock) { if (pt._subtypes != null) { - foreach (WeakReference wr in pt._subtypes) { - if (wr.Target is PythonType type) { + foreach (var wr in pt._subtypes) { + if (wr.TryGetTarget(out PythonType type)) { type._objectInit = null; ClearObjectInitInSubclasses(type); @@ -2459,9 +2456,9 @@ private void EnsureInstanceCtor() { #region Private implementation details private void UpdateVersion() { - foreach (WeakReference wr in SubTypes) { - if (wr.IsAlive) { - ((PythonType)wr.Target).UpdateVersion(); + foreach (var wr in SubTypes) { + if (wr.TryGetTarget(out PythonType pt)) { + pt.UpdateVersion(); } } @@ -2497,25 +2494,26 @@ private void EnsureDict() { /// private void AddSubType(PythonType subtype) { if (_subtypes == null) { - Interlocked.CompareExchange>(ref _subtypes, new List(), null); + Interlocked.CompareExchange(ref _subtypes, new List>(), null); } lock (_subtypesLock) { - _subtypes.Add(new WeakReference(subtype)); + _subtypes.Add(new WeakReference(subtype)); } } private void RemoveSubType(PythonType subtype) { - int i = 0; - if (_subtypes != null) { - lock (_subtypesLock) { - while (i < _subtypes.Count) { - if (!_subtypes[i].IsAlive || _subtypes[i].Target == subtype) { - _subtypes.RemoveAt(i); - continue; - } - i++; + if (_subtypes is null) return; + + lock (_subtypesLock) { + int i = 0; + while (i < _subtypes.Count) { + var wr = _subtypes[i]; + if (!wr.TryGetTarget(out PythonType target) || target == subtype) { + _subtypes.RemoveAt(i); + continue; } + i++; } } } @@ -2524,9 +2522,9 @@ private void RemoveSubType(PythonType subtype) { /// Gets a list of weak references to all the subtypes of this class. May return null /// if there are no subtypes of the class. /// - private IList SubTypes { + private IList> SubTypes { get { - if (_subtypes == null) return _emptyWeakRef; + if (_subtypes == null) return Array.Empty>(); lock (_subtypesLock) { return _subtypes.ToArray(); From 4c2df1ba04b7bfc0879ddf222e16a4c4d62ba951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 13 Dec 2020 10:57:47 -0500 Subject: [PATCH 2/5] Enable test_memory --- src/core/IronPython/Runtime/Types/PythonType.cs | 1 + tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini | 6 ------ tests/suite/test_memory.py | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/core/IronPython/Runtime/Types/PythonType.cs b/src/core/IronPython/Runtime/Types/PythonType.cs index a64b404b6..be7a22ec8 100644 --- a/src/core/IronPython/Runtime/Types/PythonType.cs +++ b/src/core/IronPython/Runtime/Types/PythonType.cs @@ -2498,6 +2498,7 @@ private void AddSubType(PythonType subtype) { } lock (_subtypesLock) { + _subtypes.RemoveAll(x => !x.TryGetTarget(out _)); // remove dead entries _subtypes.Add(new WeakReference(subtype)); } } diff --git a/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini b/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini index 8fbc70f1b..b2ea0205e 100644 --- a/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini +++ b/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini @@ -65,12 +65,6 @@ Reason=New test needs to be written for new csharp version Ignore=true Reason=Assertion error -[IronPython.test_memory] -#RunCondition=NOT $(IS_POSIX) -#Reason=Memory allocation on Mono may not match MS.NET -Ignore=true -Reason=Fails intermittently - https://github.com/IronLanguages/ironpython3/issues/508 - [IronPython.test_number] Timeout=300000 # 5 minute timeout - slow on macOS diff --git a/tests/suite/test_memory.py b/tests/suite/test_memory.py index 48887d7d6..e4493b1a4 100644 --- a/tests/suite/test_memory.py +++ b/tests/suite/test_memory.py @@ -82,9 +82,9 @@ def setUp(self): from Microsoft.Scripting.Generation import Snippets self.skipMemoryCheck = Snippets.Shared.SaveSnippets or clr.GetCurrentRuntime().Configuration.DebugMode - self.expectedMem = 24000 # account for adaptive compilation + self.expectedMem = 24000 if is_cli64: self.expectedMem = int(self.expectedMem*1.25) From ab116ff79f115b2a6630411a3ca6201d4c3ba868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 13 Dec 2020 11:32:27 -0500 Subject: [PATCH 3/5] Set IsolationLevel PROCESS --- tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini b/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini index b2ea0205e..11aa1a1e8 100644 --- a/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini +++ b/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini @@ -65,6 +65,9 @@ Reason=New test needs to be written for new csharp version Ignore=true Reason=Assertion error +[IronPython.test_memory] +IsolationLevel=PROCESS # to avoid having a long list of object subtypes + [IronPython.test_number] Timeout=300000 # 5 minute timeout - slow on macOS From 7a927723b17032d278552acec71a3713f99a135b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 13 Dec 2020 12:40:43 -0500 Subject: [PATCH 4/5] Don't run on Mono --- tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini b/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini index 11aa1a1e8..af214b748 100644 --- a/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini +++ b/tests/IronPython.Tests/Cases/IronPythonCasesManifest.ini @@ -67,6 +67,8 @@ Reason=Assertion error [IronPython.test_memory] IsolationLevel=PROCESS # to avoid having a long list of object subtypes +RunCondition=NOT $(IS_MONO) +Reason=Memory allocation on Mono may not match MS.NET [IronPython.test_number] Timeout=300000 # 5 minute timeout - slow on macOS From 20c5732ed2e934a422c0ba21dd14ed676c659069 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Tue, 21 Jun 2022 09:12:20 -0400 Subject: [PATCH 5/5] Only clean up on Capacity change --- src/core/IronPython/Runtime/Types/PythonType.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/IronPython/Runtime/Types/PythonType.cs b/src/core/IronPython/Runtime/Types/PythonType.cs index be7a22ec8..0539d758f 100644 --- a/src/core/IronPython/Runtime/Types/PythonType.cs +++ b/src/core/IronPython/Runtime/Types/PythonType.cs @@ -2498,7 +2498,9 @@ private void AddSubType(PythonType subtype) { } lock (_subtypesLock) { - _subtypes.RemoveAll(x => !x.TryGetTarget(out _)); // remove dead entries + if (_subtypes.Count == _subtypes.Capacity) { + _subtypes.RemoveAll(x => !x.TryGetTarget(out _)); // remove dead entries + } _subtypes.Add(new WeakReference(subtype)); } }