Skip to content

[Xamarin.Android.Build.Tasks] Use class-parse.exe out of MSBuild process - #6551

Merged
jonpryor merged 1 commit into
mainfrom
out-of-proc-class-parse
Dec 14, 2021
Merged

[Xamarin.Android.Build.Tasks] Use class-parse.exe out of MSBuild process#6551
jonpryor merged 1 commit into
mainfrom
out-of-proc-class-parse

Conversation

@jpobst

@jpobstjpobst commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

Context: dotnet/java-interop#499

There are potentially some conflicts with using Xamarin.Android.Tools.Bytecode directly in the MSBuild process, as it now imports protobuf-net which is a commonly used library.

To ensure it isn't running in-process, switch our targets to shell out to class-parse[.exe/.dll].

Additionally add class-parse.dll, etc. to the .NET 6 install pack. We are already shipping class-parse.exe, etc. in the Classic installer even though it wasn't used, so no additional installer changes are needed there.

As expected, moving this out-of-process is slightly slower, but still acceptable.

Before
292 ms ClassParse 1 calls
After
425 ms ClassParse 1 calls

Additionally adds java-resolution-report.log to <FileWrites /> which was added in dotnet/java-interop#849.

@jpobst
jpobstforce-pushed the out-of-proc-class-parse branch 3 times, most recently from 89c9866 to a02297eCompareDecember 8, 2021 02:51
Comment threadbuild-tools/create-packs/Microsoft.Android.Sdk.proj
@jpobst
jpobst marked this pull request as ready for review December 9, 2021 16:35

@jonathanpeppersjonathanpeppers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you measure the build time before & after?

I think you could build a binding with -clp:performancesummary or -bl and just see how long this task takes.

classPath.SaveXmlDescription (output);
var cmd = GetCommandLineBuilder ();

var responseFile = Path.Combine (Path.GetDirectoryName (OutputFile), "class-parse.rsp");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If class-parse.exe succeeds, should we delete this file immediately? Just leave it on disk if there is a non-zero exit code?

Otherwise, should something delete it when you run the Clean target?

@jpobstjpobstDec 13, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good suggestions, added performance data and <FileWrites /> for response file.

@jpobst
jpobstforce-pushed the out-of-proc-class-parse branch from a02297e to 7fdad64CompareDecember 13, 2021 21:37

@jonathanpeppersjonathanpeppers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good if CI ends up green 👍

@jonpryor
jonpryor merged commit 014c1cb into mainDec 14, 2021
@jonpryor
jonpryor deleted the out-of-proc-class-parse branch December 14, 2021 16:36
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 24, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jpobst@jonathanpeppers@jonpryor