Attached is a unified diff of the changes (supposedly equivalent to diff -u).
Let me know if some other diff format would be better.
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Kamil Dworakowski
Sent: Monday, October 13, 2008 9:42 AM
To: Discussion of IronPython
Subject: Re: [IronPython] parallel importing
Could you provide a patch?
On Mon, Oct 13, 2008 at 5:31 PM, Dino Viehland <[EMAIL PROTECTED]> wrote:
> Making the code changes is easy - the hard part is really doing a new 1.x
> release and all of the sign off work that entails. We haven't ruled it out
> but we sure would like to avoid it if possible.
>
> -----Original Message-----
> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Kamil
> Dworakowski
> Sent: Monday, October 13, 2008 9:07 AM
> To: Discussion of IronPython
> Subject: Re: [IronPython] parallel importing
>
> Would that be easy to backport fix for #2 to 1.x branch?
>
> On Mon, Oct 13, 2008 at 5:00 PM, Dino Viehland <[EMAIL PROTECTED]> wrote:
>> This is all still on 1.x, right? It looks like #1 is fixed in 2.0 (we are
>> locking but on the wrong object in 1.x).
>>
>> #2 is still broken in 2.x though as well.
>>
>> -----Original Message-----
>> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Kamil
>> Dworakowski
>> Sent: Monday, October 13, 2008 4:45 AM
>> To: users@lists.ironpython.com
>> Subject: [IronPython] parallel importing
>>
>> Importing time is such a pain that I wanted to do it in parallel on
>> many threads. I figured that if the modules where imported in such a
>> way that:
>>
>> no two threads import the same module at the same time
>>
>> everything would be fine. To ensure that condition, it is enough to
>> build a dependency graph and import based on that.
>>
>> I did it: Resolver One start up time improved by 20% on a two-core
>> machine. But it crashes, surprisingly on single core machines it is
>> more often (6 crashes on 200 starts).
>>
>> So far I have identified two causes for crashes:
>>
>> 1. One thread imports a module with class B inside while another is
>> importing a module with class C inside. If B and C are subclasses of
>> A, it can result in IndexOutOfRangeException being raised, when, under
>> the hood, IronPython.Runtime.Types.DynamicType.AddSubclass is being
>> executed.
>>
>> 2. Attributes on .NET modules are loaded lazily, so importing
>> namespaces only is not enough. Attribute getting from reflected
>> packages is not thread safe. Looks like I would have to import every
>> class explicitly (would that be enough?).
>>
>> Second cause would be pretty easy to address, but I'm not so sure
>> about the first one. Are there any more potential points of problems?
>> I am beginning to think I was to optimistic about all of this
>> importing on multiple cores, but if these are the only ones it could
>> probably be still fixed.
>>
>> If anyone is interested the code for it is on github:
>> http://github.com/luntain/ipy-parallel-import.
>>
>> --
>> Kamil Dworakowski
>> Resolver Systems Ltd.
>> _______________________________________________
>> Users mailing list
>> Users@lists.ironpython.com
>> http://lists.ironpython.com/listinfo.cgi/users-ironpython.com
>> _______________________________________________
>> Users mailing list
>> Users@lists.ironpython.com
>> http://lists.ironpython.com/listinfo.cgi/users-ironpython.com
>>
> _______________________________________________
> Users mailing list
> Users@lists.ironpython.com
> http://lists.ironpython.com/listinfo.cgi/users-ironpython.com
> _______________________________________________
> Users mailing list
> Users@lists.ironpython.com
> http://lists.ironpython.com/listinfo.cgi/users-ironpython.com
>
_______________________________________________
Users mailing list
Users@lists.ironpython.com
http://lists.ironpython.com/listinfo.cgi/users-ironpython.com
edit:
$/Dev10/feature/vsl_dynamic/Merlin/Legacy/IronPython_Main/IronPython/Public/Src/IronPython/Runtime/ReflectedPackage.cs;C576154
File: ReflectedPackage.cs
===================================================================
---
$/Dev10/feature/vsl_dynamic/Merlin/Legacy/IronPython_Main/IronPython/Public/Src/IronPython/Runtime/ReflectedPackage.cs;C576154
(server) 10/13/2008 3:26 PM
+++ Shelved Change:
$/Dev10/feature/vsl_dynamic/Merlin/Legacy/IronPython_Main/IronPython/Public/Src/IronPython/Runtime/ReflectedPackage.cs;10ThreadSafety
@@ -68,9 +68,11 @@
public object TryGetPackageAny(SystemState state, string name) {
Initialize(state);
- object ret;
- if (__dict__.TryGetValue(SymbolTable.StringToId(name), out ret)) {
- return ret;
+ lock (__dict__) {
+ object ret;
+ if (__dict__.TryGetValue(SymbolTable.StringToId(name), out
ret)) {
+ return ret;
+ }
}
return null;
}
@@ -80,58 +82,60 @@
}
internal bool LoadAssembly(SystemState state, Assembly assem, bool
isInteropAssembly) {
- bool loaded;
- if (loadedAssemblies.TryGetValue(assem, out loaded)) {
- return false;
- }
+ lock (__dict__) {
+ bool loaded;
+ if (loadedAssemblies.TryGetValue(assem, out loaded)) {
+ return false;
+ }
- if (!loaded) {
- foreach (PythonModuleAttribute pma in
assem.GetCustomAttributes(typeof(PythonModuleAttribute), false)) {
- builtins.Add(pma.name, pma.type);
- builtinModuleNames[pma.type] = pma.name;
+ if (!loaded) {
+ foreach (PythonModuleAttribute pma in
assem.GetCustomAttributes(typeof(PythonModuleAttribute), false)) {
+ builtins.Add(pma.name, pma.type);
+ builtinModuleNames[pma.type] = pma.name;
+ }
}
- }
- // GetExportedTypes does not work on dynamic assemblies, and this
could be an Interop assembly
- // generated by Marshal.GetTypeForITypeInfo using Reflection.Emit,
-
-
- // isInteropAssembly flag can now be removed and replaced w/ a
call to LoadTypesFromAssembly.
- Type[] types = isInteropAssembly ? assem.GetTypes() :
LoadTypesFromAssembly(assem);
+ // GetExportedTypes does not work on dynamic assemblies, and
this could be an Interop assembly
+ // generated by Marshal.GetTypeForITypeInfo using
Reflection.Emit,
- foreach (Type type in types) {
- // Skip nested types. They get loaded during parent type
initalization.
- //
- if (type.IsNested || (!type.IsPublic &&
!Compiler.Options.PrivateBinding)) {
- continue;
- }
- // save all the namespaces, types will be lazily initialized
- // on demand in GetAttr
- ReflectedPackage pkg = GetOrMakeTopPackage(state, assem,
type.Namespace);
+ // isInteropAssembly flag can now be removed and replaced w/ a
call to LoadTypesFromAssembly.
+ Type[] types = isInteropAssembly ? assem.GetTypes() :
LoadTypesFromAssembly(assem);
- if (!loaded) {
- // We dont save all types since it requires us to hold on
to the Type object unnecessarily.
- // We do load all the types, so its not clear if this
optimizations is really useful.
+ foreach (Type type in types) {
+ // Skip nested types. They get loaded during parent type
initalization.
+ //
+ if (type.IsNested || (!type.IsPublic &&
!Compiler.Options.PrivateBinding)) {
+ continue;
+ }
- // Publish all COM types immediately so that ComObject can
access it when needed
- // for generic Runtime-callable-wrappers
- ComObject.AddType(type.GUID, type);
+ // save all the namespaces, types will be lazily
initialized
+ // on demand in GetAttr
+ ReflectedPackage pkg = GetOrMakeTopPackage(state, assem,
type.Namespace);
- // We need to save top-level types immediately
- if (String.IsNullOrEmpty(type.Namespace)) {
- pkg.SaveType(type);
+ if (!loaded) {
+ // We dont save all types since it requires us to hold
on to the Type object unnecessarily.
+ // We do load all the types, so its not clear if this
optimizations is really useful.
+
+ // Publish all COM types immediately so that ComObject
can access it when needed
+ // for generic Runtime-callable-wrappers
+ ComObject.AddType(type.GUID, type);
+
+ // We need to save top-level types immediately
+ if (String.IsNullOrEmpty(type.Namespace)) {
+ pkg.SaveType(type);
+ }
+ } else {
+ // doing a non-lazy reload... force all types to get
loaded
+ pkg.LoadAllTypes();
}
- } else {
- // doing a non-lazy reload... force all types to get
loaded
- pkg.LoadAllTypes();
}
- }
- // Assembly was loaded
- loadedAssemblies[assem] = true;
+ // Assembly was loaded
+ loadedAssemblies[assem] = true;
- return true;
+ return true;
+ }
}
public void Initialize(SystemState state) {
@@ -164,6 +168,7 @@
#region Private Implementation Details
private ReflectedPackage GetOrMakeTopPackage(SystemState state,
Assembly assm, string ns) {
+ // lock is held
ReflectedPackage ret = this;
if (ns != null) {
@@ -228,6 +233,7 @@
#region Internal API Surface
internal DynamicType SaveType(Type type) {
+ // lock is held
string name = GetCoreTypeName(type);
object existingType;
@@ -288,6 +294,7 @@
}
internal ReflectedPackage GetOrMakePackage(SystemState state, string
fullName, string name, bool isolated) {
+ // lock is held
object ret;
if (__dict__.TryGetValue(SymbolTable.StringToId(name), out ret)) {
// if it's not a module we'll wipe it out below, eg def
System(): pass then
@@ -306,6 +313,7 @@
}
private ReflectedPackage MakePackage(SystemState state, string
fullName, string name, bool isolated) {
+ // lock is held
ReflectedPackage rp = new ReflectedPackage();
object mod;
PythonModule pmod;
@@ -329,6 +337,8 @@
}
internal void LoadAllTypes() {
+ // the lock is held when we call here
+
// if new assemblies are loaded we need to re-load their types...
if (assemblyTypeLoadIndex == packageAssemblies.Count) return;
@@ -384,100 +394,110 @@
// If we fail to find a name in our dictionary then we will only
iterate all of the types
// if we don't have the full assembly loaded. This is controllved
via our assemblyTypeLoadIndex.
- if (assemblyTypeLoadIndex != -1 && assemblyTypeLoadIndex <
packageAssemblies.Count)
- LoadAllTypes();
+ lock (__dict__) {
+ if (assemblyTypeLoadIndex != -1 && assemblyTypeLoadIndex <
packageAssemblies.Count)
+ LoadAllTypes();
- if (__dict__.TryGetValue(name, out value)) {
- if (value == Uninitialized.instance) return false;
+ if (__dict__.TryGetValue(name, out value)) {
+ if (value == Uninitialized.instance) return false;
- int level;
- if (!loadLevels.TryGetValue(name, out level) || level >=
packageAssemblies.Count) {
- return true;
+ int level;
+ if (!loadLevels.TryGetValue(name, out level) || level >=
packageAssemblies.Count) {
+ return true;
+ }
}
- }
- if (assemblyTypeLoadIndex != packageAssemblies.Count) {
- value = null;
- string typeName = fullName + "." +
SymbolTable.IdToString(name);
+ if (assemblyTypeLoadIndex != packageAssemblies.Count) {
+ value = null;
+ string typeName = fullName + "." +
SymbolTable.IdToString(name);
- bool fRemovedOld = false;
+ bool fRemovedOld = false;
- // try and find the type name...
- for (int i = 0; i < packageAssemblies.Count; i++) {
- string arityName = typeName +
ReflectionUtil.GenericArityDelimiter;
- Type[] allTypes =
LoadTypesFromAssembly(packageAssemblies[i]);
+ // try and find the type name...
+ for (int i = 0; i < packageAssemblies.Count; i++) {
+ string arityName = typeName +
ReflectionUtil.GenericArityDelimiter;
+ Type[] allTypes =
LoadTypesFromAssembly(packageAssemblies[i]);
- for (int j = 0; j < allTypes.Length; j++) {
- Type t = allTypes[j];
- int nested = t.FullName.IndexOf('+');
- if (nested != -1) continue;
+ for (int j = 0; j < allTypes.Length; j++) {
+ Type t = allTypes[j];
+ int nested = t.FullName.IndexOf('+');
+ if (nested != -1) continue;
- object[] attrs =
t.GetCustomAttributes(typeof(PythonTypeAttribute), false);
- if ((attrs.Length > 0 &&
((PythonTypeAttribute)attrs[0]).name == typeName) ||
- t.FullName == typeName ||
- String.Compare(t.FullName, 0, arityName, 0,
arityName.Length) == 0) {
+ object[] attrs =
t.GetCustomAttributes(typeof(PythonTypeAttribute), false);
+ if ((attrs.Length > 0 &&
((PythonTypeAttribute)attrs[0]).name == typeName) ||
+ t.FullName == typeName ||
+ String.Compare(t.FullName, 0, arityName, 0,
arityName.Length) == 0) {
- if (!fRemovedOld) {
- // remove the old entry, we replace it w/ the
values
- // we get from the full iteration now.
- if (__dict__.ContainsKey(name))
__dict__.Remove(name);
+ if (!fRemovedOld) {
+ // remove the old entry, we replace it w/
the values
+ // we get from the full iteration now.
+ if (__dict__.ContainsKey(name))
__dict__.Remove(name);
- fRemovedOld = true;
- }
+ fRemovedOld = true;
+ }
- ComObject.AddType(t.GUID, t);
+ ComObject.AddType(t.GUID, t);
- value = SaveType(t);
+ value = SaveType(t);
+ }
}
+ loadLevels[name] = packageAssemblies.Count;
}
- loadLevels[name] = packageAssemblies.Count;
+
+ if (value != null) return true;
}
- if (value != null) return true;
- }
+ // could have been a namespace, try the dictionary one last
time...
+ if (__dict__.TryGetValue(name, out value)) return true;
- // could have been a namespace, try the dictionary one last time...
- if (__dict__.TryGetValue(name, out value)) return true;
-
- if (name == SymbolTable.File) {
- if (packageAssemblies.Count == 1) {
- value = packageAssemblies[0].FullName;
- } else {
- List res = new List();
- for (int i = 0; i < packageAssemblies.Count; i++) {
- res.Add(packageAssemblies[i].FullName);
+ if (name == SymbolTable.File) {
+ if (packageAssemblies.Count == 1) {
+ value = packageAssemblies[0].FullName;
+ } else {
+ List res = new List();
+ for (int i = 0; i < packageAssemblies.Count; i++) {
+ res.Add(packageAssemblies[i].FullName);
+ }
+ value = res;
}
- value = res;
+ return true;
}
- return true;
+
+ value = null;
+ return false;
}
-
- value = null;
- return false;
}
public void SetAttr(ICallerContext context, SymbolId name, object
value) {
- __dict__[name] = value;
+ lock (__dict__) {
+ __dict__[name] = value;
+ }
}
public void DeleteAttr(ICallerContext context, SymbolId name) {
- if (!__dict__.ContainsKey(name)) throw
Ops.AttributeErrorForMissingAttribute(ToString(), name);
+ lock (__dict__) {
+ if (!__dict__.ContainsKey(name)) throw
Ops.AttributeErrorForMissingAttribute(ToString(), name);
- __dict__[name] = Uninitialized.instance;
+ __dict__[name] = Uninitialized.instance;
+ }
}
public List GetAttrNames(ICallerContext context) {
- LoadAllTypes();
+ lock (__dict__) {
+ LoadAllTypes();
- List res = new List(((IDictionary<object, object>)__dict__).Keys);
- res.Sort();
- return res;
+ List res = new List(((IDictionary<object,
object>)__dict__).Keys);
+ res.Sort();
+ return res;
+ }
}
public IDictionary<object, object> GetAttrDict(ICallerContext context)
{
- LoadAllTypes();
+ lock (__dict__) {
+ LoadAllTypes();
- return (IDictionary<object, object>)__dict__;
+ return (IDictionary<object, object>)__dict__;
+ }
}
#endregion
===================================================================
edit:
$/Dev10/feature/vsl_dynamic/Merlin/Legacy/IronPython_Main/IronPython/Public/Src/IronPython/Runtime/Types/DynamicType.cs;C576154
File: DynamicType.cs
===================================================================
---
$/Dev10/feature/vsl_dynamic/Merlin/Legacy/IronPython_Main/IronPython/Public/Src/IronPython/Runtime/Types/DynamicType.cs;C576154
(server) 10/13/2008 3:25 PM
+++ Shelved Change:
$/Dev10/feature/vsl_dynamic/Merlin/Legacy/IronPython_Main/IronPython/Public/Src/IronPython/Runtime/Types/DynamicType.cs;10ThreadSafety
@@ -306,13 +306,13 @@
public void AddSubclass(DynamicType subclass) {
// stored as a weak ref so when GC collects the subtypes we can
// get rid of our reference.
- lock (subclass) {
+ lock (subclasses) {
subclasses.Add(new WeakReference(subclass, true));
}
}
public void RemoveSubclass(DynamicType subclass) {
- lock (subclass) {
+ lock (subclasses) {
foreach (WeakReference subType in subclasses) {
if (subclass == (subType.Target as DynamicType)) {
subclasses.Remove(subType);
===================================================================
_______________________________________________
Users mailing list
Users@lists.ironpython.com
http://lists.ironpython.com/listinfo.cgi/users-ironpython.com
