From 9de6f7bd071f60c67b42847f33453c0d5bd61c7a Mon Sep 17 00:00:00 2001 From: dse Date: Thu, 31 Aug 2017 19:34:31 +0400 Subject: [PATCH 01/10] Runtime/Shutdown loop stores old caches and static variables. It's produces bugs when CPython freeing up enough objects. --- CHANGELOG.md | 2 ++ src/runtime/assemblymanager.cs | 8 +++----- src/runtime/classderived.cs | 9 ++++++++- src/runtime/classmanager.cs | 5 +++++ src/runtime/genericutil.cs | 6 ++++++ src/runtime/moduleobject.cs | 11 +++++++++++ src/runtime/pyscope.cs | 7 ++++++- src/runtime/runtime.cs | 7 +++++++ src/runtime/typemanager.cs | 6 +++++- 9 files changed, 53 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d408a8d1..54b84841e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ### Fixed +- Fixed secondary PythonEngine.Initialize call, all sensitive static variables now reseted. + This is a hidden bug. Once python cleaning up enough memory, objects from previous engine run becomes corrupted. - Fixed Visual Studio 2017 compat (#434) for setup.py - Fixed crash on exit of the Python interpreter if a python class derived from a .NET class has a `__namespace__` or `__assembly__` diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 06a4449a2..34d08f0ec 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -17,13 +17,14 @@ internal class AssemblyManager { // modified from event handlers below, potentially triggered from different .NET threads // therefore this should be a ConcurrentDictionary - private static ConcurrentDictionary> namespaces; + private static ConcurrentDictionary> namespaces = + new ConcurrentDictionary>(); //private static Dictionary> generics; private static AssemblyLoadEventHandler lhandler; private static ResolveEventHandler rhandler; // updated only under GIL? - private static Dictionary probed; + private static Dictionary probed = new Dictionary(32); // modified from event handlers below, potentially triggered from different .NET threads private static AssemblyList assemblies; @@ -40,9 +41,6 @@ private AssemblyManager() /// internal static void Initialize() { - namespaces = new ConcurrentDictionary>(); - probed = new Dictionary(32); - //generics = new Dictionary>(); assemblies = new AssemblyList(16); pypath = new List(16); diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index 16d3b99db..ec3734ea5 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -1,8 +1,9 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Reflection.Emit; +using System.Resources; using System.Runtime.InteropServices; using System.Threading.Tasks; @@ -32,6 +33,12 @@ static ClassDerivedObject() moduleBuilders = new Dictionary, ModuleBuilder>(); } + public static void Reset() + { + assemblyBuilders = new Dictionary(); + moduleBuilders = new Dictionary, ModuleBuilder>(); + } + internal ClassDerivedObject(Type tp) : base(tp) { } diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 6a9d40ebd..0b084a49d 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -34,6 +34,11 @@ static ClassManager() dtype = typeof(MulticastDelegate); } + public static void Reset() + { + cache = new Dictionary(128); + } + /// /// Return the ClassBase-derived instance that implements a particular /// reflected managed type, creating it if it doesn't yet exist. diff --git a/src/runtime/genericutil.cs b/src/runtime/genericutil.cs index 9772d082f..3a230e12c 100644 --- a/src/runtime/genericutil.cs +++ b/src/runtime/genericutil.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Resources; namespace Python.Runtime { @@ -20,6 +21,11 @@ static GenericUtil() mapping = new Dictionary>>(); } + public static void Reset() + { + mapping = new Dictionary>>(); + } + /// /// Register a generic type that appears in a given namespace. /// diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index e683026f9..0ec9900dd 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -328,6 +328,17 @@ public CLRModule() : base("clr") } } + public static void Reset() + { + hacked = false; + interactive_preload = true; + preload = false; + + // XXX Test performance of new features // + _SuppressDocs = false; + _SuppressOverloads = false; + } + /// /// The initializing of the preload hook has to happen as late as /// possible since sys.ps1 is created after the CLR module is diff --git a/src/runtime/pyscope.cs b/src/runtime/pyscope.cs index 67f93c6e2..8e6957855 100644 --- a/src/runtime/pyscope.cs +++ b/src/runtime/pyscope.cs @@ -537,10 +537,15 @@ public void Dispose() public class PyScopeManager { - public readonly static PyScopeManager Global = new PyScopeManager(); + public static PyScopeManager Global; private Dictionary NamedScopes = new Dictionary(); + internal static void Reset() + { + Global = new PyScopeManager(); + } + internal PyScope NewScope(string name) { if (name == null) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index abd0661a4..1625d36ea 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -221,6 +221,13 @@ internal static void Initialize() PyEval_InitThreads(); } + CLRModule.Reset(); + GenericUtil.Reset(); + PyScopeManager.Reset(); + ClassManager.Reset(); + ClassDerivedObject.Reset(); + TypeManager.Reset(); + IntPtr op; IntPtr dict; if (IsPython3) diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 6570ee083..ececcc79c 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Reflection; @@ -21,6 +21,10 @@ static TypeManager() cache = new Dictionary(128); } + public static void Reset() + { + cache = new Dictionary(128); + } /// /// Given a managed Type derived from ExtensionType, get the handle to From e9e5c60641574504255695b9828aece6aff642ec Mon Sep 17 00:00:00 2001 From: dse Date: Sat, 2 Sep 2017 22:51:58 +0400 Subject: [PATCH 02/10] Added all tests finalizer routine. --- src/embed_tests/Python.EmbeddingTest.csproj | 1 + src/embed_tests/TestsSuite.cs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 src/embed_tests/TestsSuite.cs diff --git a/src/embed_tests/Python.EmbeddingTest.csproj b/src/embed_tests/Python.EmbeddingTest.csproj index fe02b0526..dc5478575 100644 --- a/src/embed_tests/Python.EmbeddingTest.csproj +++ b/src/embed_tests/Python.EmbeddingTest.csproj @@ -102,6 +102,7 @@ + diff --git a/src/embed_tests/TestsSuite.cs b/src/embed_tests/TestsSuite.cs new file mode 100644 index 000000000..44ce2d4b8 --- /dev/null +++ b/src/embed_tests/TestsSuite.cs @@ -0,0 +1,18 @@ +using NUnit.Framework; +using Python.Runtime; + +namespace Python.EmbeddingTest +{ + [SetUpFixture] + public class TestsSuite + { + [OneTimeTearDown] + public void FinalCleanup() + { + if (PythonEngine.IsInitialized) + { + PythonEngine.Shutdown(); + } + } + } +} From 64acfcb34abac5c0fa3722eea44c758b96da9d49 Mon Sep 17 00:00:00 2001 From: dse Date: Sat, 2 Sep 2017 23:01:47 +0400 Subject: [PATCH 03/10] Bug: Py_Initialize/Py_Finalize calls during alive PythonEngine. Fixed. --- src/embed_tests/TestRuntime.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs index 2e0598da7..bd7e39a78 100644 --- a/src/embed_tests/TestRuntime.cs +++ b/src/embed_tests/TestRuntime.cs @@ -1,4 +1,4 @@ -using System; +using System; using NUnit.Framework; using Python.Runtime; @@ -6,10 +6,21 @@ namespace Python.EmbeddingTest { public class TestRuntime { + [OneTimeSetUp] + public void SetUp() + { + // We needs to ensure that no any engines are running. + if (PythonEngine.IsInitialized) + { + PythonEngine.Shutdown(); + } + } + [Test] public static void Py_IsInitializedValue() { - Runtime.Runtime.Py_Finalize(); // In case another test left it on. + // We defenitely knows that all engines are shut downed. + // Runtime.Runtime.Py_Finalize(); Assert.AreEqual(0, Runtime.Runtime.Py_IsInitialized()); Runtime.Runtime.Py_Initialize(); Assert.AreEqual(1, Runtime.Runtime.Py_IsInitialized()); From 7898833d2be9833021c20bbe606c3594fb728348 Mon Sep 17 00:00:00 2001 From: dse Date: Sat, 2 Sep 2017 22:48:05 +0400 Subject: [PATCH 04/10] Init/Shutdown state variable (IsFinalizing) fix. --- src/runtime/runtime.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 1625d36ea..164b1bfaa 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -221,6 +221,8 @@ internal static void Initialize() PyEval_InitThreads(); } + IsFinalizing = false; + CLRModule.Reset(); GenericUtil.Reset(); PyScopeManager.Reset(); From ed29200beba1a6f4918a25332a07d8b6695959dd Mon Sep 17 00:00:00 2001 From: dse Date: Wed, 6 Sep 2017 00:45:44 +0400 Subject: [PATCH 05/10] TestPythonEngineProperties fixes. --- src/embed_tests/TestPythonEngineProperties.cs | 40 +++++++++++++++++-- src/runtime/pythonengine.cs | 16 +++++--- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/embed_tests/TestPythonEngineProperties.cs b/src/embed_tests/TestPythonEngineProperties.cs index 01c6ae7e3..d95942577 100644 --- a/src/embed_tests/TestPythonEngineProperties.cs +++ b/src/embed_tests/TestPythonEngineProperties.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using NUnit.Framework; using Python.Runtime; @@ -109,18 +110,41 @@ public static void GetPythonHomeDefault() [Test] public void SetPythonHome() { + // We needs to ensure that engine was started and shutdown at least once before setting dummy home. + // Otherwise engine will not run with dummy path with random problem. + if (!PythonEngine.IsInitialized) + { + PythonEngine.Initialize(); + } + + PythonEngine.Shutdown(); + + var pythonHomeBackup = PythonEngine.PythonHome; + var pythonHome = "/dummypath/"; PythonEngine.PythonHome = pythonHome; PythonEngine.Initialize(); - Assert.AreEqual(pythonHome, PythonEngine.PythonHome); PythonEngine.Shutdown(); + + // Restoring valid pythonhome. + PythonEngine.PythonHome = pythonHomeBackup; } [Test] public void SetPythonHomeTwice() { + // We needs to ensure that engine was started and shutdown at least once before setting dummy home. + // Otherwise engine will not run with dummy path with random problem. + if (!PythonEngine.IsInitialized) + { + PythonEngine.Initialize(); + } + PythonEngine.Shutdown(); + + var pythonHomeBackup = PythonEngine.PythonHome; + var pythonHome = "/dummypath/"; PythonEngine.PythonHome = "/dummypath2/"; @@ -129,11 +153,20 @@ public void SetPythonHomeTwice() Assert.AreEqual(pythonHome, PythonEngine.PythonHome); PythonEngine.Shutdown(); + + PythonEngine.PythonHome = pythonHomeBackup; } [Test] public void SetProgramName() { + if (PythonEngine.IsInitialized) + { + PythonEngine.Shutdown(); + } + + var programNameBackup = PythonEngine.ProgramName; + var programName = "FooBar"; PythonEngine.ProgramName = programName; @@ -141,6 +174,8 @@ public void SetProgramName() Assert.AreEqual(programName, PythonEngine.ProgramName); PythonEngine.Shutdown(); + + PythonEngine.ProgramName = programNameBackup; } [Test] @@ -156,7 +191,7 @@ public void SetPythonPath() string path = PythonEngine.PythonPath; PythonEngine.Shutdown(); - PythonEngine.ProgramName = path; + PythonEngine.PythonPath = path; PythonEngine.Initialize(); Assert.AreEqual(path, PythonEngine.PythonPath); @@ -171,7 +206,6 @@ public void SetPythonPathExceptionOn27() Assert.Pass(); } - // Get previous path to avoid crashing Python PythonEngine.Initialize(); string path = PythonEngine.PythonPath; PythonEngine.Shutdown(); diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index a23c7ac79..76e954ac5 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -295,12 +295,16 @@ public static void Shutdown() if (initialized) { PyScopeManager.Global.Clear(); - Marshal.FreeHGlobal(_pythonHome); - _pythonHome = IntPtr.Zero; - Marshal.FreeHGlobal(_programName); - _programName = IntPtr.Zero; - Marshal.FreeHGlobal(_pythonPath); - _pythonPath = IntPtr.Zero; + // We should not release memory for variables that can be used without initialized python engine. + // It's assumed that Py_GetPythonHome returns valid string without engine initialize. Py_GetPythonHome will always return the + // same pointer that was passed before to Py_SetPythonHome and stored in the _pythonHome. + + ////Marshal.FreeHGlobal(_pythonHome); + ////_pythonHome = IntPtr.Zero; + ////Marshal.FreeHGlobal(_programName); + ////_programName = IntPtr.Zero; + ////Marshal.FreeHGlobal(_pythonPath); + ////_pythonPath = IntPtr.Zero; Runtime.Shutdown(); initialized = false; From 507edc9610f472ea72fb666979742d156e762887 Mon Sep 17 00:00:00 2001 From: dse Date: Tue, 10 Oct 2017 14:45:45 +0200 Subject: [PATCH 06/10] Update TestRuntime.cs --- src/embed_tests/TestRuntime.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/embed_tests/TestRuntime.cs b/src/embed_tests/TestRuntime.cs index bd7e39a78..9dfb04c9c 100644 --- a/src/embed_tests/TestRuntime.cs +++ b/src/embed_tests/TestRuntime.cs @@ -19,7 +19,7 @@ public void SetUp() [Test] public static void Py_IsInitializedValue() { - // We defenitely knows that all engines are shut downed. + // We know for sure that all engines are shut down. // Runtime.Runtime.Py_Finalize(); Assert.AreEqual(0, Runtime.Runtime.Py_IsInitialized()); Runtime.Runtime.Py_Initialize(); From 741070b2acc1e53bd41603ba94f0276f2083232a Mon Sep 17 00:00:00 2001 From: dse Date: Mon, 3 Jul 2017 09:04:36 +0400 Subject: [PATCH 07/10] Python string to CLR string marshaling cache added. --- src/runtime/Python.Runtime.csproj | 6 + .../EncodedStringsFifoDictionary.cs | 73 +++++++++ .../perf_utils/EncodingGetStringPolyfill.cs | 52 +++++++ src/runtime/perf_utils/FifoDictionary.cs | 62 ++++++++ .../perf_utils/RawImmutableMemBlock.cs | 84 +++++++++++ src/runtime/perf_utils/RawMemUtils.cs | 140 ++++++++++++++++++ .../perf_utils/RawMemoryFifoDictionary.cs | 59 ++++++++ src/runtime/pyobject.cs | 34 +++++ src/runtime/runtime.cs | 26 +++- 9 files changed, 531 insertions(+), 5 deletions(-) create mode 100644 src/runtime/perf_utils/EncodedStringsFifoDictionary.cs create mode 100644 src/runtime/perf_utils/EncodingGetStringPolyfill.cs create mode 100644 src/runtime/perf_utils/FifoDictionary.cs create mode 100644 src/runtime/perf_utils/RawImmutableMemBlock.cs create mode 100644 src/runtime/perf_utils/RawMemUtils.cs create mode 100644 src/runtime/perf_utils/RawMemoryFifoDictionary.cs diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index 82825a626..d9735e3c7 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -76,6 +76,12 @@ + + + + + + Properties\SharedAssemblyInfo.cs diff --git a/src/runtime/perf_utils/EncodedStringsFifoDictionary.cs b/src/runtime/perf_utils/EncodedStringsFifoDictionary.cs new file mode 100644 index 000000000..74a88f0ec --- /dev/null +++ b/src/runtime/perf_utils/EncodedStringsFifoDictionary.cs @@ -0,0 +1,73 @@ +using System; + +namespace Python.Runtime +{ + using System.Runtime.InteropServices; + + public class EncodedStringsFifoDictionary: IDisposable + { + private readonly FifoDictionary _innerDictionary; + + private readonly IntPtr _rawMemory; + + private readonly int _allocatedSize; + + public EncodedStringsFifoDictionary(int capacity, int maxItemSize) + { + if (maxItemSize < 1) + { + throw new ArgumentOutOfRangeException( + nameof(maxItemSize), + "Maximum item size should be non-zero positive."); + } + + _innerDictionary = new FifoDictionary(capacity); + _allocatedSize = maxItemSize * capacity; + _rawMemory = Marshal.AllocHGlobal(_allocatedSize); + + MaxItemSize = maxItemSize; + } + + public int MaxItemSize { get; } + + public bool TryGetValue(string key, out IntPtr value) + { + return _innerDictionary.TryGetValue(key, out value); + } + + public IntPtr AddUnsafe(string key) + { + int nextSlot = _innerDictionary.NextSlotToAdd; + IntPtr ptr = _rawMemory + (MaxItemSize * nextSlot); + _innerDictionary.AddUnsafe(key, ptr); + return ptr; + } + + public bool IsKnownPtr(IntPtr ptr) + { + var uptr = (ulong)ptr; + var umem = (ulong)_rawMemory; + + return uptr >= umem && uptr < umem + (ulong)_allocatedSize; + } + + private void ReleaseUnmanagedResources() + { + if (_rawMemory != IntPtr.Zero) + { + Marshal.FreeHGlobal(_rawMemory); + } + } + + public void Dispose() + { + ReleaseUnmanagedResources(); + GC.SuppressFinalize(this); + } + + ~EncodedStringsFifoDictionary() + { + ReleaseUnmanagedResources(); + } + } +} diff --git a/src/runtime/perf_utils/EncodingGetStringPolyfill.cs b/src/runtime/perf_utils/EncodingGetStringPolyfill.cs new file mode 100644 index 000000000..3ddd2d9f6 --- /dev/null +++ b/src/runtime/perf_utils/EncodingGetStringPolyfill.cs @@ -0,0 +1,52 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Runtime.InteropServices; +using System.Text; + +namespace Python.Runtime +{ + /// + /// This polyfill is thread unsafe. + /// +#if !NETSTANDARD + public static class EncodingGetStringPolyfill + { + private static readonly MethodInfo PlatformGetStringMethodInfo = + typeof(Encoding).GetMethod( + "GetString", + BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, + new[] + { + typeof(byte*), typeof(int) + }, null); + + private static readonly byte[] StdDecodeBuffer = PlatformGetStringMethodInfo == null ? new byte[1024 * 1024] : null; + + private static Dictionary PlatformGetStringMethodsDelegatesCache = new Dictionary(); + + private unsafe delegate string EncodingGetStringUnsafeDelegate(byte* pstr, int size); + + public unsafe static string GetString(this Encoding encoding, byte* pstr, int size) + { + if (PlatformGetStringMethodInfo != null) + { + EncodingGetStringUnsafeDelegate getStringDelegate; + if (!PlatformGetStringMethodsDelegatesCache.TryGetValue(encoding, out getStringDelegate)) + { + getStringDelegate = + (EncodingGetStringUnsafeDelegate)PlatformGetStringMethodInfo.CreateDelegate( + typeof(EncodingGetStringUnsafeDelegate), encoding); + PlatformGetStringMethodsDelegatesCache.Add(encoding, getStringDelegate); + } + return getStringDelegate(pstr, size); + } + + byte[] buffer = size <= StdDecodeBuffer.Length ? StdDecodeBuffer : new byte[size]; + Marshal.Copy((IntPtr)pstr, buffer, 0, size); + return encoding.GetString(buffer, 0, size); + } + } +#endif + +} diff --git a/src/runtime/perf_utils/FifoDictionary.cs b/src/runtime/perf_utils/FifoDictionary.cs new file mode 100644 index 000000000..3af127de1 --- /dev/null +++ b/src/runtime/perf_utils/FifoDictionary.cs @@ -0,0 +1,62 @@ +using System; + +namespace Python.Runtime +{ + using System.Collections.Generic; + + public class FifoDictionary + { + private readonly Dictionary _innerDictionary; + + private readonly KeyValuePair[] _fifoList; + + private bool _hasEmptySlots = true; + + public FifoDictionary(int capacity) + { + if (capacity <= 0) + { + throw new ArgumentOutOfRangeException(nameof(capacity), "Capacity should be non-zero positive."); + } + + _innerDictionary = new Dictionary(capacity); + _fifoList = new KeyValuePair[capacity]; + + Capacity = capacity; + } + + public bool TryGetValue(TKey key, out TValue value) + { + int index; + if (_innerDictionary.TryGetValue(key, out index)) + { + value =_fifoList[index].Value; + return true; + } + + value = default(TValue); + return false; + } + + public void AddUnsafe(TKey key, TValue value) + { + if (!_hasEmptySlots) + { + _innerDictionary.Remove(_fifoList[NextSlotToAdd].Key); + } + + _innerDictionary.Add(key, NextSlotToAdd); + _fifoList[NextSlotToAdd] = new KeyValuePair(key, value); + + NextSlotToAdd++; + if (NextSlotToAdd >= Capacity) + { + _hasEmptySlots = false; + NextSlotToAdd = 0; + } + } + + public int NextSlotToAdd { get; private set; } + public int Capacity { get; } + } +} diff --git a/src/runtime/perf_utils/RawImmutableMemBlock.cs b/src/runtime/perf_utils/RawImmutableMemBlock.cs new file mode 100644 index 000000000..6230bf5ec --- /dev/null +++ b/src/runtime/perf_utils/RawImmutableMemBlock.cs @@ -0,0 +1,84 @@ +namespace Python.Runtime +{ + using System; + + public struct RawImmutableMemBlock: IEquatable + { + private readonly int _hash; + + public RawImmutableMemBlock(IntPtr ptr, int size) + { + if (ptr == IntPtr.Zero) + { + throw new ArgumentException("Memory pointer should not be zero", nameof(ptr)); + } + + if (size < 0) + { + throw new ArgumentOutOfRangeException(nameof(size), "Size should be zero or positive."); + } + + Ptr = ptr; + Size = size; + _hash = RawMemUtils.FastXorHash(ptr, size); + } + + public RawImmutableMemBlock(RawImmutableMemBlock memBlock, IntPtr newPtr) + { + if (memBlock.Ptr == IntPtr.Zero) + { + throw new ArgumentException("Cannot copy non initialized RawImmutableMemBlock structure.", nameof(memBlock)); + } + + if (newPtr == IntPtr.Zero) + { + throw new ArgumentException("Cannot copy to zero pointer."); + } + + RawMemUtils.CopyMemBlocks(memBlock.Ptr, newPtr, memBlock.Size); + Ptr = newPtr; + Size = memBlock.Size; + _hash = memBlock._hash; + } + + public IntPtr Ptr { get; } + + public int Size { get; } + + public bool Equals(RawImmutableMemBlock other) + { + bool preEqual = _hash == other._hash && Size == other.Size; + if (!preEqual) + { + return false; + } + + return RawMemUtils.CompareMemBlocks(Ptr, other.Ptr, Size); + } + + /// + public override bool Equals(object obj) + { + return obj is RawImmutableMemBlock && Equals((RawImmutableMemBlock)obj); + } + + /// + public override int GetHashCode() + { + unchecked + { + return (_hash * 397) ^ Size; + } + } + + public static bool operator ==(RawImmutableMemBlock left, RawImmutableMemBlock right) + { + return left.Equals(right); + } + + public static bool operator !=(RawImmutableMemBlock left, RawImmutableMemBlock right) + { + return !left.Equals(right); + } + } +} diff --git a/src/runtime/perf_utils/RawMemUtils.cs b/src/runtime/perf_utils/RawMemUtils.cs new file mode 100644 index 000000000..694acd1e8 --- /dev/null +++ b/src/runtime/perf_utils/RawMemUtils.cs @@ -0,0 +1,140 @@ +namespace Python.Runtime +{ + using System; + + public static class RawMemUtils + { + public static unsafe bool CopyMemBlocks(IntPtr src, IntPtr dest, int size) + { + // XOR with 64 bit step + var p64_1 = (ulong*)src; + var p64_2 = (ulong*)dest; + int c64count = size >> 3; + + int i = 0; + while (i + /// Calculating simple 32 bit xor hash for raw memory. + /// + /// Memory pointer. + /// Size to hash. + /// 32 bit hash the in signed int format. + public static unsafe int FastXorHash(IntPtr mem, int size) + { + unchecked + { + // XOR with 64 bit step + ulong r64 = 0; + var p64 = (ulong*)mem; + var pn = (byte*)(mem + (size & ~7)); + while (p64 < pn) + { + r64 ^= *p64++; + } + + uint r32 = (uint)r64 ^ (uint)(r64 >> 32); + if ((size & 4) != 0) + { + r32 ^= *(uint*)pn; + pn += 4; + } + + if ((size & 2) != 0) + { + r32 ^= *(ushort*)pn; + pn += 2; + } + + if ((size & 1) != 0) + { + r32 ^= *pn; + } + + return (int)r32; + } + } + } +} diff --git a/src/runtime/perf_utils/RawMemoryFifoDictionary.cs b/src/runtime/perf_utils/RawMemoryFifoDictionary.cs new file mode 100644 index 000000000..d845271d5 --- /dev/null +++ b/src/runtime/perf_utils/RawMemoryFifoDictionary.cs @@ -0,0 +1,59 @@ +namespace Python.Runtime +{ + using System; + using System.Runtime.InteropServices; + + public class RawMemoryFifoDictionary : IDisposable + { + private readonly FifoDictionary _innerDictionary; + + private readonly IntPtr _rawMemory; + + public RawMemoryFifoDictionary(int capacity, int maxItemSize) + { + if (maxItemSize < 1) + { + throw new ArgumentOutOfRangeException( + nameof(maxItemSize), + "Maximum item size should be non-zero positive."); + } + + MaxItemSize = maxItemSize; + _innerDictionary = new FifoDictionary(capacity); + _rawMemory = Marshal.AllocHGlobal(maxItemSize*capacity); + } + + ~RawMemoryFifoDictionary() + { + ReleaseUnmanagedResources(); + } + + public int MaxItemSize { get; } + + public bool TryGetValue(RawImmutableMemBlock key, out TValue value) + { + return _innerDictionary.TryGetValue(key, out value); + } + + public void AddUnsafe(RawImmutableMemBlock key, TValue value) + { + int nextSlot = _innerDictionary.NextSlotToAdd; + var localKey = new RawImmutableMemBlock(key, _rawMemory + (MaxItemSize * nextSlot)); + _innerDictionary.AddUnsafe(localKey, value); + } + + public void Dispose() + { + ReleaseUnmanagedResources(); + GC.SuppressFinalize(this); + } + + private void ReleaseUnmanagedResources() + { + if (_rawMemory != IntPtr.Zero) + { + Marshal.FreeHGlobal(_rawMemory); + } + } + } +} diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 5900e80b7..0d3be7cb2 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -5,6 +5,9 @@ namespace Python.Runtime { + using System.Reflection; + using System.Threading; + /// /// Represents a generic Python object. The methods of this class are /// generally equivalent to the Python "abstract object API". See @@ -260,9 +263,34 @@ public PyObject GetAttr(PyObject name) { throw new PythonException(); } + return new PyObject(op); } + public T GetAttr(string name) + { + IntPtr op = Runtime.PyObject_GetAttrString(obj, name); + + if (op == IntPtr.Zero) + { + throw new PythonException(); + } + + try + { + object resultObj; + if (!Converter.ToManaged(op, typeof(T), out resultObj, false)) + { + throw new InvalidCastException("cannot convert object to target type"); + } + + return (T)resultObj; + } + finally + { + Runtime.XDecref(op); + } + } /// /// GetAttr Method @@ -1088,6 +1116,12 @@ public override bool TryInvoke(InvokeBinder binder, object[] args, out object re public override bool TryConvert(ConvertBinder binder, out object result) { + if (typeof(PyObject).IsAssignableFrom(binder.Type)) + { + result = this; + return true; + } + return Converter.ToManaged(this.obj, binder.Type, out result, false); } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 164b1bfaa..9f89fba36 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -206,6 +206,12 @@ public class Runtime /// internal static readonly Encoding PyEncoding = _UCS == 2 ? Encoding.Unicode : Encoding.UTF32; + /// + /// 4Mb of Python strings to .Net strings cache. + /// + private static readonly RawMemoryFifoDictionary UcsStringsInternDictionary = + new RawMemoryFifoDictionary(10000, 100 * _UCS); + /// /// Initialize the runtime... /// @@ -1316,7 +1322,7 @@ internal static IntPtr PyUnicode_FromString(string s) /// /// PyStringType or PyUnicodeType object to convert /// Managed String - internal static string GetManagedString(IntPtr op) + internal static unsafe string GetManagedString(IntPtr op) { IntPtr type = PyObject_TYPE(op); @@ -1331,11 +1337,21 @@ internal static string GetManagedString(IntPtr op) { IntPtr p = PyUnicode_AsUnicode(op); int length = PyUnicode_GetSize(op); - int size = length * _UCS; - var buffer = new byte[size]; - Marshal.Copy(p, buffer, 0, size); - return PyEncoding.GetString(buffer, 0, size); + if (size <= UcsStringsInternDictionary.MaxItemSize) + { + var ucsStringMemBlock = new RawImmutableMemBlock(p, size); + string str; + if (!UcsStringsInternDictionary.TryGetValue(ucsStringMemBlock, out str)) + { + str = PyEncoding.GetString((byte*)p, size); + UcsStringsInternDictionary.AddUnsafe(ucsStringMemBlock, str); + } + + return str; + } + + return PyEncoding.GetString((byte*)p, size); } return null; From fc549e3943e9d6bc7e7c8169a639dd1331616026 Mon Sep 17 00:00:00 2001 From: dse Date: Thu, 7 Sep 2017 02:03:39 +0400 Subject: [PATCH 08/10] Managed to python string marshaling cache added. --- CHANGELOG.md | 2 + src/runtime/CustomMarshaler.cs | 126 +++++++++++++++++++++++++++------ 2 files changed, 107 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54b84841e..b93feebe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ## [unreleased][] ### Added +- Improved performance. String marshaling between python and clr now cached. + Cache reduces GC pressure and saves from extensive memory copying. - Added support for embedding python into dotnet core 2.0 (NetStandard 2.0) - Added new build system (pythonnet.15.sln) based on dotnetcore-sdk/xplat(crossplatform msbuild). Currently there two side-by-side build systems that produces the same output (net40) from the same sources. diff --git a/src/runtime/CustomMarshaler.cs b/src/runtime/CustomMarshaler.cs index b51911816..507710f87 100644 --- a/src/runtime/CustomMarshaler.cs +++ b/src/runtime/CustomMarshaler.cs @@ -18,7 +18,7 @@ public object MarshalNativeToManaged(IntPtr pNativeData) public abstract IntPtr MarshalManagedToNative(object managedObj); - public void CleanUpNativeData(IntPtr pNativeData) + public virtual void CleanUpNativeData(IntPtr pNativeData) { Marshal.FreeHGlobal(pNativeData); } @@ -44,7 +44,12 @@ internal class UcsMarshaler : MarshalerBase private static readonly MarshalerBase Instance = new UcsMarshaler(); private static readonly Encoding PyEncoding = Runtime.PyEncoding; - public override IntPtr MarshalManagedToNative(object managedObj) + private const int MaxStringLength = 100; + private const int MaxItemSize = 4 * (MaxStringLength + 1); + private static readonly EncodedStringsFifoDictionary EncodedStringsDictionary = + new EncodedStringsFifoDictionary(10000, MaxItemSize); + + public override unsafe IntPtr MarshalManagedToNative(object managedObj) { var s = managedObj as string; @@ -53,16 +58,36 @@ public override IntPtr MarshalManagedToNative(object managedObj) return IntPtr.Zero; } - byte[] bStr = PyEncoding.GetBytes(s + "\0"); - IntPtr mem = Marshal.AllocHGlobal(bStr.Length); - try + IntPtr mem; + int stringBytesCount; + if (s.Length <= MaxStringLength) { - Marshal.Copy(bStr, 0, mem, bStr.Length); + if (EncodedStringsDictionary.TryGetValue(s, out mem)) + { + return mem; + } + + stringBytesCount = PyEncoding.GetByteCount(s); + mem = EncodedStringsDictionary.AddUnsafe(s); } - catch (Exception) + else { - Marshal.FreeHGlobal(mem); - throw; + stringBytesCount = PyEncoding.GetByteCount(s); + mem = Marshal.AllocHGlobal(stringBytesCount + 4); + } + + fixed (char* str = s) + { + try + { + PyEncoding.GetBytes(str, s.Length, (byte*)mem, stringBytesCount); + } + catch + { + // Do nothing with this. Very strange problem. + } + + *(int*)(mem + stringBytesCount) = 0; } return mem; @@ -106,6 +131,14 @@ public static int GetUnicodeByteLength(IntPtr p) } } + public override void CleanUpNativeData(IntPtr pNativeData) + { + if (!EncodedStringsDictionary.IsKnownPtr(pNativeData)) + { + base.CleanUpNativeData(pNativeData); + } + } + /// /// Utility function for Marshaling Unicode on PY3 and AnsiStr on PY2. /// Use on functions whose Input signatures changed between PY2/PY3. @@ -118,11 +151,29 @@ public static int GetUnicodeByteLength(IntPtr p) /// /// You MUST deallocate the IntPtr of the Return when done with it. /// - public static IntPtr Py3UnicodePy2StringtoPtr(string s) + public unsafe static IntPtr Py3UnicodePy2StringtoPtr(string s) { - return Runtime.IsPython3 - ? Instance.MarshalManagedToNative(s) - : Marshal.StringToHGlobalAnsi(s); + if (Runtime.IsPython3) + { + int stringBytesCount = PyEncoding.GetByteCount(s); + IntPtr mem = Marshal.AllocHGlobal(stringBytesCount + 4); + fixed (char* str = s) + { + try + { + PyEncoding.GetBytes(str, s.Length, (byte*)mem, stringBytesCount); + } + catch + { + // Do nothing with this. Very strange problem. + } + + *(int*)(mem + stringBytesCount) = 0; + } + return mem; + } + + return Marshal.StringToHGlobalAnsi(s); } /// @@ -208,7 +259,12 @@ internal class Utf8Marshaler : MarshalerBase private static readonly MarshalerBase Instance = new Utf8Marshaler(); private static readonly Encoding PyEncoding = Encoding.UTF8; - public override IntPtr MarshalManagedToNative(object managedObj) + private const int MaxStringLength = 100; + + private static readonly EncodedStringsFifoDictionary EncodedStringsDictionary = + new EncodedStringsFifoDictionary(10000, 4 * (MaxStringLength + 1)); + + public override unsafe IntPtr MarshalManagedToNative(object managedObj) { var s = managedObj as string; @@ -217,21 +273,49 @@ public override IntPtr MarshalManagedToNative(object managedObj) return IntPtr.Zero; } - byte[] bStr = PyEncoding.GetBytes(s + "\0"); - IntPtr mem = Marshal.AllocHGlobal(bStr.Length); - try + IntPtr mem; + int stringBytesCount; + if (s.Length <= MaxStringLength) { - Marshal.Copy(bStr, 0, mem, bStr.Length); + if (EncodedStringsDictionary.TryGetValue(s, out mem)) + { + return mem; + } + + stringBytesCount = PyEncoding.GetByteCount(s); + mem = EncodedStringsDictionary.AddUnsafe(s); } - catch (Exception) + else { - Marshal.FreeHGlobal(mem); - throw; + stringBytesCount = PyEncoding.GetByteCount(s); + mem = Marshal.AllocHGlobal(stringBytesCount + 1); + } + + fixed (char* str = s) + { + try + { + PyEncoding.GetBytes(str, s.Length, (byte*)mem, stringBytesCount); + } + catch + { + // Do nothing with this. Very strange problem. + } + + ((byte*)mem)[stringBytesCount] = 0; } return mem; } + public override void CleanUpNativeData(IntPtr pNativeData) + { + if (!EncodedStringsDictionary.IsKnownPtr(pNativeData)) + { + base.CleanUpNativeData(pNativeData); + } + } + public static ICustomMarshaler GetInstance(string cookie) { return Instance; From 06f4faac1bbc71736bb94e151878b5ddbef287e4 Mon Sep 17 00:00:00 2001 From: dse Date: Thu, 7 Sep 2017 02:52:11 +0400 Subject: [PATCH 09/10] Net40 compatibility fix. --- src/runtime/perf_utils/EncodingGetStringPolyfill.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/perf_utils/EncodingGetStringPolyfill.cs b/src/runtime/perf_utils/EncodingGetStringPolyfill.cs index 3ddd2d9f6..164234016 100644 --- a/src/runtime/perf_utils/EncodingGetStringPolyfill.cs +++ b/src/runtime/perf_utils/EncodingGetStringPolyfill.cs @@ -35,8 +35,8 @@ public unsafe static string GetString(this Encoding encoding, byte* pstr, int si if (!PlatformGetStringMethodsDelegatesCache.TryGetValue(encoding, out getStringDelegate)) { getStringDelegate = - (EncodingGetStringUnsafeDelegate)PlatformGetStringMethodInfo.CreateDelegate( - typeof(EncodingGetStringUnsafeDelegate), encoding); + (EncodingGetStringUnsafeDelegate)Delegate.CreateDelegate( + typeof(EncodingGetStringUnsafeDelegate), encoding, PlatformGetStringMethodInfo); PlatformGetStringMethodsDelegatesCache.Add(encoding, getStringDelegate); } return getStringDelegate(pstr, size); From 5136c856842a8bd0f3319e2a85168b45dfed8a4a Mon Sep 17 00:00:00 2001 From: dse Date: Thu, 7 Sep 2017 14:15:34 +0400 Subject: [PATCH 10/10] Build warnings fix. --- src/runtime/perf_utils/EncodingGetStringPolyfill.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/runtime/perf_utils/EncodingGetStringPolyfill.cs b/src/runtime/perf_utils/EncodingGetStringPolyfill.cs index 164234016..ac1d0ddcf 100644 --- a/src/runtime/perf_utils/EncodingGetStringPolyfill.cs +++ b/src/runtime/perf_utils/EncodingGetStringPolyfill.cs @@ -6,10 +6,11 @@ namespace Python.Runtime { +#if !NETSTANDARD /// /// This polyfill is thread unsafe. /// -#if !NETSTANDARD + [CLSCompliant(false)] public static class EncodingGetStringPolyfill { private static readonly MethodInfo PlatformGetStringMethodInfo =