Allow custom steps to share state #2019
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The test infrastructure runs all tests in the same process, so custom step assemblies from different testcases must have different assembly names.
vitek-karas
approved these changes
May 10, 2021
marek-safar
approved these changes
May 10, 2021
jonpryor
pushed a commit
to dotnet/android
that referenced
this pull request
Jun 2, 2021
Context: dotnet/linker#1953 Context: dotnet/linker#1774 Context: dotnet/linker#1774 (comment) Context: dotnet/linker#1979 Context: dotnet/linker#2019 Context: dotnet/linker#2045 Context: dotnet/java-interop@2573dc8 Context: #5870 Context: #5878 Update the custom linker steps to use the new `IMarkHandler` API which runs logic during "MarkStep". (See [this list][0] of pipeline steps for additional context.) As part of this, I've removed the workaround that loads all referenced assemblies up-front and simplified some of the linker MSBuild targets. Some of the `MonoDroid.Tuner` steps were duplicated and changed to implement the new `IMarkHandler` interface, to avoid touching the `MonoDroid.Tuner` code. In my analysis of the custom steps, most of them "just work" if we call them only for marked members, because they ultimately either: - Just call `AddPreserved*()` to conditionally keep members of types (which with the new API will just happen when the type is marked) - In the case of `FixAbstractMethodsStep()`, inject missing interface implementations which will only be kept if they are marked for some other reason. Most of the steps have been updated in a straightforward way based on the above. The exceptions are: - `AddKeepAlivesStep` needs to run on all code that survived linking, and can run as a normal step. - `ApplyPreserveAttribute`: this step globally marks members with `PreserveAttribute`. - The step is only active for non-SDK link assemblies. Usually we root non-SDK assemblies, but it will be a problem if linking all assemblies. - For now, I updated it to use the new custom step API on assembly mark -- so it will scan for the attribute in all marked assemblies -- but we should investigate whether this is good enough. - This can't be migrated to use `IMarkHandler` because it needs to scan code for attributes, including unmarked code. - `PreserveExportedTypes`: similar to the above, this globally marks members with `Export*Attribute`. It's used for java interop in cases where the java methods aren't referenced statically, like from xml, or for special serialization methods. - It's only active for non-SDK assemblies, so like above it will be a problem only if linking all assemblies. - Like above, I've made it scan marked assemblies. - `PreserveApplications`: globally scans for `ApplicationAttribute` on types/assemblies, and sets the `TypePreserve` annotation for any types referenced by the attribute. - This step technically does a global scan, but it's likely to work on "mark type"/"mark assembly", as `ApplicationAttribute` is only used on types/assembies that are already kept. - I've updated it to use the new `IMarkHandler` API. Additionally, as per dotnet/java-interop@2573dc8c, stop using `TypeDefinitionCache` everywhere and instead use `IMetadataResolver` to support caching `TypeDefinition`s across shared steps. For .NET 6, `LinkContextMetadataResolver` implements the `IMetadataResolver` interface in terms of `LinkContext`. [0]: https://github.com/mono/linker/blob/main/src/linker/Linker/Driver.cs#L714-L730
agocke
pushed a commit
to dotnet/runtime
that referenced
this pull request
Nov 16, 2022
Commit migrated from dotnet/linker@8aeec85
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Loading from byte array would load each
--custom-stepargument into a new ALC, even when using the same assembly for multiple custom steps. Instead of going all-in with ALCs, this change just loads custom steps into the default ALC to allow sharing state. This means it won't be possible to use custom steps from different assemblies which happen to have the same name.In the future if we need to support isolated custom steps with dependencies, we could consider following the guidance for plugins at https://docs.microsoft.com/en-us/dotnet/core/tutorials/creating-app-with-plugin-support, which would involve:
Fixes #1314