Move AddKeepAlives trimmer step to post-trimming task#10952
Move AddKeepAlives trimmer step to post-trimming task#10952jonathanpeppers merged 12 commits intomainfrom
AddKeepAlives trimmer step to post-trimming task#10952Conversation
Migrate AddKeepAlivesStep out of the ILLink custom step pipeline into a standalone MSBuild task that runs AfterTargets="ILLink", following the same pattern established by StripEmbeddedLibraries in #10894. Core IL-rewriting logic is extracted into AddKeepAlivesHelper, shared by both the new task (trimmed builds) and the existing pipeline step (non-trimmed builds via LinkAssembliesNoShrink).
Replace DefaultAssemblyResolver with DirectoryAssemblyResolver (ReadWrite=true) to avoid file handle conflicts on Windows. The directory resolver caches all assemblies (both explicit and dependency-resolved), preventing duplicate file opens that caused IOException when the same assembly was opened as both a dependency and a primary target.
Replace standalone AddKeepAlives and StripEmbeddedLibraries MSBuild tasks with a single PostTrimmingPipeline task that opens assemblies once (via DirectoryAssemblyResolver with ReadWrite) and runs both modifications in a single pass. Extract StripEmbeddedLibrariesStep as an IAssemblyModifierPipelineStep for reuse.
…Step pattern Use List<IAssemblyModifierPipelineStep> with StripEmbeddedLibrariesStep and PostTrimmingAddKeepAlivesStep instead of calling helpers directly. Move the IsFrameworkAssembly check into StripEmbeddedLibrariesStep.ProcessAssembly and remove all outer-loop filtering so each step handles its own guards internally.
There was a problem hiding this comment.
Pull request overview
Moves the AddKeepAlives IL rewriting from an ILLink custom step into a post-ILLink MSBuild task, aligning with the earlier “post-trimming task” pattern and allowing the same KeepAlive injection to run for trimmed builds after ILLink.
Changes:
- Introduces a new
PostTrimmingPipelineMSBuild task that runs post-trimming assembly modifications (resource stripping + optional KeepAlive injection) in one pass. - Extracts KeepAlive IL-rewrite logic into
AddKeepAlivesHelperand reuses it from both the existing non-trim pipeline and the new post-trim pipeline. - Removes the old
StripEmbeddedLibrariestask and dropsAddKeepAlivesStepfrom the ILLink project/custom step registration.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj | Adds compilation of new linker helper/steps required by the new post-trim pipeline. |
| src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs | Removes the old standalone StripEmbeddedLibraries task implementation. |
| src/Xamarin.Android.Build.Tasks/Tasks/PostTrimmingPipeline.cs | Adds new post-ILLink task that opens assemblies read/write and applies post-trim modification steps. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets | Switches from StripEmbeddedLibraries to PostTrimmingPipeline and removes ILLink custom-step registration for AddKeepAlives. |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibrariesStep.cs | Adds pipeline step that strips embedded Android resource payloads from assemblies. |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/PostTrimmingAddKeepAlivesStep.cs | Adds post-trimming pipeline step wrapper to call the shared KeepAlive helper. |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesStep.cs | Refactors to delegate logic to AddKeepAlivesHelper for the non-trim pipeline path. |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesHelper.cs | Adds shared IL-rewrite helper used by both non-trim and post-trim paths. |
| src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj | Stops compiling/linking AddKeepAlivesStep into the ILLink project. |
src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/AddKeepAlivesHelper.cs
Outdated
Show resolved
Hide resolved
src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibrariesStep.cs
Show resolved
Hide resolved
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj
Outdated
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…ated csproj comment Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com>
…esolution attempts Co-authored-by: sbomer <787361+sbomer@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/android/sessions/399b2f01-0576-42f4-a1d4-d0f0d31f5eb7
Fix nullable Func delegate, whitespace, and csproj comment in PostTrimmingPipeline
…beddedLibrariesStep.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
AddKeepAlives trimmer step to standalone MSBuild taskAddKeepAlives trimmer step to post-trimming task
|
@jonathanpeppers this now moves the step to a separate post-trimming pipeline. #10891 might let us merge this with the existing assembly modification pipeline, but for now it is separate. PTAL! |
jonathanpeppers
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ✅ LGTM
Found 0 issues.
Clean refactoring that extracts AddKeepAlives IL-rewriting logic from the ILLink custom step pipeline into a shared AddKeepAlivesHelper class, then runs it as a post-trimming MSBuild task via PostTrimmingPipeline. This follows the same pattern established by StripEmbeddedLibraries in #10894.
What I verified:
PostTrimmingPipelineextendsAndroidTask, hasTaskPrefix = "PTP", returns!Log.HasLoggedErrors✅[Required]propertyAssemblieshas proper default= []✅- Non-required
AddKeepAlivesandDeterministicare value types (bool), no nullable issue ✅ - Core IL-rewriting logic in
AddKeepAlivesHelperis a pure extraction — no behavioral changes, same guards (HasTypeReference,IsDotNetAndroidAssembly) ✅ StripEmbeddedLibrariesStepcorrectly moved from the deletedStripEmbeddedLibrariestask with logic preserved ✅PostTrimmingAddKeepAlivesStepcorrectly documents that it skips theIsAndroidUserAssemblypre-filter (matching original ILLink behavior) ✅- Corlib resolution is properly memoized in the lambda closure ✅
- Target renamed from
_StripEmbeddedLibrariesto_PostTrimmingPipelinewithAfterTargets="ILLink"— same ordering ✅ AddKeepAlivesStepin the ILLink_TrimmerCustomStepslist is correctly removed ✅- All CI checks pass ✅
👍 Nice consolidation — running both steps in a single pass over assemblies with one assembly open/write cycle is a good efficiency improvement.
Review generated by android-reviewer from review guidelines.
Migrate AddKeepAlivesStep out of the ILLink custom step pipeline into a standalone MSBuild task that runs AfterTargets="ILLink", following the same pattern established by StripEmbeddedLibraries in #10894.
Core IL-rewriting logic is extracted into AddKeepAlivesHelper, shared by both the new task (trimmed builds) and the existing pipeline step (non-trimmed builds via LinkAssembliesNoShrink).