Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mono] Add the capability of trimming IL code of individual methods #86722

Merged
merged 27 commits into from
Jun 16, 2023

Conversation

fanyang-mono
Copy link
Member

@fanyang-mono fanyang-mono commented May 24, 2023

Contributes to #44855

@jonathanpeppers After this is merged. It should be adoptable by Android. I have updated the HelloWorld sample as an example to showcase how to use this feature.

Usage of this feature:

  1. Enable AOT to log compiled methods. This could be achieved by setting CollectCompiledMethods and CompiledMethodsOutputDirectory for MonoAOTCompiler
  2. Run ILStrip after MonoAOTCompiler
  3. Overwrite the assemblies with the trimmed ones.

For more details, please refer to the HelloWorld sample app change included in this PR.

Comment on lines 47 to 49
int allowedParallelism = DisableParallelStripping ? 1 : Math.Min(Assemblies.Length, Environment.ProcessorCount);
if (BuildEngine is IBuildEngine9 be9)
allowedParallelism = be9.RequestCores(allowedParallelism);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int allowedParallelism = DisableParallelStripping ? 1 : Math.Min(Assemblies.Length, Environment.ProcessorCount);
if (BuildEngine is IBuildEngine9 be9)
allowedParallelism = be9.RequestCores(allowedParallelism);
int allowedParallelism = DisableParallelStripping ? 1 : BuildEngine9.RequestCores(Math.Min(Assemblies.Length, Environment.ProcessorCount));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hit an error when applying your change

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.

Comment on lines 77 to 79
int allowedParallelism = DisableParallelStripping ? 1 : Math.Min(MethodTokenFiles.Length, Environment.ProcessorCount);
if (BuildEngine is IBuildEngine9 be9)
allowedParallelism = be9.RequestCores(allowedParallelism);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int allowedParallelism = DisableParallelStripping ? 1 : Math.Min(MethodTokenFiles.Length, Environment.ProcessorCount);
if (BuildEngine is IBuildEngine9 be9)
allowedParallelism = be9.RequestCores(allowedParallelism);
int allowedParallelism = DisableParallelStripping ? 1 : BuildEngine9.RequestCores(Math.Min(Assemblies.Length, Environment.ProcessorCount));

@fanyang-mono fanyang-mono requested a review from radical as a code owner May 25, 2023 14:57
@fanyang-mono fanyang-mono changed the title [WIP][Mono] Add the capability of trimming IL code of individual methods [Mono] Add the capability of trimming IL code of individual methods May 26, 2023
@fanyang-mono fanyang-mono requested a review from marek-safar as a code owner May 26, 2023 21:38
@ivanpovazan
Copy link
Member

It would be nice to include a specification/documentation of how this feature is used/enabled (possibly with an example of MethodTokenFile). Might be enough to just include it in the PR description.

fanyang-mono and others added 7 commits June 5, 2023 17:55
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Ankit Jain <radical@gmail.com>
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Outdated Show resolved Hide resolved
src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs Show resolved Hide resolved
@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jun 12, 2023
Co-authored-by: Ankit Jain <radical@gmail.com>
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Jun 13, 2023
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍

@fanyang-mono
Copy link
Member Author

CI failures are not related to this PR.

@fanyang-mono fanyang-mono merged commit 9fc19a1 into dotnet:main Jun 16, 2023
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request Jul 6, 2023
Context: dotnet/runtime#86722

This adds an `<ILStrip/>` step after the `<MonoAOTCompiler/>` task.

This trims away IL of AOT-compiled methods. This is WIP.

Builds work currently, but the app crashes at runtime with:

    07-06 16:57:07.413  8865  8865 E companyname.foo: * Assertion at /__w/1/s/src/mono/mono/mini/mini-trampolines.c:1416, condition `invoke' not met
@ghost ghost locked as resolved and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants