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

Run template tests run in parallel to save 15min of run time #17672

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

Eilon
Copy link
Member

@Eilon Eilon commented Sep 26, 2023

It looks like maybe yes! I set a max of 2 threads parallel because that seems to get significant gains (15min faster!) and so far seems reliable.

Here are examples of build times from a few random builds without the change and the last one is with the change in this PR:

AzDO build report link Build platform: Windows (just tests) Build platform: macOS (just tests)
https://dev.azure.com/xamarin/public/_build/results?buildId=98121&view=results 1h 9m (52m) 1h 4m (58m)
https://dev.azure.com/xamarin/public/_build/results?buildId=98115&view=results 1h 14m (56m) 1h 3m (58m)
https://dev.azure.com/xamarin/public/_build/results?buildId=98112&view=results 1h 8m (51m) 1h 5m (59m)
https://dev.azure.com/xamarin/public/_build/results?buildId=98124&view=results 1h 8m (50m) 1h 11m (1h 6m)
https://dev.azure.com/xamarin/public/_build/results?buildId=98135&view=results
Attempt 1 from this PR - about 15min faster!!!
Attempt 1: 53m (36m) Attempt 1: 51m (46m)
https://dev.azure.com/xamarin/public/_build/results?buildId=98118&view=results
Attempt 2 from this PR - about 15min faster!!!
Attempt 2: 1h 0m (39m) Attempt 2: 49m (45m)

And more historical data available in build analytics (go to https://dev.azure.com/xamarin/public/_pipeline/analytics/duration?definitionId=57&contextType=build, filter Stages by Test Templates):
image
And the times range from 51min-59min over the last two weeks.

I propose that we take this change to significantly improve build times. We can later consider cranking up the parallelization level and see if it remains reliable. Or, if it seems to make things flaky, it's easy to set it to 1 thread.

This PR includes a few related changes:

  1. Actually make the tests run in parallel via an NUnit attribute.
  2. Guarantee that each test always gets its own unique folder. With the previous code there are hypothetical cases where tests could end up re-using folders (extremely unlikely, but possible).
  3. Ensure max project name is not exceeded. On WinUI the AppX uses the project name for some metadata that has a max length. By sheer luck we never exceeded it before (we were at the exact max length with project names like Buildmauilibnet60DebugTrue\id01039Buildmauilibnet60DebugTrue.csproj). In this change the project names are shortened if necessary to prevent issues. As part of (2) to make super-unique names, I caused names to just barely exceed the max length, so (3) became necessary.

@Eilon Eilon added do-not-merge Don't merge this PR area-templates Project templates, Item Templates for Blazor and MAUI labels Sep 26, 2023
@Eilon Eilon force-pushed the eilon/parallel-templates branch from 156560f to 419a2af Compare September 29, 2023 16:18
@@ -1,16 +1,19 @@
using Microsoft.Maui.IntegrationTests.Apple;

[assembly: LevelOfParallelism(2)]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key part of the change. Can adjust in the future higher/lower.

@Eilon Eilon changed the title Can template tests run in parallel? Run template tests run in parallel to save 15min of run time Sep 29, 2023
@Eilon Eilon marked this pull request as ready for review September 29, 2023 21:37
@Eilon Eilon requested a review from a team as a code owner September 29, 2023 21:37
@Eilon Eilon requested review from rmarinho and PureWeen September 29, 2023 21:37
Comment on lines +61 to +62
var projectName = DotnetInternal.GetProjectName(projectDir);
var projectFile = Path.Combine(projectDir, $"{projectName}.csproj");
Copy link
Member Author

Choose a reason for hiding this comment

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

This change and other projectName stuff is repeated for nearly every test: it's what ensures that the lengthy project names used throughout the tests are not too long for WinUI AppX (see PR description for more info).

@Eilon Eilon removed the do-not-merge Don't merge this PR label Oct 13, 2023
@Eilon Eilon requested a review from mattleibow October 13, 2023 23:21
@Eilon
Copy link
Member Author

Eilon commented Oct 13, 2023

@ruiminhu / @PureWeen / @mattleibow - any thoughts on getting this in? I see this as potentially improving our engineering lives by not only getting test results quite a bit faster, but potentially shorter queue times as well.

@rmarinho rmarinho merged commit 1719011 into main Oct 16, 2023
@rmarinho rmarinho deleted the eilon/parallel-templates branch October 16, 2023 17:20
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2023
@samhouts samhouts added the fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-templates Project templates, Item Templates for Blazor and MAUI fixed-in-8.0.6 Look for this fix in 8.0.6 SR1!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants