-
Notifications
You must be signed in to change notification settings - Fork 163
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.
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
WIP: Design proposal for dynamic component loading. #47
Conversation
Marked as WIP as parts of this proposal are still being worked on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks a good approach. I think we should start prototyping.
* The new load context is initialized by specifying the full path to the main assembly of the component to load. | ||
* It will look for the `.deps.json` next to that assembly to determine its dependencies. Lack of `.deps.json` will be treated in the same way it is today for executable apps - that is all the assemblies in the same folder will be used as dependencies. | ||
* Parsing and understanding of the `.deps.json` will be performed by the same host components which do this for executable apps (so same behavior/quirks/bugs, very little code duplication). Specifically `hostpolicy.dll` is the component which parses `.deps.json` during app startup. See [host-components](https://github.com/dotnet/core-setup/blob/master/Documentation/design-docs/host-components.md) for more details. If this functionality is required when running with a custom host, the said host would need to provide this functionality to the runtime. | ||
* If the component has `.runtimeconfig.json` and/or `.runtimeconfig.dev.json` it will only be used to verify runtime version and provide probing paths. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given @natemcmaster 's post here
https://natemcmaster.com/blog/2017/12/21/netcore-primitives/ and the related cited documentation in CLI...
It seems the .runtimeconfig.json
is intended for the human edited configuration. While this is in its infancy, I wonder if it is the only config file we should support. Once we work through exactly what we want we can adjust the tooling to write the .deps.json
file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry - probably missing the point of your question.
.runtimeconfig.json
serves different purpose than .deps.json
.
The former is used to locate the runtime and the runtime options to start the runtime with. It also stores framework dependencies - this is somewhat weird since the "dependencies" part of that should be in .deps.json
logically. On the other hand, the way to find the runtime is to locate the lowest-level framework, so that part needs to be in .runtimeconfig.json
.
In a way the runtime-config part is meant to be human editted. The framework dependencies are generally machine generated as those fall out of the compilation process.
The latter is used to locate specific assembly dependencies. This almost entirely a result of the compilation process and thus machine generated.
Please note that vast majority of dynamically loaded components will not have .runtimeconfig.json
. The proposed new functionality is about finding assembly dependencies and as such it's not really tied to the .runtimeconfig.json
(other than the probing paths), on the other hand it is very much tied to .deps.json
* Auto-upgrade - components will auto-upgrade to the version of a dependency they share with the app (downgrade will not occur, in that case the dependency would be loaded in isolation). This can lead to incompatibilities. | ||
|
||
Open questions: | ||
* Which is the default behavior? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My initial preference was for the Full Isolation
case, but the more I think about it the more I prefer the Prefer default
implementation.
I would rename it Prefer parent
and use the hierarchy. This would help allow for plugin families. It could easily be mimic with natural hierarchical directory structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like class loaders in Java? Perhaps than assemblies loading can have an API as well so it may decide whether to give a chance to parent to load assembly first. The same way as class loaders works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyKanzhelev I have only very limited knowledge of Java in general. So far what I've read about class loaders they are very similar to .NET Core's AssemblyLoadContext
. This class already provides ways for a custom implementation to defer to the parent first if the custom code wants to do that.
This proposal heavily relies on AssemblyLoadContext
as the way to achieve isolation. I guess one way of thinking about this proposal is that we're trying to add an easier-to-use class loader to dynamically load components with their dependencies. But again, I know very little about the Java world, so maybe I'm completely wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to read about AssemblyLoadContext
more.
Yes, I was mostly referring to the fact that class loaders can be custom coded to decide on loading ordering themselves. And also there are few well-known class loaders which user is aware of and can decide to load some assemblies there so they will share statics.
If simplified version if enough for most scenarios - it should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a description of AssemblyLoadContext
. It's not entirely up-to-date, but the basics are there. Also added this link to the doc. Let me know if there are unclear parts, I'll go fix the other doc as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know my comment won't be of any help in terms of how you guys would proceed but I just wanted to say, with 20 or so many years of software development background, the AssemblyLoadContext
is very hard to understand. Either due to the API design or due to the lack of documentation (samples rather than API description), I don't know. Just wanted to share how I feel about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CoskunSunali Thanks. We are hearing the same thing from lots of customers.
* Which is the default behavior? | ||
* Does the framework implement more than one behavior and lets users choose? | ||
* In the "prefer default" behavior - what does it mean for the parent context to "satisfy" the dependency? Does it mean exact version match, or does it allow auto-upgrade (patch, minor or even major)? Do we even allow downgrades? | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the default should be auto upgrade.
However, it seems this should also be configurable. I would suggest a new config property in the .runtime.config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.runtimeconfig.json
already has settings which control auto-upgrade - also called roll-forward (I should use that term instead). See this doc.
Those apply to frameworks though, not assemblies. But in the context of components (since those rely on framework fromt he app) it would make sense to use them for individual assemblies as well.
|
||
"Advanced" version which would return an ALC instance and not just the main assembly. Allows for additional changes to the `AssemblyLoadContext`, like registering an event handler to the `Resolving` event for example. | ||
Also adds the ability to specify: | ||
* `fallbackContext` which is the `AssemblyLoadContext` to defer to when the assembly resolution is not possible in this current context. By default this is the `AssemblyLoadContext.Default` (the app's context). This allows for creating effectively parent-child relationships between the load contexts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the parent-child relationship and the various possible strategies mentioned below, it seems this would be better named parentContext
|
||
Open questions: | ||
* Which is the default behavior? | ||
* Does the framework implement more than one behavior and lets users choose? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkotas suggested in another thread that we defer the convenience API until we get a feel for which is the right approach.
When we were prototyping hardware intrinsics for 2.1, late in the cycle we pushed the work into experimental nuget packages. I am wondering whether that might be the right approach here.
I don't see a reason why we couldn't create a nuget package to expose these API's experimentally so that we can get real world experience and hand on feedback.
The experimental package(s) could be used to provide sample code for the usage of the existing 2.1 AssemblyLoadContext and help us develop proper documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To help move forward providing an instance method on AssemblyLoadConext to enable the LoadWithDependencies seems like a reasonable way to acheive the balance we have been discussing. It will lead people towards going for isolation, but enables them to choose which ALC to load into. We can then choose which version is most appropriate for Assembly.Load*WithDependencies (eg. the default)
* There's no pollution of the parent context - no new files will be loaded into the parent context. | ||
* Auto-upgrade - components will auto-upgrade to the version of a dependency they share with the app (downgrade will not occur, in that case the dependency would be loaded in isolation). This can lead to incompatibilities. | ||
|
||
Open questions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important consideration for me is versioning of EventSource names and manifests when multiple versions are loaded into the process. Same for diagnostics source.
It is my understanding that all isolation models will allow to subscribe to event and diagnostics sources from the loaded assembly as those sources are defined in system assemblies. If so - some naming issue may occur.
CC: @vancem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that isolation is not a new feature. It has been available for a long time now. It's done through the AssemblyLoadContext
class which is not new. Also some methods on Assembly
provide isolation (Assembly.LoadFile
loads the specified file in isolation). So it's already possible to load even the same assembly twice (or two similar assemblies and so on). I don't know if there's been any thinking on the impact of this capability on the EventSource mechanisms. I'll let @vancem or @brianrob to comment on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm that from assembly binding perspective the System.Private.CoreLib
which is where most of the code for EventSource
and alike lives is only ever loaded into the app once. In fact corelib is special and the runtime will not allow loading a second copy even if custom code would specifically ask for it. But that doesn't mean that the event source code doesn't have some internal data structures which might have issues with multiple copies of the same assembly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyKanzhelev - I view all this loader stuff as pretty orthogonal to event/source naming issues in EventSource/DiagnosticSource. (Yes there are issues but this feature does not make the problem any better or worse).
The one place I can think of that is a issue is that TODAY we make it illegal to create two EventSources with the same name in the same AppDomain. We did this for relatively weak reasons (it is a perf issue). This behavior WILL cause grief if an assembly is loaded twice (because of isolation). It will call the SHARED System.Private.Corelib version of EventSource which will fail because the name is already used. I have always been ambivalent about this rule, so I don't mind if we stop checking for it.
As far as the rest goes, I think it is roughly orthogonal. The names WILL overlap but that is good (you think of them as the 'same' events really). For DiagnosticSource, there will be a separate publication point for each System.Diagnostic.DiagnosticSource assembly that is loaded (since there is a static in their that you subscribe to to get events.
So we may wish to relax the uniqueness check in EventSource, but otherwise I think it will be OK. If you have specific concerns, we can walk through specific cases in more detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This behavior WILL cause grief if an assembly is loaded twice (because of isolation). It will call the SHARED System.Private.Corelib version of EventSource which will fail because the name is already used.
So custom EventSource creation will crash? Potentially an entire process? Or this will be handled? In any case - this may become a common case with plugins using libraries like Application Insights for telemetry reporting. Application Insights SDK instantiate an EventSource
for troubleshooting.
For DiagnosticSource, there will be a separate publication point for each System.Diagnostic.DiagnosticSource assembly that is loaded
Will different DiagnosticSource
-s share the AsyncLocal (Activity.Current
)? If not - distributed tracing scenarios like Activity started in ASP.NET core host and than child activity created inside the plugin would not work. Is there anything that can be done to special-case the DiagnosticsSource?
* MSBuild tasks - all tasks in MSBuild are dynamically loaded. Some tasks come with additional dependencies which can collide with each other or MSBuild itself as well. | ||
* Roslyn analyzers - similar to MSBuild tasks, the Roslyn compiler dynamically loads analyzers which are separate components with potentially conflicting dependencies. | ||
* XUnit loading tests - the test runner acts as an app and the test is loaded dynamically. The test can have any number of dependencies. Finding and resolving those dependencies is challenging. | ||
* ASP .NET's `dotnet watch` - ability to dynamically reload an app without restarting the process. Each version of the app is inherently in collision with any previous version. The old version should be unloaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include ASP.NET Core plugins like z-pages (see performance-profiling-controller.md) and monitoring tools like Application Insights.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC: @glennc @davidfowl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyKanzhelev In which way do you think the performance profiling controller is related to the dynamic component loading? I can potentially see some use cases, but I don't see that as a good example of the scenarios we want to support (not saying it's not applicable).
Monitoring tools and other profiler-like capabilities are probably more related to the [statup-hook] (https://github.com/dotnet/core-setup/blob/master/Documentation/design-docs/host-startup-hook.md) functionality. Also this new feature will explicitely not customize startup loading in any way. It will be on-demand - that is called by a user code. It is possible for startup-hooks to use this feature to load their own dependencies though.
I guess I'm missing the point here though - can you please describe in a bit more detail how do you think these are related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The discussion was how one can load ASP.NET components to implement profiling controller into an application that has no ASP.NET stack loaded or has it's own version of ASP.NET stack. Also those profiling controllers may implement some pluggable model to load data collection plug-ins on demand in runtime. Versioning and isolation was one of the discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For isolation, AssemblyLoadContext
is definitely the solution. For the on demand runtime loading - that would be this feature (it's possible, but rather hard right now). Thanks for pointing that out - I'll include it in the list.
* MSBuild tasks - all tasks in MSBuild are dynamically loaded. Some tasks come with additional dependencies which can collide with each other or MSBuild itself as well. | ||
* Roslyn analyzers - similar to MSBuild tasks, the Roslyn compiler dynamically loads analyzers which are separate components with potentially conflicting dependencies. | ||
* XUnit loading tests - the test runner acts as an app and the test is loaded dynamically. The test can have any number of dependencies. Finding and resolving those dependencies is challenging. | ||
* ASP .NET's `dotnet watch` - ability to dynamically reload an app without restarting the process. Each version of the app is inherently in collision with any previous version. The old version should be unloaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another scenario is startup hook. Perhaps spec of profiler hook should be updated to suggest using isolation loading for those hooks. @sbomer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to ICorProfiler
when you say profiler? That is native and is not impacted by this work. The startup hook is interesting though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, I misspoke. I meant startup hook I referenced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Sorry about the previous comment, didn't see this one yet). I agree that using isolation for startup hooks makes sense in some cases. But that functionality is available already. This feature is actually not about providing isolation per-se. It's about the ability to load components with dependencies (which has been hard so far). As seen in the discussion in this proposal, it's likely we will provide a way to do this loading both in isolation or not.
I've implemented a lot of this already, and been down the road of quirks you'll have to handle if you want a managed AssemblyLoadContext to imitate corehost's deps.json parsing. Feel free to use this as a prototype. https://github.com/natemcmaster/DotNetCorePlugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I like the approach. I still feel we need some thinking around if ALC.LoadWIthDependencies is a factory method or an instance.
```csharp | ||
class AssemblyLoadContext | ||
{ | ||
public static AssemblyLoadContext CreateForAssemblyWithDependencies( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel we are going to have scenarios where we will want multiple plug-ins loaded into the same assembly load context. I prefer this factory method to be an instance method so that developers can choose.
|
||
Open questions: | ||
* Which is the default behavior? | ||
* Does the framework implement more than one behavior and lets users choose? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To help move forward providing an instance method on AssemblyLoadConext to enable the LoadWithDependencies seems like a reasonable way to acheive the balance we have been discussing. It will lead people towards going for isolation, but enables them to choose which ALC to load into. We can then choose which version is most appropriate for Assembly.Load*WithDependencies (eg. the default)
@natemcmaster I'm very familiar with your code - thanks a lot for putting that together. Please note that we're explicitly proposing to not implement the |
+💯. Awesome, that's a much better way to do this. |
If I understand this feature properly it does not add anything truly new to the runtime (users COULD implement it themselves with AssemblyLoadContext), but is effectively a 'shortcut' for a common case (a plugin with non-trivial dependencies. If this is not true, we should add the (hopefully small) additions to AssemblyLoadContext to expose the primitives to make it true. My bigest piece of advice is that in the past the loader has become a mess of special cases that became very hard to use or reason able. We should try to avoid that here. In particular.
This is important because most of the open issues have to do with having different policies or extended features etc. We want these to be easy to add or simply have users do it (because it is that easy). Note that we can keep this object model 'private' for a while if we want (until we are comfortable that we won't want to change it). The basic point is that loaders really don't DO much (they take names and find the assembly that 'matches'), but they are SHARED (that is the whole app uses it), which is what forces it to have 1000 masters (each needing a tweek). |
What is the status of this proposal? It is now over a year old and with 3.0 (I haven't tried 3.1 yet) I am still unable to dynamically load an assembly with AssemblyLoadContext and have the dependent assemblies (in deps.json) automatically loaded.I see this is still open. Is there a plan to included this? Am I missing something? |
Largely this design was implemented in .NET Core 3. Here are a few links that hopefully will help: |
@vitek-karas what's the status of this? |
A simpler approach was taken for .NET Core 3.0
The higher-level API to make it even easier to load plugins without the need for an explicit custom load context is not part of .NET Core 3.0. There are also no immediate plans to add that functionality to .NET. As such I'm closing this proposal. |
This design proposal is trying to address the lack of easy-to-use functionality to dynamically load components with their own dependencies.