Move marshal method pipeline from outer build to inner (per-RID) build#11076
Open
Move marshal method pipeline from outer build to inner (per-RID) build#11076
Conversation
… inner (per-RID) build Move marshal method classification, assembly rewriting, and LLVM IR generation into a new _RewriteMarshalMethodsInner target that runs AfterTargets=_PostTrimmingPipeline in the inner build. Previously, GenerateJavaStubs classified marshal methods and stored the result in NativeCodeGenState via BuildEngine4. RewriteMarshalMethods then consumed that state and mutated assemblies in-place, changing metadata tokens. Downstream outer-build tasks (GenerateTypeMappings, GenerateNativeMarshalMethodSources) could see stale token data from the pre-rewrite NativeCodeGenState. By moving classification and rewriting into the inner build, the outer build only ever sees already-rewritten assemblies. The typemap.xml files generated by _AfterILLinkAdditionalSteps (FindTypeMapObjectsStep) now contain correct post-rewrite tokens, so GenerateTypeMappings can always use the .xml path without special-casing. Key changes: - RewriteMarshalMethods is now self-contained: opens trimmed assemblies with Cecil, classifies marshal methods, rewrites in-place, and generates the marshal_methods.<abi>.ll file - GenerateJavaStubs no longer classifies marshal methods (classifier is null) - GenerateNativeMarshalMethodSources skips .ll generation when marshal methods are enabled (inner build already produced the file) - GenerateTypeMappings always uses .typemap.xml files since tokens are now stable post-rewrite - MarshalMethodCecilAdapter gains CreateNativeCodeGenStateObjectFromClassifier for building the LLVM IR state without full NativeCodeGenState
Move all marshal_methods.{abi}.ll LLVM IR generation into the inner
build's RewriteMarshalMethods task, eliminating the duplicate code path
in the outer build's GenerateNativeMarshalMethodSources.
When marshal methods are enabled, RewriteMarshalMethods classifies,
rewrites assemblies, and generates a full .ll as before. When disabled,
it now generates an empty/minimal .ll with just the structural
scaffolding the native runtime links against (using an empty
NativeCodeGenStateObject).
Strip GenerateNativeMarshalMethodSources down to P/Invoke preservation
only: remove EnableMarshalMethods, EnableManagedMarshalMethodsLookup,
ResolvedAssemblies, SatelliteAssemblies, AndroidRuntime properties and
all .ll generation code.
Remove dead code from the generator hierarchy:
- MarshalMethodsNativeAssemblyGenerator: remove generateEmptyCode flag,
GenerateEmptyCode property, targetArch field, and disabled constructor
- MarshalMethodsNativeAssemblyGeneratorMonoVM: remove disabled ctor
- MarshalMethodsNativeAssemblyGeneratorCoreCLR: remove disabled ctor
and unused Xamarin.Android.Tools using
The _RewriteMarshalMethodsInner target now runs unconditionally (no
conditions on PublishTrimmed, _AndroidUseMarshalMethods, or
AndroidIncludeDebugSymbols) and passes EnableMarshalMethods based on
the _AndroidUseMarshalMethods property.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves marshal method classification + assembly rewriting + marshal_methods.{abi}.ll generation from the outer build into the inner (per-RID) build, so it operates on already-trimmed assemblies and avoids stale metadata token issues.
Changes:
- Run
RewriteMarshalMethodsper-RID after ILLink to classify/rewrite assemblies and generatemarshal_methods.{abi}.ll(or a minimal scaffolding.llwhen disabled). - Simplify
GenerateNativeMarshalMethodSourcesto only generatepinvoke_preserve.{abi}.llwhen native runtime linking is enabled. - Adjust typemap generation to always use
.typemap.xml(tokens should now be post-rewrite) and remove marshal-method-specific typemap fallbacks.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Stops passing marshal-method-specific inputs to GenerateNativeMarshalMethodSources after responsibility shift to inner build. |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGeneratorMonoVM.cs | Removes “disabled” constructor path and updates method-name emission logic. |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGeneratorCoreCLR.cs | Removes “disabled” constructor path (generator now always uses state object). |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs | Removes generateEmptyCode plumbing; empty output now driven by empty NativeCodeGenStateObject. |
| src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs | Adds adapter for inner-build classification → NativeCodeGenStateObject conversion. |
| src/Xamarin.Android.Build.Tasks/Tasks/RewriteMarshalMethods.cs | Reworks task to run per-RID, classify/rewrite, and generate marshal_methods.{abi}.ll (or minimal .ll). |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTypeMappings.cs | Always generates typemaps from .typemap.xml; limits native-state comparison to checked builds. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeMarshalMethodSources.cs | Now only handles P/Invoke preservation and unregisters state object for cleanup. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs | Removes outer-build marshal method classification; classifier is now always null here. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets | Adds _RewriteMarshalMethodsInner target to execute marshal method pipeline after trimming in inner build. |
…tory Instead of rewriting assemblies in-place (which fails when multiple RIDs share the same input path, e.g. NuGet runtime pack with PublishTrimmed=false), write rewritten assemblies to a per-RID output directory and update @(ResolvedFileToPublish) items so downstream processing uses the copies. This removes the PublishTrimmed gate entirely - EnableMarshalMethods is now passed directly as $(_AndroidUseMarshalMethods).
Exclude _RewriteMarshalMethodsInner from NativeAOT builds (which don't use .ll-based marshal methods) and remove PDB output items that caused NETSDK1152 duplicate relative path errors.
Address three bugs introduced in the marshal methods pipeline move: 1. Wrap XAAssemblyResolver in try/finally so it is always disposed, even when classification, rewriting, or LLVM IR generation throws. 2. Thread ManagedMarshalMethodsLookupInfo through to CreateNativeCodeGenStateObjectFromClassifier so that when EnableManagedMarshalMethodsLookup is true, the NativeCallback indices (AssemblyIndex/ClassIndex/MethodIndex) are populated before LLVM IR generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hods are enabled When marshal methods are enabled, Application/Instrumentation types that have no dynamically registered methods should be excluded from ApplicationRegistration.java — their JCWs no longer have __md_methods. Previously, the classifier in GenerateJavaStubs (outer build) filtered these types. The marshal-methods-v2 PR moved classification to the inner build, but left ApplicationRegistration.java generation in the outer build without a classifier, causing javac errors. Fix: generate ApplicationRegistration.java in RewriteMarshalMethods (inner build) where the classifier is available. Each inner build writes to a per-RID directory; the target copies it into the shared Java source tree. When marshal methods are disabled, the outer build's GenerateAdditionalProviderSources still generates it as before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Moves marshal method classification, assembly rewriting, and
.llLLVM IR generation from the outer build into the inner (per-RID) build'sRewriteMarshalMethodstask, running after ILLink trimming on already-trimmed assemblies. This eliminates the token staleness problem where classification captured token state, then rewriting mutated tokens, then downstream tasks saw stale data.When marshal methods are enabled, the task classifies, rewrites assemblies, and generates a full
.ll. When disabled, it generates an empty/minimal.llwith just the structural scaffolding the native runtime links against.GenerateNativeMarshalMethodSourcesis stripped down to P/Invoke preservation only.