[wasm] Standup EnableAgressiveTrimming for library tests #46367
Conversation
…me into mdhwang/trim_library_tests
…me into mdhwang/trim_library_tests
…me into mdhwang/trim_library_tests
version copied as part of trimming publish
…me into mdhwang/trim_library_tests
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
…ker" as MainAssembly is back to $(PublishDir)\WasmTestRunner.dll This reverts commit 79dc240.
| <ManagedAssemblyToLink Include="$(OutDir)\*xunit*"> | ||
| <IsTrimmable>true</IsTrimmable> | ||
| <TrimMode>link</TrimMode> | ||
| </ManagedAssemblyToLink> | ||
| <ManagedAssemblyToLink Condition="('%(ManagedAssemblyToLink.FileName).dll' != '$(MSBuildProjectName).dll')" > | ||
| <TrimMode>link</TrimMode> | ||
| </ManagedAssemblyToLink> | ||
| <ManagedAssemblyToLink Condition="('%(ManagedAssemblyToLink.FileName).dll' == '$(AssemblyName).dll')" > | ||
| <TrimMode>copy</TrimMode> | ||
| </ManagedAssemblyToLink> |
There was a problem hiding this comment.
Looking at https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L142-L144 , I think instead of this, we should have a target that adds to @(ResolvedFileToPublish), and sets the TrimMode metadata as needed.
This will get processed by https://github.com/dotnet/sdk/blob/master/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L235-L237 and subsequently by PrepareForILLink target.
- IIUC, you just need to add the xunit files, if they aren't there already, and set
TrimMode=linkfor them. - And make sure that the test assembly doesn't have
IsTrimmable, orTrimModeset. this should handle the rest for you.
There was a problem hiding this comment.
Using ResolvedFileToPublish makes sense if we can hook it early enough. Though I'd keep copy setting to be explicit and maybe add a comment that we want the tested assembly to be always copied only.
There was a problem hiding this comment.
Will address in a follow-up PR
| <TrimmerRootAssembly Condition="'$(TargetOS)' == 'Android'" Include="AndroidTestRunner"/> | ||
| <TrimmerRootAssembly Condition="'$(TargetOS)' == 'Browser'" Include="WasmTestRunner"/> | ||
| <TrimmerRootAssembly Condition="'$(TargetOS)' == 'iOS'" Include="AppleTestRunner"/> | ||
| <TrimmerRootAssembly Include="$(AssemblyName)"/> |
There was a problem hiding this comment.
These shouldn't be needed, I think, since the PrepareForILLink target will set these up for us.
There was a problem hiding this comment.
These are different settings. I think PrepareForILLink will set only <TrimmerRootAssembly Include="$(AssemblyName)"/>
There was a problem hiding this comment.
I was thinking that they might get picked up by
runtime/eng/testing/tests.mobile.targets
Lines 222 to 226 in 3262912
@(ResolvedFileToPublish)
There was a problem hiding this comment.
I just tried removing L181 <TrimmerRootAssembly Include="$(AssemblyName)"/> and started seeing
fail: 10:28:48.0231240 System.AggregateException: AggregateException_ctor_DefaultMessage ()
fail: 10:28:48.0232360 ---> System.IO.FileNotFoundException:
fail: 10:28:48.0232420 IO_FileName_Name, System.Buffers.Tests
fail: 10:28:48.0232440 at System.Reflection.Assembly.Load(AssemblyName assemblyRef, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext)
fail: 10:28:48.0232480 at System.Reflection.Assembly.Load(AssemblyName assemblyRef)
fail: 10:28:48.0232610 at Xunit.Sdk.ReflectionAssemblyInfo..ctor(String assemblyFileName)
fail: 10:28:48.0232650 at System.Reflection.RuntimeConstructorInfo.InternalInvoke(Object obj, Object[] parameters, Boolean wrapExceptions)
fail: 10:28:48.0232660 --- End of stack trace from previous location ---
for ./dotnet.sh build /t:Test src/libraries/System.Buffers/tests/ /p:TargetOS=Browser /p:TargetArchitecture=wasm /p:Configuration=Debug /p:EnableAggressiveTrimming=true
There was a problem hiding this comment.
Will address in a follow-up PR
ghost
locked as resolved and limited conversation to collaborators
This PR looks to enable trimming of the Browser wasm library tests to reduce the managed library size as part of #45126.
An important step to reducing the size is to enable
PublishTrimmedin the library tests and trim more aggressively by removing unused members from types via<TrimMode>link</TrimMode>.The linker from the nuget package is imported to override the .NET SDK version of the linker that the mobile library tests use because the nuget package version includes fixes that have not made it to the .NET SDK version.
For example,
System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')was an error that was resolved with the import override.The
ConfigureTrimmingtarget had been added to force<TrimMode>link</TrimMode>of all assemblies except for the test assembly, which should be TrimMode copy.*xunitfiles were still not being linked out with<TrimMode>link</TrimMode>and resulted in numerous other assemblies being included, so they are linked out by manually adding the*xunitfiles toManagedAssemblyToLink. Several assembly methods could not be tracked by the linker itself, so a linker description for xunit code had been added ineng/testing/ILLink.Descriptor.xunit.xml.As a result of these addition, the library size for a test suite such as
System.Buffers.Testsis reduced from 30MB to ~3MB.The changes lie behind an
EnableAggressiveTrimmingflag so that CI will not yet run the tests in trimming mode.