Skip to content

Run crossgen in parallel in crossgen_comparison.py - #33175

Merged
jashook merged 15 commits into
dotnet:masterfrom
jashook:cross-crossgen-parallel
Mar 26, 2020
Merged

Run crossgen in parallel in crossgen_comparison.py#33175
jashook merged 15 commits into
dotnet:masterfrom
jashook:cross-crossgen-parallel

Conversation

@jashook

Copy link
Copy Markdown
Contributor

This will hopefully reduce the runtime of crossgen_compairision.py and address #33172

@Dotnet-GitSync-BotDotnet-GitSync-Bot added the area-Infrastructure-coreclr Only use for closed issues label Mar 4, 2020
Comment threadsrc/coreclr/tests/scripts/crossgen_comparison.py Outdated
Comment threadsrc/coreclr/tests/scripts/crossgen_comparison.py Outdated
@jashook
jashookforce-pushed the cross-crossgen-parallel branch from 3c9c5c3 to 0752950CompareMarch 23, 2020 19:07
@jashook
jashook requested a review from echesakovMarch 25, 2020 21:24
@jashook

Copy link
Copy Markdown
ContributorAuthor

I believe this is finally ready for review @echesakovMSFT @dotnet/runtime-infrastructure ptal

@jashook

Copy link
Copy Markdown
ContributorAuthor

Will post runtime improvements once the run finishes

@echesakovechesakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes look good to me. I wonder what speedup we will get
Thank you @jashook !

platform: Linux_arm
container:
image: ubuntu-16.04-cross-14.04-23cacb0-20200121150126
image: ubuntu-18.04-cross-arm-16.04-09ec757-20200324125113

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.

Why did we need to upgrade the image here?

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.

The script needs python3.6+

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.

Its also generally not needed for us to target 14.04 anymore as 14.04 is eol

Comment threadeng/pipelines/coreclr/templates/crossgen-comparison-job.yml Outdated
inputs:
scriptSource: 'filePath'
scriptPath: $(coreClrRepoRoot)/tests/scripts/crossgen_comparison.py
pythonInterpreter: /usr/bin/python3

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.

Do we need to add python3 to our linux requirements markdown file?

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.

We should be one python3, seeing that python2 is eol. We have not exactly mandated it yet.

@jashook

Copy link
Copy Markdown
ContributorAuthor

Time in seconds

Branchx64-arm crossgen (build machine)arm crossgen (on device)
master189.140369448.044597
change87.963722133.222282

@jashook

Copy link
Copy Markdown
ContributorAuthor

This will leave our cross-crossgen job with ~3 minutes of helix setup time (download, zip and docker) and ~2 minutes of execution time. Total time is reduced ~40% with this change.

@trylek

Copy link
Copy Markdown
Member

Very nice. That matches the ratio between Crossgen / Crossgen2 framework compilation times in "build-test generatelayoutonly" (where there's a straight linear loop over the assemblies) and SuperIlc (which uses parallelization much like you do, just written in C#), something like 3:1, almost reaching the 4:1 ideal speedup on my 4-core box.

@trylektrylek 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 great, thanks for pulling off such a dramatic perf improvement!

@jashookjashook linked an issue Mar 25, 2020 that may be closed by this pull request
@jashook
jashook merged commit 9656059 into dotnet:masterMar 26, 2020
@jashook
jashook deleted the cross-crossgen-parallel branch March 26, 2020 00:29
@ghostghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-coreclrOnly use for closed issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve cross-crossgen run time

6 participants

@jashook@trylek@lpereira@jaredpar@echesakov@Dotnet-GitSync-Bot