Skip to content

Commit

Permalink
first attempt to add python interpreter lock. Not working
Browse files Browse the repository at this point in the history
  • Loading branch information
ncguilbeault committed Feb 23, 2024
1 parent db1ed53 commit 8e3f362
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/Bonsai.Scripting.Python/Bonsai.Scripting.Python.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<Description>Bonsai Scripting Library for interfacing with the Python runtime.</Description>
<PackageTags>Bonsai Rx Scripting Python Python.NET</PackageTags>
<TargetFramework>net472</TargetFramework>
<VersionPrefix>0.1.0</VersionPrefix>
<VersionPrefix>0.2.0</VersionPrefix>
<LangVersion>8.0</LangVersion>
</PropertyGroup>

Expand Down
20 changes: 20 additions & 0 deletions src/Bonsai.Scripting.Python/GilManager.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using Python.Runtime;
using System;

public sealed class GilManager
{
private static readonly Lazy<GilManager> _instance = new Lazy<GilManager>(() => new GilManager());
public static GilManager Instance => _instance.Value;

private GilManager()
{
}

public void ExecuteWithGil(Action action)
{
using (Py.GIL()) // Acquire GIL
{
action();
}
}
}
31 changes: 12 additions & 19 deletions src/Bonsai.Scripting.Python/PythonInterpreterLock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace Bonsai.Scripting.Python
public class PythonInterpreterLock : WorkflowExpressionBuilder
{
static readonly Range<int> argumentRange = Range.Create(lowerBound: 1, upperBound: 1);
private static readonly object _lock = new object();

public PythonInterpreterLock()
: this(new ExpressionBuilderGraph())
Expand All @@ -27,44 +28,36 @@ public PythonInterpreterLock(ExpressionBuilderGraph workflow)

public override Expression Build(IEnumerable<Expression> arguments)
{
// var source = arguments.Single();
// var sourceType = source.Type.GetGenericArguments()[0]; // Get TSource from IObservable<TSource>
// var factoryParameter = Expression.Parameter(typeof(IObservable<>).MakeGenericType(sourceType));
Console.WriteLine($"here1 args: {arguments}");
var source = arguments.FirstOrDefault();
if (source == null)
{
throw new InvalidOperationException("There must be at least one input.");
}
var sourceType = source.Type.GetGenericArguments()[0]; // Get TSource from IObservable<TSource>
Console.WriteLine($"here2 sourceType: {source.Type}");
// var factoryParameter = Expression.Parameter(source.Type);
var factoryParameter = Expression.Parameter(typeof(IObservable<>).MakeGenericType(sourceType), "factoryParam");
Console.WriteLine($"here3 factoryParam: {factoryParameter}");

return BuildWorkflow(arguments, factoryParameter, selectorBody =>
{
Console.WriteLine($"here4 selectorBody: {selectorBody}");
var selector = Expression.Lambda(selectorBody, factoryParameter);
Console.WriteLine($"here5 selector: {selector}");
var resultType = selectorBody.Type.GetGenericArguments()[0];
Console.WriteLine($"here6 resultType: {resultType}");
// var processMethod = typeof(PythonInterpreterLock).GetMethod(nameof(Process)).MakeGenericMethod(sourceType, resultType);
// Console.WriteLine($"here7 processMethod: {processMethod}");
// return Expression.Call(processMethod, source, selector);
return Expression.Call(GetType(), nameof(Process), new Type[] {sourceType, resultType}, source, selector);
});
}

static IObservable<TResult> Process<TSource, TResult>(IObservable<TSource> source, Func<IObservable<TSource>, IObservable<TResult>> selector)
{
Console.WriteLine("process");
return Observable.Defer(() =>
lock(_lock)
{
using (Py.GIL())
return source.SelectMany(value =>
{
return selector(source);
}
});
using (Py.GIL())
{
return selector(Observable.Return(value));
// );
}
});
}
// return result;
}
}
}
5 changes: 1 addition & 4 deletions src/Bonsai.Scripting.Python/RuntimeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ internal RuntimeManager(string pythonHome, string scriptPath, IObserver<RuntimeM
{
Initialize(pythonHome);
threadState = PythonEngine.BeginAllowThreads();
using (Py.GIL())
{
MainModule = CreateModule(scriptPath: scriptPath);
}
MainModule = CreateModule(scriptPath: scriptPath);

This comment has been minimized.

Copy link
@glopesdev

glopesdev Feb 24, 2024

Member

I think this line might be a problem. Module creation should run inside the GIL and this code is not under control from any nested operator, so I don't think we can assume that we can remove the call to Py.GIL here.

This comment has been minimized.

Copy link
@ncguilbeault

ncguilbeault Feb 24, 2024

Author Contributor

I thought that since the CreateModule function has the line "using (Py.GIL())" at the very beginning of the function (line 79), this additional call isn't necessary

observer.OnNext(this);
});
}
Expand Down
6 changes: 3 additions & 3 deletions src/Bonsai.Scripting.Python/Set.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ public override IObservable<TSource> Process<TSource>(IObservable<TSource> sourc
{
return source.Do(value =>
{
using (Py.GIL())
{
// using (Py.GIL())
// {
var module = Module ?? runtime.MainModule;
module.Set(VariableName, value);
}
// }
});
});
}
Expand Down

0 comments on commit 8e3f362

Please sign in to comment.