From 0268925799418cda5f4601760a7bd40634cc504b Mon Sep 17 00:00:00 2001 From: Martin Molinero Date: Mon, 11 Nov 2019 16:25:28 -0300 Subject: [PATCH 1/2] Improve assembly manager initialization - Perform assembly scan in separate worker tasks for performance and to avoid initialization hang --- src/runtime/assemblymanager.cs | 165 ++++++++++++++++++--------------- src/runtime/classmanager.cs | 5 +- src/runtime/genericutil.cs | 141 ++++++++++++++-------------- src/runtime/moduleobject.cs | 24 +---- 4 files changed, 168 insertions(+), 167 deletions(-) diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 4f0f1e417..1beaf4789 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -1,11 +1,11 @@ using System; -using System.Collections; using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Reflection; using System.Threading; +using System.Threading.Tasks; namespace Python.Runtime { @@ -17,17 +17,15 @@ 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 Dictionary> generics; - private static AssemblyLoadEventHandler lhandler; - private static ResolveEventHandler rhandler; + private static ConcurrentDictionary> namespaces; + private static ConcurrentDictionary assembliesNamesCache; + private static ConcurrentDictionary lookupTypeCache; + private static ConcurrentQueue assemblies; + private static int pendingAssemblies; // updated only under GIL? private static Dictionary probed; - - // modified from event handlers below, potentially triggered from different .NET threads - private static ConcurrentQueue assemblies; - internal static List pypath; + private static List pypath; private AssemblyManager() { @@ -40,33 +38,41 @@ private AssemblyManager() /// internal static void Initialize() { - namespaces = new ConcurrentDictionary>(); + namespaces = new ConcurrentDictionary>(); + assembliesNamesCache = new ConcurrentDictionary(); + lookupTypeCache = new ConcurrentDictionary(); probed = new Dictionary(32); - //generics = new Dictionary>(); assemblies = new ConcurrentQueue(); pypath = new List(16); AppDomain domain = AppDomain.CurrentDomain; - lhandler = new AssemblyLoadEventHandler(AssemblyLoadHandler); - domain.AssemblyLoad += lhandler; - - rhandler = new ResolveEventHandler(ResolveHandler); - domain.AssemblyResolve += rhandler; + domain.AssemblyLoad += AssemblyLoadHandler; + domain.AssemblyResolve += ResolveHandler; - Assembly[] items = domain.GetAssemblies(); - foreach (Assembly a in items) + foreach (var assembly in domain.GetAssemblies()) { try { - ScanAssembly(a); - assemblies.Enqueue(a); + LaunchAssemblyLoader(assembly); } catch (Exception ex) { - Debug.WriteLine("Error scanning assembly {0}. {1}", a, ex); + Debug.WriteLine("Error scanning assembly {0}. {1}", assembly, ex); } } + + var safeCount = 0; + // lets wait until all assemblies are loaded + do + { + if (safeCount++ > 200) + { + throw new TimeoutException("Timeout while waiting for assemblies to load"); + } + + Thread.Sleep(50); + } while (pendingAssemblies > 0); } @@ -76,8 +82,8 @@ internal static void Initialize() internal static void Shutdown() { AppDomain domain = AppDomain.CurrentDomain; - domain.AssemblyLoad -= lhandler; - domain.AssemblyResolve -= rhandler; + domain.AssemblyLoad -= AssemblyLoadHandler; + domain.AssemblyResolve -= ResolveHandler; } @@ -88,13 +94,41 @@ internal static void Shutdown() /// so that we can know about assemblies that get loaded after the /// Python runtime is initialized. /// + /// Scanning assemblies here caused internal hangs when calling + /// private static void AssemblyLoadHandler(object ob, AssemblyLoadEventArgs args) { Assembly assembly = args.LoadedAssembly; - assemblies.Enqueue(assembly); - ScanAssembly(assembly); + LaunchAssemblyLoader(assembly); } + /// + /// Launches a new task that will load the provided assembly + /// + private static void LaunchAssemblyLoader(Assembly assembly) + { + if (assembly != null) + { + Interlocked.Increment(ref pendingAssemblies); + Task.Factory.StartNew(() => + { + try + { + if (assembliesNamesCache.TryAdd(assembly.GetName().Name, assembly)) + { + assemblies.Enqueue(assembly); + ScanAssembly(assembly); + } + } + catch + { + // pass + } + + Interlocked.Decrement(ref pendingAssemblies); + }); + } + } /// /// Event handler for assembly resolve events. This is needed because @@ -106,12 +140,12 @@ private static void AssemblyLoadHandler(object ob, AssemblyLoadEventArgs args) private static Assembly ResolveHandler(object ob, ResolveEventArgs args) { string name = args.Name.ToLower(); - foreach (Assembly a in assemblies) + foreach (var assembly in assemblies) { - string full = a.FullName.ToLower(); + var full = assembly.FullName.ToLower(); if (full.StartsWith(name)) { - return a; + return assembly; } } return LoadAssemblyPath(args.Name); @@ -133,23 +167,26 @@ internal static void UpdatePath() { IntPtr list = Runtime.PySys_GetObject("path"); int count = Runtime.PyList_Size(list); + var sep = Path.DirectorySeparatorChar; + if (count != pypath.Count) { pypath.Clear(); probed.Clear(); + // add first the current path + pypath.Add(""); for (var i = 0; i < count; i++) { IntPtr item = Runtime.PyList_GetItem(list, i); string path = Runtime.GetManagedString(item); if (path != null) { - pypath.Add(path); + pypath.Add(path == string.Empty ? path : path + sep); } } } } - /// /// Given an assembly name, try to find this assembly file using the /// PYTHONPATH. If not found, return null to indicate implicit load @@ -157,30 +194,17 @@ internal static void UpdatePath() /// public static string FindAssembly(string name) { - char sep = Path.DirectorySeparatorChar; - - foreach (string head in pypath) + foreach (var head in pypath) { - string path; - if (head == null || head.Length == 0) - { - path = name; - } - else + var dll = $"{head}{name}.dll"; + if (File.Exists(dll)) { - path = head + sep + name; + return dll; } - - string temp = path + ".dll"; - if (File.Exists(temp)) - { - return temp; - } - - temp = path + ".exe"; - if (File.Exists(temp)) + var executable = $"{head}{name}.exe"; + if (File.Exists(executable)) { - return temp; + return executable; } } return null; @@ -200,10 +224,7 @@ public static Assembly LoadAssembly(string name) } catch (Exception) { - //if (!(e is System.IO.FileNotFoundException)) - //{ - // throw; - //} + // ignored } return assembly; } @@ -214,7 +235,7 @@ public static Assembly LoadAssembly(string name) /// public static Assembly LoadAssemblyPath(string name) { - string path = FindAssembly(name); + var path = FindAssembly(name); Assembly assembly = null; if (path != null) { @@ -224,6 +245,7 @@ public static Assembly LoadAssemblyPath(string name) } catch (Exception) { + // ignored } } return assembly; @@ -251,6 +273,7 @@ public static Assembly LoadAssemblyFullPath(string name) } catch (Exception) { + // ignored } } } @@ -262,14 +285,8 @@ public static Assembly LoadAssemblyFullPath(string name) /// public static Assembly FindLoadedAssembly(string name) { - foreach (Assembly a in assemblies) - { - if (a.GetName().Name == name) - { - return a; - } - } - return null; + Assembly result; + return assembliesNamesCache.TryGetValue(name, out result) ? result : null; } /// @@ -306,10 +323,6 @@ public static bool LoadImplicit(string name, bool warn = true) { a = LoadAssemblyPath(s); } - if (a == null) - { - a = LoadAssembly(s); - } if (a != null && !assembliesSet.Contains(a)) { loaded = true; @@ -344,12 +357,6 @@ internal static void ScanAssembly(Assembly assembly) // gather a list of all of the namespaces contributed to by // the assembly. - // skip this assembly, it causes 'GetTypes' call to hang - if (assembly.FullName.StartsWith("System.Windows.Forms")) - { - return; - } - Type[] types = assembly.GetTypes(); foreach (Type t in types) { @@ -361,13 +368,13 @@ internal static void ScanAssembly(Assembly assembly) for (var n = 0; n < names.Length; n++) { s = n == 0 ? names[0] : s + "." + names[n]; - namespaces.TryAdd(s, new ConcurrentDictionary()); + namespaces.TryAdd(s, new ConcurrentDictionary()); } } if (ns != null) { - namespaces[ns].TryAdd(assembly, string.Empty); + namespaces[ns].TryAdd(assembly, 1); } if (ns != null && t.IsGenericTypeDefinition) @@ -457,11 +464,17 @@ public static List GetNames(string nsname) /// public static Type LookupType(string qname) { + Type type; + if (lookupTypeCache.TryGetValue(qname, out type)) + { + return type; + } foreach (Assembly assembly in assemblies) { - Type type = assembly.GetType(qname); + type = assembly.GetType(qname); if (type != null) { + lookupTypeCache[qname] = type; return type; } } diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 6a9d40ebd..83495cec4 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -40,9 +40,8 @@ static ClassManager() /// internal static ClassBase GetClass(Type type) { - ClassBase cb = null; - cache.TryGetValue(type, out cb); - if (cb != null) + ClassBase cb; + if (cache.TryGetValue(type, out cb)) { return cb; } diff --git a/src/runtime/genericutil.cs b/src/runtime/genericutil.cs index 9772d082f..507a901c8 100644 --- a/src/runtime/genericutil.cs +++ b/src/runtime/genericutil.cs @@ -25,32 +25,33 @@ static GenericUtil() /// internal static void Register(Type t) { - if (null == t.Namespace || null == t.Name) + lock (mapping) { - return; - } + if (null == t.Namespace || null == t.Name) + { + return; + } - Dictionary> nsmap = null; - mapping.TryGetValue(t.Namespace, out nsmap); - if (nsmap == null) - { - nsmap = new Dictionary>(); - mapping[t.Namespace] = nsmap; - } - string basename = t.Name; - int tick = basename.IndexOf("`"); - if (tick > -1) - { - basename = basename.Substring(0, tick); - } - List gnames = null; - nsmap.TryGetValue(basename, out gnames); - if (gnames == null) - { - gnames = new List(); - nsmap[basename] = gnames; + Dictionary> nsmap; + if (!mapping.TryGetValue(t.Namespace, out nsmap)) + { + nsmap = new Dictionary>(); + mapping[t.Namespace] = nsmap; + } + string basename = t.Name; + int tick = basename.IndexOf("`"); + if (tick > -1) + { + basename = basename.Substring(0, tick); + } + List gnames; + if (!nsmap.TryGetValue(basename, out gnames)) + { + gnames = new List(); + nsmap[basename] = gnames; + } + gnames.Add(t.Name); } - gnames.Add(t.Name); } /// @@ -58,18 +59,20 @@ internal static void Register(Type t) /// public static List GetGenericBaseNames(string ns) { - Dictionary> nsmap = null; - mapping.TryGetValue(ns, out nsmap); - if (nsmap == null) - { - return null; - } - var names = new List(); - foreach (string key in nsmap.Keys) + lock (mapping) { - names.Add(key); + Dictionary> nsmap; + if (!mapping.TryGetValue(ns, out nsmap)) + { + return null; + } + var names = new List(); + foreach (string key in nsmap.Keys) + { + names.Add(key); + } + return names; } - return names; } /// @@ -99,38 +102,38 @@ public static List GenericsForType(Type t) public static List GenericsByName(string ns, string basename) { - Dictionary> nsmap = null; - mapping.TryGetValue(ns, out nsmap); - if (nsmap == null) + lock (mapping) { - return null; - } + Dictionary> nsmap; + if (!mapping.TryGetValue(ns, out nsmap)) + { + return null; + } - int tick = basename.IndexOf("`"); - if (tick > -1) - { - basename = basename.Substring(0, tick); - } + int tick = basename.IndexOf("`"); + if (tick > -1) + { + basename = basename.Substring(0, tick); + } - List names = null; - nsmap.TryGetValue(basename, out names); - if (names == null) - { - return null; - } + List names; + if (!nsmap.TryGetValue(basename, out names)) + { + return null; + } - var result = new List(); - foreach (string name in names) - { - string qname = ns + "." + name; - Type o = AssemblyManager.LookupType(qname); - if (o != null) + var result = new List(); + foreach (string name in names) { - result.Add(o); + string qname = ns + "." + name; + Type o = AssemblyManager.LookupType(qname); + if (o != null) + { + result.Add(o); + } } + return result; } - - return result; } /// @@ -138,17 +141,19 @@ public static List GenericsByName(string ns, string basename) /// public static string GenericNameForBaseName(string ns, string name) { - Dictionary> nsmap = null; - mapping.TryGetValue(ns, out nsmap); - if (nsmap == null) - { - return null; - } - List gnames = null; - nsmap.TryGetValue(name, out gnames); - if (gnames?.Count > 0) + lock (mapping) { - return gnames[0]; + Dictionary> nsmap; + if (!mapping.TryGetValue(ns, out nsmap)) + { + return null; + } + List gnames = null; + nsmap.TryGetValue(name, out gnames); + if (gnames?.Count > 0) + { + return gnames[0]; + } } return null; } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index c06eaf4f0..4522fb9fa 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -67,9 +67,8 @@ public ModuleObject(string name) /// public ManagedType GetAttribute(string name, bool guess) { - ManagedType cached = null; - cache.TryGetValue(name, out cached); - if (cached != null) + ManagedType cached; + if (cache.TryGetValue(name, out cached)) { return cached; } @@ -78,16 +77,6 @@ public ManagedType GetAttribute(string name, bool guess) ClassBase c; Type type; - //if (AssemblyManager.IsValidNamespace(name)) - //{ - // IntPtr py_mod_name = Runtime.PyString_FromString(name); - // IntPtr modules = Runtime.PyImport_GetModuleDict(); - // IntPtr module = Runtime.PyDict_GetItem(modules, py_mod_name); - // if (module != IntPtr.Zero) - // return (ManagedType)this; - // return null; - //} - string qname = _namespace == string.Empty ? name : _namespace + "." + name; @@ -186,14 +175,9 @@ private void StoreAttribute(string name, ManagedType ob) /// public void LoadNames() { - ManagedType m = null; - foreach (string name in AssemblyManager.GetNames(_namespace)) + foreach (var name in AssemblyManager.GetNames(_namespace)) { - cache.TryGetValue(name, out m); - if (m == null) - { - ManagedType attr = GetAttribute(name, true); - } + var attr = GetAttribute(name, true); } } From 3fef6242ed2858303eccca8967ca6a9dde09126f Mon Sep 17 00:00:00 2001 From: Martin Molinero Date: Mon, 11 Nov 2019 16:54:45 -0300 Subject: [PATCH 2/2] Version bump 1.0.5.28 --- .bumpversion.cfg | 2 +- conda.recipe/meta.yaml | 2 +- setup.py | 2 +- src/SharedAssemblyInfo.cs | 2 +- src/clrmodule/ClrModule.cs | 2 +- src/runtime/resources/clr.py | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 66c2a831e..c3f226ae6 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.0.5.26 +current_version = 1.0.5.28 parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\.(?P[a-z]+)(?P\d+))? serialize = {major}.{minor}.{patch}.{release}{dev} diff --git a/conda.recipe/meta.yaml b/conda.recipe/meta.yaml index b3433a871..c322f4a39 100644 --- a/conda.recipe/meta.yaml +++ b/conda.recipe/meta.yaml @@ -1,6 +1,6 @@ package: name: pythonnet - version: "1.0.5.26" + version: "1.0.5.28" build: skip: True # [not win] diff --git a/setup.py b/setup.py index 27fc0edf3..75bfb5c77 100644 --- a/setup.py +++ b/setup.py @@ -485,7 +485,7 @@ def run(self): setup( name="pythonnet", - version="1.0.5.26", + version="1.0.5.28", description=".Net and Mono integration for Python", url='https://pythonnet.github.io/', license='MIT', diff --git a/src/SharedAssemblyInfo.cs b/src/SharedAssemblyInfo.cs index 96376e07a..e1b69d556 100644 --- a/src/SharedAssemblyInfo.cs +++ b/src/SharedAssemblyInfo.cs @@ -25,4 +25,4 @@ // Version Information. Keeping it simple. May need to revisit for Nuget // See: https://codingforsmarties.wordpress.com/2016/01/21/how-to-version-assemblies-destined-for-nuget/ // AssemblyVersion can only be numeric -[assembly: AssemblyVersion("1.0.5.26")] +[assembly: AssemblyVersion("1.0.5.28")] diff --git a/src/clrmodule/ClrModule.cs b/src/clrmodule/ClrModule.cs index d5112da67..89bbcf1aa 100644 --- a/src/clrmodule/ClrModule.cs +++ b/src/clrmodule/ClrModule.cs @@ -53,7 +53,7 @@ public static void initclr() { #if USE_PYTHON_RUNTIME_VERSION // Has no effect until SNK works. Keep updated anyways. - Version = new Version("1.0.5.26"), + Version = new Version("1.0.5.28"), #endif CultureInfo = CultureInfo.InvariantCulture }; diff --git a/src/runtime/resources/clr.py b/src/runtime/resources/clr.py index 23aad8d32..61b127d94 100644 --- a/src/runtime/resources/clr.py +++ b/src/runtime/resources/clr.py @@ -2,7 +2,7 @@ Code in this module gets loaded into the main clr module. """ -__version__ = "1.0.5.26" +__version__ = "1.0.5.28" class clrproperty(object):