From 39add659b5468afaa88416c550265754718c500f Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Fri, 14 Jun 2019 13:47:28 -0700 Subject: [PATCH 1/3] Improve performance of unwrapping .NET objects passed from Python This addresses the following scenario: 1. A C# object `a` is created and filled with some data. 2. `a` is passed via Python.NET to Python. To do that Python.NET creates a wrapper object `w`, and stores reference to `a` in one of its fields. 3. Python code later passes `w` back to C#, e.g. calls `SomeCSharpMethod(w)`. 4. Python.NET has to unwrap `w`, so it reads the reference to `a` from it. Prior to this change in 4. Python.NET had to determine what kind of an object `a` is. If it is an exception, a different offset needed to be used. That check was very expensive (up to 4 calls into Python interpreter). This change replaces that check with computing offset unconditionally by subtracting a constant from the object size (which is read from the wrapper), thus avoiding calls to Python interpreter. --- CHANGELOG.md | 1 + src/runtime/classbase.cs | 2 +- src/runtime/classderived.cs | 2 +- src/runtime/clrobject.cs | 4 +- src/runtime/interop.cs | 136 +++++++++++++++++++++++++----------- src/runtime/managedtype.cs | 2 +- src/runtime/moduleobject.cs | 2 +- src/runtime/typemanager.cs | 6 +- 8 files changed, 106 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 705b33b7d..0d52c353e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. - Added argument types information to "No method matches given arguments" message - Moved wheel import in setup.py inside of a try/except to prevent pip collection failures - Removes PyLong_GetMax and PyClass_New when targetting Python3 +- Improved performance of calls from Python to C# ### Fixed diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 5846fa82a..e4719177f 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -253,7 +253,7 @@ public static IntPtr tp_str(IntPtr ob) public static void tp_dealloc(IntPtr ob) { ManagedType self = GetManagedObject(ob); - IntPtr dict = Marshal.ReadIntPtr(ob, ObjectOffset.DictOffset(ob)); + IntPtr dict = Marshal.ReadIntPtr(ob, ObjectOffset.TypeDictOffset(self.tpHandle)); if (dict != IntPtr.Zero) { Runtime.XDecref(dict); diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index ec3734ea5..af16b1359 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -877,7 +877,7 @@ public static void Finalize(IPythonDerivedType obj) // the C# object is being destroyed which must mean there are no more // references to the Python object as well so now we can dealloc the // python object. - IntPtr dict = Marshal.ReadIntPtr(self.pyHandle, ObjectOffset.DictOffset(self.pyHandle)); + IntPtr dict = Marshal.ReadIntPtr(self.pyHandle, ObjectOffset.TypeDictOffset(self.tpHandle)); if (dict != IntPtr.Zero) { Runtime.XDecref(dict); diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs index 502677655..8858b2b4f 100644 --- a/src/runtime/clrobject.cs +++ b/src/runtime/clrobject.cs @@ -14,11 +14,11 @@ internal CLRObject(object ob, IntPtr tp) long flags = Util.ReadCLong(tp, TypeOffset.tp_flags); if ((flags & TypeFlags.Subclass) != 0) { - IntPtr dict = Marshal.ReadIntPtr(py, ObjectOffset.DictOffset(tp)); + IntPtr dict = Marshal.ReadIntPtr(py, ObjectOffset.TypeDictOffset(tp)); if (dict == IntPtr.Zero) { dict = Runtime.PyDict_New(); - Marshal.WriteIntPtr(py, ObjectOffset.DictOffset(tp), dict); + Marshal.WriteIntPtr(py, ObjectOffset.TypeDictOffset(tp), dict); } } diff --git a/src/runtime/interop.cs b/src/runtime/interop.cs index 4ae4b61e0..9b4ab7f57 100644 --- a/src/runtime/interop.cs +++ b/src/runtime/interop.cs @@ -1,6 +1,7 @@ using System; using System.Collections; -using System.Collections.Specialized; +using System.Collections.Generic; +using System.Diagnostics; using System.Runtime.InteropServices; using System.Reflection; using System.Text; @@ -67,11 +68,47 @@ public ModulePropertyAttribute() } } + internal static class ManagedDataOffsets + { + static ManagedDataOffsets() + { + FieldInfo[] fi = typeof(ManagedDataOffsets).GetFields(BindingFlags.Static | BindingFlags.Public); + for (int i = 0; i < fi.Length; i++) + { + fi[i].SetValue(null, -(i * IntPtr.Size) - IntPtr.Size); + } - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] - internal class ObjectOffset + size = fi.Length * IntPtr.Size; + } + + public static readonly int ob_data; + public static readonly int ob_dict; + + private static int BaseOffset(IntPtr type) + { + Debug.Assert(type != IntPtr.Zero); + int typeSize = Marshal.ReadInt32(type, TypeOffset.tp_basicsize); + Debug.Assert(typeSize > 0 && typeSize <= ExceptionOffset.Size()); + return typeSize; + } + public static int DataOffset(IntPtr type) + { + return BaseOffset(type) + ob_data; + } + + public static int DictOffset(IntPtr type) + { + return BaseOffset(type) + ob_dict; + } + + public static int Size { get { return size; } } + + private static readonly int size; + } + + internal static class OriginalObjectOffsets { - static ObjectOffset() + static OriginalObjectOffsets() { int size = IntPtr.Size; var n = 0; // Py_TRACE_REFS add two pointers to PyObject_HEAD @@ -82,42 +119,58 @@ static ObjectOffset() #endif ob_refcnt = (n + 0) * size; ob_type = (n + 1) * size; - ob_dict = (n + 2) * size; - ob_data = (n + 3) * size; } - public static int magic(IntPtr ob) + public static int Size { get { return size; } } + + private static readonly int size = +#if PYTHON_WITH_PYDEBUG + 4 * IntPtr.Size; +#else + 2 * IntPtr.Size; +#endif + +#if PYTHON_WITH_PYDEBUG + public static int _ob_next; + public static int _ob_prev; +#endif + public static int ob_refcnt; + public static int ob_type; + } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] + internal class ObjectOffset + { + static ObjectOffset() { - if ((Runtime.PyObject_TypeCheck(ob, Exceptions.BaseException) || - (Runtime.PyType_Check(ob) && Runtime.PyType_IsSubtype(ob, Exceptions.BaseException)))) - { - return ExceptionOffset.ob_data; - } - return ob_data; +#if PYTHON_WITH_PYDEBUG + _ob_next = OriginalObjectOffsets._ob_next; + _ob_prev = OriginalObjectOffsets._ob_prev; +#endif + ob_refcnt = OriginalObjectOffsets.ob_refcnt; + ob_type = OriginalObjectOffsets.ob_type; + + size = OriginalObjectOffsets.Size + ManagedDataOffsets.Size; } - public static int DictOffset(IntPtr ob) + public static int magic(IntPtr type) { - if ((Runtime.PyObject_TypeCheck(ob, Exceptions.BaseException) || - (Runtime.PyType_Check(ob) && Runtime.PyType_IsSubtype(ob, Exceptions.BaseException)))) - { - return ExceptionOffset.ob_dict; - } - return ob_dict; + return ManagedDataOffsets.DataOffset(type); } - public static int Size(IntPtr ob) + public static int TypeDictOffset(IntPtr type) { - if ((Runtime.PyObject_TypeCheck(ob, Exceptions.BaseException) || - (Runtime.PyType_Check(ob) && Runtime.PyType_IsSubtype(ob, Exceptions.BaseException)))) + return ManagedDataOffsets.DictOffset(type); + } + + public static int Size(IntPtr pyType) + { + if (IsException(pyType)) { return ExceptionOffset.Size(); } -#if PYTHON_WITH_PYDEBUG - return 6 * IntPtr.Size; -#else - return 4 * IntPtr.Size; -#endif + + return size; } #if PYTHON_WITH_PYDEBUG @@ -126,8 +179,15 @@ public static int Size(IntPtr ob) #endif public static int ob_refcnt; public static int ob_type; - private static int ob_dict; - private static int ob_data; + private static readonly int size; + + private static bool IsException(IntPtr pyObject) + { + var type = Runtime.PyObject_TYPE(pyObject); + return Runtime.PyObjectType_TypeCheck(type, Exceptions.BaseException) + || Runtime.PyObjectType_TypeCheck(type, Runtime.PyTypeType) + && Runtime.PyType_IsSubtype(pyObject, Exceptions.BaseException); + } } [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] @@ -136,19 +196,17 @@ internal class ExceptionOffset static ExceptionOffset() { Type type = typeof(ExceptionOffset); - FieldInfo[] fi = type.GetFields(); - int size = IntPtr.Size; + FieldInfo[] fi = type.GetFields(BindingFlags.Static | BindingFlags.Public); for (int i = 0; i < fi.Length; i++) { - fi[i].SetValue(null, (i * size) + ObjectOffset.ob_type + size); + fi[i].SetValue(null, (i * IntPtr.Size) + OriginalObjectOffsets.Size); } - } - public static int Size() - { - return ob_data + IntPtr.Size; + size = fi.Length * IntPtr.Size + OriginalObjectOffsets.Size + ManagedDataOffsets.Size; } + public static int Size() { return size; } + // PyException_HEAD // (start after PyObject_HEAD) public static int dict = 0; @@ -162,9 +220,7 @@ public static int Size() public static int suppress_context = 0; #endif - // extra c# data - public static int ob_dict; - public static int ob_data; + private static readonly int size; } diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index 3191da949..23f5898d1 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -33,7 +33,7 @@ internal static ManagedType GetManagedObject(IntPtr ob) { IntPtr op = tp == ob ? Marshal.ReadIntPtr(tp, TypeOffset.magic()) - : Marshal.ReadIntPtr(ob, ObjectOffset.magic(ob)); + : Marshal.ReadIntPtr(ob, ObjectOffset.magic(tp)); if (op == IntPtr.Zero) { return null; diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 7a45c6c81..c9e96ed91 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -53,7 +53,7 @@ public ModuleObject(string name) Runtime.XDecref(pyfilename); Runtime.XDecref(pydocstring); - Marshal.WriteIntPtr(pyHandle, ObjectOffset.DictOffset(pyHandle), dict); + Marshal.WriteIntPtr(pyHandle, ObjectOffset.TypeDictOffset(tpHandle), dict); InitializeModuleMembers(); } diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 127e82eaa..4c7f652aa 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -85,7 +85,7 @@ internal static IntPtr CreateType(Type impl) // Set tp_basicsize to the size of our managed instance objects. Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)ob_size); - var offset = (IntPtr)ObjectOffset.DictOffset(type); + var offset = (IntPtr)ObjectOffset.TypeDictOffset(type); Marshal.WriteIntPtr(type, TypeOffset.tp_dictoffset, offset); InitializeSlots(type, impl); @@ -123,7 +123,6 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) IntPtr base_ = IntPtr.Zero; int ob_size = ObjectOffset.Size(Runtime.PyTypeType); - int tp_dictoffset = ObjectOffset.DictOffset(Runtime.PyTypeType); // XXX Hack, use a different base class for System.Exception // Python 2.5+ allows new style class exceptions but they *must* @@ -131,9 +130,10 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) if (typeof(Exception).IsAssignableFrom(clrType)) { ob_size = ObjectOffset.Size(Exceptions.Exception); - tp_dictoffset = ObjectOffset.DictOffset(Exceptions.Exception); } + int tp_dictoffset = ob_size + ManagedDataOffsets.ob_dict; + if (clrType == typeof(Exception)) { base_ = Exceptions.Exception; From b480a0cec4e01238e9f01c134716077db024cbf9 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Wed, 20 Nov 2019 18:45:42 -0800 Subject: [PATCH 2/3] added Runtime.PyType_IsSameAsOrSubtype --- src/runtime/interop.cs | 4 ++-- src/runtime/runtime.cs | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/runtime/interop.cs b/src/runtime/interop.cs index 9b4ab7f57..858c86207 100644 --- a/src/runtime/interop.cs +++ b/src/runtime/interop.cs @@ -184,8 +184,8 @@ public static int Size(IntPtr pyType) private static bool IsException(IntPtr pyObject) { var type = Runtime.PyObject_TYPE(pyObject); - return Runtime.PyObjectType_TypeCheck(type, Exceptions.BaseException) - || Runtime.PyObjectType_TypeCheck(type, Runtime.PyTypeType) + return Runtime.PyType_IsSameAsOrSubtype(type, ofType: Exceptions.BaseException) + || Runtime.PyType_IsSameAsOrSubtype(type, ofType: Runtime.PyTypeType) && Runtime.PyType_IsSubtype(pyObject, Exceptions.BaseException); } } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 7a78cd6e1..181710ec7 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1771,6 +1771,11 @@ internal static bool PyObject_TypeCheck(IntPtr ob, IntPtr tp) return (t == tp) || PyType_IsSubtype(t, tp); } + internal static bool PyType_IsSameAsOrSubtype(IntPtr type, IntPtr ofType) + { + return (type == ofType) || PyType_IsSubtype(type, ofType); + } + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern IntPtr PyType_GenericNew(IntPtr type, IntPtr args, IntPtr kw); From 0564832867c028694e65b4eee6aae5fac19a3cd6 Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 7 Mar 2020 11:50:35 +0100 Subject: [PATCH 3/3] Update performance goals according to test results --- src/perf_tests/BenchmarkTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/perf_tests/BenchmarkTests.cs b/src/perf_tests/BenchmarkTests.cs index baa825ca8..6e0afca69 100644 --- a/src/perf_tests/BenchmarkTests.cs +++ b/src/perf_tests/BenchmarkTests.cs @@ -30,14 +30,14 @@ public void SetUp() public void ReadInt64Property() { double optimisticPerfRatio = GetOptimisticPerfRatio(this.summary.Reports); - AssertPerformanceIsBetterOrSame(optimisticPerfRatio, target: 0.66); + AssertPerformanceIsBetterOrSame(optimisticPerfRatio, target: 0.57); } [Test] public void WriteInt64Property() { double optimisticPerfRatio = GetOptimisticPerfRatio(this.summary.Reports); - AssertPerformanceIsBetterOrSame(optimisticPerfRatio, target: 0.64); + AssertPerformanceIsBetterOrSame(optimisticPerfRatio, target: 0.57); } static double GetOptimisticPerfRatio(