Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

use ALC for deps #1

Merged
merged 1 commit into from
Dec 12, 2023
Merged

use ALC for deps #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions DSIronPython/DSIronPython.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<Import Project="$(SolutionDir)Config\shared.props" />
</ImportGroup>
<PropertyGroup>
<OutputPath>$(SolutionDir)\package_output\DSIronPython\bin\</OutputPath>
<OutputPath>$(SolutionDir)\package_output\DSIronPython\extra\</OutputPath>
<ProjectGuid>{9EEF4F42-6B3B-4358-9A8A-C2701539A822}</ProjectGuid>
<OutputType>Library</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
Expand All @@ -29,7 +29,7 @@
</AssemblyAttribute>
</ItemGroup>

<Target Name="Move python libs to extra" AfterTargets="Build">
<!--<Target Name="Move python libs to extra" AfterTargets="Build">
<ItemGroup>
<MySourceFiles Include="$(OutputPath)\*lib\**;" />
</ItemGroup>
Expand All @@ -39,7 +39,7 @@

<Target Name="Remove Lib" AfterTargets="Move python libs to extra">
<RemoveDir Directories="$(OutputPath)\lib" />
</Target>
</Target>-->

<Target Name="copypkgjson" AfterTargets="Build">
<Copy SourceFiles="pkg.json" DestinationFolder="$(OutputPath)..\"/>
Expand Down
55 changes: 35 additions & 20 deletions IronPythonExtension/IronPythonExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.IO;
using System.Reflection;
using System.Runtime.Loader;
using Dynamo.Extensions;
using Dynamo.Logging;
using Dynamo.PythonServices;
Expand Down Expand Up @@ -61,26 +62,9 @@ public void Startup(StartupParams sp)
/// <param name="sp"></param>
public void Ready(ReadyParams sp)
{
// Searches for DSIronPython engine binary in same folder with extension itself
var targetDir = Path.GetDirectoryName(Assembly.GetAssembly(typeof(IronPythonExtension)).Location);
var libraryLoader = sp.StartupParams.LibraryLoader;
Assembly pythonEvaluatorLib = null;
try
{
pythonEvaluatorLib = Assembly.LoadFrom(Path.Combine(targetDir, PythonEvaluatorAssembly + ".dll"));
}
catch (Exception ex)
{
// Most likely the IronPython engine is excluded in this case
// but logging the exception message in case for diagnose
OnMessageLogged(LogMessage.Info(ex.Message));
return;
}
// Import IronPython Engine into VM, so Python node using IronPython engine could evaluate correctly
if (pythonEvaluatorLib != null)
{
libraryLoader.LoadNodeLibrary(pythonEvaluatorLib);
}
var extraPath = Path.Combine(new FileInfo(Assembly.GetAssembly(typeof(IronPythonExtension)).Location).Directory.Parent.FullName, "extra");
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
var alc = new IsolatedPythoContext(Path.Combine(extraPath,"DSIronPython.dll"));
alc.LoadFromAssemblyName(new AssemblyName("DSIronPython"));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means that only the DSIronPython.dll assembly (and its own dependencies) will be isolated
The IronPythonExtension assembly will be part of Dynamo's ALC

This sounds fine to me. I did the same thing with Dynamo4Revit and it worked ok
As long as the IronPythonExtension 's dependencies are ok to be in Dynamo's ALC

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I do not remember how the dependencies of the entrypoint (DSIronPython.dll) are resolved.
Hopefully it happens magically by using the ALC resolver and the deps.json file

I remember there were some issues with the dependencies that required special Resolver code in the Host app (that would be Dynamo in this case)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double check how the dependencies of DSIronPython.dll are resolved and in which ALC

Copy link
Member Author

@mjkkirschner mjkkirschner Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, correct IronPythonExtension basically has no dependencies except for Dynamo and at least my testing indicates that the assemblies are isolated from the default ALC (since ironPython3 (not isolated) works side by side))

I think I can double check by using The GetAssemblyLoadContext() method on each assembly, or maybe using dotnet-trace tooling see exactly when the load occurs.

Any other ideas for how to test Please double check how the dependencies of DSIronPython.dll are resolved and in which ALC besides the ideas above?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Harder to do, but you can simply put breakpoints in all the resolvers in Dynamo and in this extension in the Assembly Load(AssemblyName assemblyName) and implement https://learn.microsoft.com/en-us/dotnet/api/system.runtime.loader.assemblyloadcontext.resolving?view=net-8.0
You could implement that in the dynamo default ALC and put a breakpoint too

Copy link
Member Author

@mjkkirschner mjkkirschner Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the algorithm, we use the name variant:
https://learn.microsoft.com/en-us/dotnet/core/dependency-loading/loading-managed#algorithm

I'm pretty sure that dependencies of the assembly that are loaded using by name - are loaded into the same ALC, or at least as indicated by the algorithm, an attempt is made to resolve them before falling back to the default ALC.

Image shows the dependencies loaded side by side

Screenshot 2023-12-11 at 5 19 45 PM

image shows the assemblies loaded into the python2.xALC

Screenshot 2023-12-11 at 5 26 15 PM

I checked the above with the deps.json files in the DSIronPython folder, and without them and got the same functioning behavior of side by side iron python versions and the same binaries loaded into the ALC.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the issue I was thinking of dotnet/runtime#93780 (comment)
But this does not seem to be the case with IronPython deps. They probably are all in the resolver folder

}

/// <summary>
Expand All @@ -91,4 +75,35 @@ public void Shutdown()
// Do nothing for now
}
}
internal class IsolatedPythoContext : AssemblyLoadContext
{
private AssemblyDependencyResolver resolver;

public IsolatedPythoContext(string libPath)
{
resolver = new AssemblyDependencyResolver(libPath);
}

protected override Assembly Load(AssemblyName assemblyName)
{
string assemblyPath = resolver.ResolveAssemblyToPath(assemblyName);
if (assemblyPath != null)
{
return LoadFromAssemblyPath(assemblyPath);
}

return null;
}

protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
{
string libraryPath = resolver.ResolveUnmanagedDllToPath(unmanagedDllName);
if (libraryPath != null)
{
return LoadUnmanagedDllFromPath(libraryPath);
}

return IntPtr.Zero;
}
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
}
}
4 changes: 2 additions & 2 deletions IronPythonExtension/IronPythonExtension.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<Import Project="$(SolutionDir)Config\shared.props" />
</ImportGroup>
<PropertyGroup>
<OutputPath>$(SolutionDir)\package_output\DSIronPython\bin\</OutputPath>
<OutputPath>$(SolutionDir)\package_output\DSIronPython\extra\</OutputPath>
<ProjectGuid>{182FCA4E-B6EF-451F-9EC4-7BF2C622F4F7}</ProjectGuid>
<OutputType>Library</OutputType>
<AppDesignerFolder>Properties</AppDesignerFolder>
Expand All @@ -21,6 +21,6 @@
<ItemGroup>
<ExtensionDefinition Include="IronPythonExtension_ExtensionDefinition.xml" />
</ItemGroup>
<Copy SourceFiles="@(ExtensionDefinition)" DestinationFolder="$(OutputPath)..\extra" />
<Copy SourceFiles="@(ExtensionDefinition)" DestinationFolder="$(OutputPath)" />
</Target>
</Project>
2 changes: 1 addition & 1 deletion IronPythonExtension/IronPythonExtension_ExtensionDefinition.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<ExtensionDefinition>
<AssemblyPath>..\bin\IronPythonExtension.dll</AssemblyPath>
<AssemblyPath>IronPythonExtension.dll</AssemblyPath>
<TypeName>IronPythonExtension.IronPythonExtension</TypeName>
</ExtensionDefinition>
Loading