Skip to content

Pipeline Performance Optimization: Make Static Analysis Run Parallelly with Build #5364

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

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

MuyuanMS
Copy link
Contributor

@MuyuanMS MuyuanMS commented Apr 25, 2025

This PR shortens build time by around 50 minutes for foundation repo, while only increase absolute build time by 25 minutes.

Below are running results for Foundation PR Build:

  • Before Change

    • Total Build Time: 2h 26m
    • Absolute Build Time (Sum of All jobs): 582 min
      image
  • After Change

    • Total Build Time: 1h 27m (-40%)
    • Absolute Build Time (Sum of All jobs): 607 min (+4.3%)
      image

Currently largest bottleneck building foundation is static analysis, which can be run independently but currently blocks dependent stage from running. After this change, now during build, two build stage will start in parallel - one without static analysis to allow dependent steps like Test or Pack to run, the other with static analysis


A microsoft employee must use /azp run to validate using the pipelines below.

WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.

For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Haonan Tang (from Dev Box) and others added 2 commits April 25, 2025 16:56
@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines failed to run 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS MuyuanMS changed the base branch from main to user/haonanttt/testStageSeparation April 28, 2025 05:33
@MuyuanMS
Copy link
Contributor Author

/azp run

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 2, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 2, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 4, 2025

This is great work. I think there is one thing we need to address, though. The existing way to avoid spending too much time on PREfast is to simply run PREfast only for x64fre. Please see this code in WindowsAppSDK-BuildMRT-Steps.yml and WindowsAppSDK-BuildFoundation-Steps.yml:

  • task: SDLNativeRules@3
    ...
    condition: and(succeeded(), eq(variables['buildPlatform'], 'x64'), eq(variables['buildConfiguration'], 'Release'))

Therefore, the Build_x86_StaticAnalysis stage is basically doing the same work as the Build_x86 stage, and that explains why they take similar time to finish. The same story applies to the Build_arm64_StaticAnalysis stage and the Build_arm64 stage. In contrast, the Build_arm64_StaticAnalysis stage takes much longer to run than Build_arm64 due to running PREfast, as expected.

I see 2 basic options: 1) continue to only run PREfast on x64fre only, that means we skip the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages, which hopefully will allow the pipeline to finish even sooner, 2) adjust the code cited above to enable running PREfast for x86 and arm64 in the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages. This is expected to increase the "total new workload" of the pipeline and is thus likely to reduce the performance gain. IMHO, #1 is a legitimate choice because it does not regress today's pipeline behavior. But if we prefer to data to support the decision, we can also first prototype #2 to confirm that running PREfast only for x64 only is "good enough" (we initially did verify our PREfast results are basically the same for x86, x64, and arm64, but that was a couple of years ago, things might have changed, probably worth another check). If the PREfast results are good, we should feel more comfortable going for option #1. Just my 2 cents. Hence, I expect this PR to take more changes. If so, I suggest rebasing this PR on the branch with the merged PR Performance Optimization for Foundation Build Pipeline: Break down build and test stage per platform by haonanttt · Pull Request #5363 · microsoft/WindowsAppSDK in place, to simplify this PR. Thank you very much!

This is great work. I think there is one thing we need to address, though. The existing way to avoid spending too much time on PREfast is to simply run PREfast only for x64fre. Please see this code in WindowsAppSDK-BuildMRT-Steps.yml and WindowsAppSDK-BuildFoundation-Steps.yml:

  • task: SDLNativeRules@3
    ...
    condition: and(succeeded(), eq(variables['buildPlatform'], 'x64'), eq(variables['buildConfiguration'], 'Release'))

Therefore, the Build_x86_StaticAnalysis stage is basically doing the same work as the Build_x86 stage, and that explains why they take similar time to finish. The same story applies to the Build_arm64_StaticAnalysis stage and the Build_arm64 stage. In contrast, the Build_arm64_StaticAnalysis stage takes much longer to run than Build_arm64 due to running PREfast, as expected.

I see 2 basic options: 1) continue to only run PREfast on x64fre only, that means we skip the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages, which hopefully will allow the pipeline to finish even sooner, 2) adjust the code cited above to enable running PREfast for x86 and arm64 in the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages. This is expected to increase the "total new workload" of the pipeline and is thus likely to reduce the performance gain. IMHO, #1 is a legitimate choice because it does not regress today's pipeline behavior. But if we prefer to data to support the decision, we can also first prototype #2 to confirm that running PREfast only for x64 only is "good enough" (we initially did verify our PREfast results are basically the same for x86, x64, and arm64, but that was a couple of years ago, things might have changed, probably worth another check). If the PREfast results are good, we should feel more comfortable going for option #1. Just my 2 cents. Hence, I expect this PR to take more changes. If so, I suggest rebasing this PR on the branch with the merged PR Performance Optimization for Foundation Build Pipeline: Break down build and test stage per platform by haonanttt · Pull Request #5363 · microsoft/WindowsAppSDK in place, to simplify this PR. Thank you very much!

Currently I tried to go with option 2 (Results: https://dev.azure.com/microsoft/ProjectReunion/_build/results?buildId=121725042&view=results), given that PREfast has been moved to a separate stage and they can run parallelly if resource allows. If we agree on running prefast for all three archs, I'll proceed to make the code changes in building MRT steps as well. Do you know how I can confirm whether PREfast results are the same for x86, x64, and arm64? (Just by build log, or do I actually have to manually make code changes to fail some PREfast checks and compare results?)

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

I see 2 basic options: 1) continue to only run PREfast on x64fre only, that means we skip the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages, which hopefully will allow the pipeline to finish even sooner, 2) adjust the code cited above to enable running PREfast for x86 and arm64 in the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages.

@alexlamtest I have updated current PR with option 1. Below are some of the results while making the decision:

@MuyuanMS
Copy link
Contributor Author

MuyuanMS commented May 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yeelam-gordon
Copy link

I see 2 basic options: 1) continue to only run PREfast on x64fre only, that means we skip the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages, which hopefully will allow the pipeline to finish even sooner, 2) adjust the code cited above to enable running PREfast for x86 and arm64 in the Build_x86_StaticAnalysis and Build_arm64_StaticAnalysis stages.

@alexlamtest I have updated current PR with option 1. Below are some of the results while making the decision:

@alexlamtest, I will suggest let Muyuan to checkin the one only improve performance first, which the result is promising and without increase much build agent cost (i.e. 4.3% only)
And if we believe it is necessary to run statistic analysis against all the build architect, it should be an easy change later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants