[tools] Add a new 'trimmable-static' registrar, that uses trimmable type maps. Fixes #23108.#25079
[tools] Add a new 'trimmable-static' registrar, that uses trimmable type maps. Fixes #23108.#25079rolfbjarne wants to merge 10 commits intomainfrom
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new trimmable-static registrar mode that leverages .NET “Interop Type Maps” to support trimmable registrations, wiring the new mode through the linker, runtime, MSBuild targets, and test variations.
Changes:
- Add a new linker step (
TrimmableRegistrarStep) that emits type-map assemblies/attributes used for type mapping and proxy lookup. - Extend runtime + native runtime flags and lookup paths to support
trimmable-static, including proxy-attribute based construction and unmanaged entrypoint lookup fallback. - Add new build/test plumbing (MSBuild properties, runtimeconfig host options, xharness variations) to exercise the new registrar mode.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/dotnet-linker/Steps/TrimmableRegistrarStep.cs | New linker step that generates typemap assemblies and proxy attribute types/mappings. |
| tools/dotnet-linker/Steps/RegistrarStep.cs | Treat TrimmableStatic similarly to managed-static in registrar output flow. |
| tools/dotnet-linker/Steps/ManagedRegistrarStep.cs | Enable managed-registrar step for trimmable-static to generate UCO trampolines. |
| tools/dotnet-linker/LinkerConfiguration.cs | Add EntryAssembly helper and new linker config values for typemap output/name. |
| tools/dotnet-linker/CecilExtensions.cs | Add TryFindSingle helper for Cecil collections. |
| tools/dotnet-linker/AppBundleRewriter.cs | Add typemap-related Cecil lookups and System.Console assembly support. |
| tools/dotnet-linker/.vscode/launch.json | Adds a VS Code launcher configuration for debugging the linker. |
| tools/common/Target.cs | Emit a runtime flag indicating trimmable-static registrar usage. |
| tools/common/StaticRegistrar.cs | Adjust static registrar generation for trimmable-static and extend dlsym signature usage. |
| tools/common/Optimizations.cs | Allow static-registrar-related optimizations for trimmable-static. |
| tools/common/Assembly.cs | Treat trimmable-static like other static registrar modes for -force_load decisions. |
| tools/common/Application.cs | Add RegistrarMode value + parsing + typemap config values. |
| tests/xharness/Jenkins/TestVariationsFactory.cs | Add CI variations for trimmable-static registrar. |
| tests/monotouch-test/Foundation/StringTest.cs | Adds extra logging in a test case (currently noisy). |
| tests/monotouch-test/dotnet/shared.csproj | Notes/placeholder for enabling trimmable-static registrar in tests. |
| tests/monotouch-test/dotnet/macOS/monotouch-test.csproj | Adds host configuration option for TypeMappingEntryAssembly. |
| tests/monotouch-test/dotnet/macOS/Makefile | Adds convenience targets for trimmable-static registrar runs. |
| tests/monotouch-test/dotnet/macOS/.vscode/tasks.json | Adds VS Code build task for monotouch-test macOS. |
| tests/monotouch-test/dotnet/macOS/.vscode/launch.json | Adds VS Code launch profiles for running monotouch-test macOS. |
| tests/dotnet/MySimpleApp/shared.csproj | Sets registrar to trimmable-static for MySimpleApp. |
| tests/common/test-variations.csproj | Defines new test variations for trimmable-static registrar. |
| tests/common/shared-dotnet.mk | Minor build log output improvement. |
| src/ObjCRuntime/TypeMaps.cs | New runtime helper to lazily initialize typemap dictionaries. |
| src/ObjCRuntime/Runtime.cs | Add trimmable-static runtime flag + proxy-based construction + new unmanaged lookup signature. |
| src/ObjCRuntime/RegistrarHelper.cs | Extend unmanaged lookup to optionally resolve via typemap/proxy attribute. |
| src/ObjCRuntime/Registrar.cs | Add IsStubClass convenience property on ObjCType. |
| src/ObjCRuntime/Class.cs | Add trimmable typemap lookup path and introduce proxy attribute base types/universe markers. |
| src/ILLink.Substitutions.tvOS.xml | Add substitutions for IsTrimmableStaticRegistrar. |
| src/ILLink.Substitutions.macOS.xml | Add substitutions for IsTrimmableStaticRegistrar. |
| src/ILLink.Substitutions.MacCatalyst.xml | Add substitutions for IsTrimmableStaticRegistrar. |
| src/ILLink.Substitutions.iOS.xml | Add substitutions for IsTrimmableStaticRegistrar. |
| src/frameworks.sources | Include new ObjCRuntime/TypeMaps.cs in build. |
| scripts/rsp-to-csproj/rsp-to-csproj.cs | Add Link="..." metadata for generated csproj items when applicable. |
| runtime/xamarin/runtime.h | Add trimmable-static registrar flag setter + extend registrar dlsym signature. |
| runtime/runtime.m | Implement trimmable-static registrar flag + pass objc class name to unmanaged lookup. |
| runtime/delegates.t4 | Extend managed delegate signature for unmanaged lookup to include objc class name. |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Wire registrar mode to linker steps, typemap output publishing, and host config options. |
| "program": "/Users/rolf/work/dotnet/macios/trimmable-type-map/macios/builds/downloads/dotnet-sdk-10.0.300-preview.0.26177.108/dotnet", | ||
| "args": [ | ||
| "/Users/rolf/work/dotnet/macios/trimmable-type-map/macios/packages/microsoft.net.illink.tasks/10.0.5/tools/net/illink.dll", | ||
| "-x", | ||
| "/Users/rolf/work/dotnet/macios/trimmable-type-map/macios/tests/nunit.framework.xml", | ||
| "-x", | ||
| "bin/Debug/net10.0-macos/osx-arm64/LinkerInputs.cache.xml", | ||
| "-a", |
There was a problem hiding this comment.
This new file contains a machine-specific VS Code launch configuration (absolute paths under /Users/..., large hard-coded args). This makes the repo non-portable and will quickly go stale for other developers/CI. Please remove it from the repo or rewrite it to use ${workspaceFolder} and relative paths with minimal, generic arguments (similar to tests/xharness/.vscode/launch.json).
| simple: export RID= | ||
| simple: export RUNTIMEIDENTIFIER= | ||
| simple: export RUNTIMEIDENTIFIERS= | ||
| simple: export TEST_VARIATION=trimmable-static-registrar | ||
| simple: export TestVariation=trimmable-static-registrar | ||
| simple: | ||
| mair | ||
| git clean -xfd | ||
| make build | ||
| bin/Debug/*/*/monotouchtest.app/Contents/MacOS/monotouchtest --test apitest.KernelNotificationTest | ||
|
|
||
| s: export RID= | ||
| s: export RUNTIMEIDENTIFIER= | ||
| s: export RUNTIMEIDENTIFIERS= | ||
| s: export TEST_VARIATION=trimmable-static-registrar | ||
| s: export TestVariation=trimmable-static-registrar | ||
| s: | ||
| madl | ||
| git clean -xfd | ||
| make build | ||
| bin/Debug/*/*/monotouchtest.app/Contents/MacOS/monotouchtest --test MonoTouchFixtures.ObjCRuntime.GCHandleSwitchRaceTest |
There was a problem hiding this comment.
The new Makefile targets (simple/s/a) invoke non-standard commands ("mair", "madl") and run git clean -xfd, which is destructive and will delete local work/artifacts. These look like local developer aliases and shouldn't be committed as part of the test harness. Please remove these targets or replace them with safe, repo-standard commands.
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
| NSString.Empty.DangerousRelease (); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
| NSString.Empty.DangerousRelease (); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
| NSString.Empty.DangerousRelease (); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
| NSString.Empty.DangerousRelease (); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
| NSString.Empty.DangerousRelease (); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
|
|
||
| Assert.That (NSString.Empty.RetainCount, Is.EqualTo (nuint.MaxValue), "RetainCount"); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | ||
| Assert.That (NSString.Empty.Compare (new NSString (string.Empty)), Is.EqualTo (NSComparisonResult.Same), "Same"); | ||
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); |
There was a problem hiding this comment.
The added Console.WriteLine calls will produce a lot of noise in CI test logs and can make failures harder to spot. Please remove these debug prints or guard them behind a conditional (e.g., #if DEBUG/TRACE) or an environment switch so normal test runs stay quiet.
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| NSString.Empty.DangerousRelease (); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| NSString.Empty.DangerousRelease (); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| NSString.Empty.DangerousRelease (); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| NSString.Empty.DangerousRelease (); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| NSString.Empty.DangerousRelease (); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| Assert.That (NSString.Empty.RetainCount, Is.EqualTo (nuint.MaxValue), "RetainCount"); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| Assert.That (NSString.Empty.Compare (new NSString (string.Empty)), Is.EqualTo (NSComparisonResult.Same), "Same"); | |
| Console.WriteLine ($"ReleaseEmptyString () NSString.Empty: {NSString.Empty.Handle} ({NSString.Empty.RetainCount})"); | |
| NSString.Empty.DangerousRelease (); | |
| NSString.Empty.DangerousRelease (); | |
| NSString.Empty.DangerousRelease (); | |
| NSString.Empty.DangerousRelease (); | |
| NSString.Empty.DangerousRelease (); | |
| Assert.That (NSString.Empty.RetainCount, Is.EqualTo (nuint.MaxValue), "RetainCount"); | |
| Assert.That (NSString.Empty.Compare (new NSString (string.Empty)), Is.EqualTo (NSComparisonResult.Same), "Same"); |
tools/common/Application.cs
Outdated
| return !AreAnyAssembliesTrimmed; | ||
| case ApplePlatform.MacOSX: | ||
| return (Registrar == RegistrarMode.Static || Registrar == RegistrarMode.ManagedStatic) && !AreAnyAssembliesTrimmed; | ||
| return (Registrar == RegistrarMode.Static || Registrar == RegistrarMode.ManagedStatic || Registrar == RegistrarMode.TrimmableStatic) && !AreAnyAssembliesTrimmed; |
There was a problem hiding this comment.
There is a non-ASCII/zero-width whitespace character in this condition (|| Registrar), which makes diffs and editing error-prone. Please replace it with a normal ASCII space (or remove extra whitespace) so the line is plain text.
| return (Registrar == RegistrarMode.Static || Registrar == RegistrarMode.ManagedStatic || Registrar == RegistrarMode.TrimmableStatic) && !AreAnyAssembliesTrimmed; | |
| return (Registrar == RegistrarMode.Static || Registrar == RegistrarMode.ManagedStatic || Registrar == RegistrarMode.TrimmableStatic) && !AreAnyAssembliesTrimmed; |
tools/common/StaticRegistrar.cs
Outdated
| var flags = MTTypeFlags.None; | ||
|
|
||
| if (IsCustomType (@class)) | ||
| flags |= MTTypeFlags.CustomType; |
There was a problem hiding this comment.
This line contains a non-ASCII whitespace character between flags and |= (visible as flags |=). Hidden characters like this are easy to miss and can cause churn in future diffs. Please replace it with normal ASCII whitespace.
| flags |= MTTypeFlags.CustomType; | |
| flags |= MTTypeFlags.CustomType; |
| </Compile> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> |
There was a problem hiding this comment.
This RuntimeHostConfigurationOption is added unconditionally for the monotouch-test macOS project. If this setting is only required for the trimmable-static registrar, it should be conditioned on the corresponding TestVariation/Registrar to avoid changing runtimeconfig for all variants (including dynamic/static/managed-static registrar runs).
| <ItemGroup> | |
| <ItemGroup Condition="'$(Registrar)' == 'trimmable-static'"> |
| @@ -725,6 +731,7 @@ | |||
| <RuntimeHostConfigurationOption Include="ObjCRUntime.AggressiveAttributeTrimmingOnlyWithStaticRegistrar" value="true" Trim="true" Condition="'$(MobileAggressiveAttributeTrimming)' == 'true' And ('$(Registrar)' == 'static' Or '$(Registrar)' == 'managed-static')" /> | |||
There was a problem hiding this comment.
ObjCRuntime.AggressiveAttributeTrimmingOnlyWithStaticRegistrar is conditioned on Registrar being static or managed-static, but the new trimmable-static registrar is also a static registrar mode. If this feature gate is intended to apply to all static registrar modes, please include trimmable-static in the condition as well so behavior stays consistent.
| <RuntimeHostConfigurationOption Include="ObjCRUntime.AggressiveAttributeTrimmingOnlyWithStaticRegistrar" value="true" Trim="true" Condition="'$(MobileAggressiveAttributeTrimming)' == 'true' And ('$(Registrar)' == 'static' Or '$(Registrar)' == 'managed-static')" /> | |
| <RuntimeHostConfigurationOption Include="ObjCRUntime.AggressiveAttributeTrimmingOnlyWithStaticRegistrar" value="true" Trim="true" Condition="'$(MobileAggressiveAttributeTrimming)' == 'true' And ('$(Registrar)' == 'static' Or '$(Registrar)' == 'managed-static' Or '$(Registrar)' == 'trimmable-static')" /> |
| var resolver = abr.PlatformAssembly.MainModule.AssemblyResolver; | ||
| var getAssembly = resolver.GetType ().GetMethod ("GetAssembly", new Type [] { typeof (string) })!; | ||
| foreach (var asm in createdAssemblies) { | ||
| var fn = Path.Combine (App.TypeMapOutputDirectory, asm.Name.Name + ".dll"); | ||
| if (!File.Exists (fn)) | ||
| continue; | ||
| var asmDef = getAssembly.Invoke (resolver, new object [] { fn }) as AssemblyDefinition; | ||
| Console.WriteLine ($"Loaded assembly: {asmDef?.FullName ?? "null!"} at {fn}"); | ||
| } |
There was a problem hiding this comment.
The reflection-based AssemblyResolver.GetAssembly invocation and the subsequent "Loaded assembly" Console.WriteLine look like leftover diagnostics. Besides the nullability concern, this adds overhead and log noise during linking without affecting output. Please remove this block or guard it behind a high verbosity/diagnostic switch.
|
|
||
| <Registrar>trimmable-static</Registrar> |
There was a problem hiding this comment.
This sets <Registrar>trimmable-static</Registrar> unconditionally for MySimpleApp. MySimpleApp is referenced by many dotnet build/unit tests, so this changes a broad set of test executions and may mask regressions in other registrar modes. Consider enabling trimmable-static via a dedicated TestVariation instead of hard-coding it in the shared project.
| <Registrar>trimmable-static</Registrar> |
| namespace Xamarin.Linker { | ||
| public class TrimmableRegistrarStep : ConfigurationAwareStep { | ||
| protected override string Name { get; } = "TrimmableRegistrar"; | ||
| protected override int ErrorCode { get; } = 2430; |
There was a problem hiding this comment.
The PR description still contains placeholder TODO text ("TODO: Update this description with more TODOs."). Please replace it with a real summary of the design/behavior changes (and any limitations/known issues) before merging so reviewers and future maintainers have the necessary context.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [CI Build #33e1906] Build passed (Build packages) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #33e1906] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #33e1906] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #33e1906] Test results 🔥Test results❌ Tests failed on VSTS: test results 3 tests crashed, 11 tests failed, 152 tests passed. Failures❌ cecil tests🔥 Failed catastrophically on VSTS: test results - cecil (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (iOS)🔥 Failed catastrophically on VSTS: test results - dotnettests_ios (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - dotnettests_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (tvOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (iOS)2 tests failed, 11 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (MacCatalyst)4 tests failed, 14 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (tvOS)2 tests failed, 11 tests passed.Failed tests
Html Report (VSDrops) Download ❌ windows tests1 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
TODO:
Fixes #23108.