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

Add VMR support for Windows-x64 PGO Instrumented leg #18672

Merged
merged 16 commits into from
Feb 23, 2024

Conversation

jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented Feb 14, 2024

Fixes dotnet/source-build#3893

Enables forwarding /p:PgoInstrument=true through the pipeline to the repos that need it (runtime, installer).

Create a generalized "reference-only" concept for the repo and a "limited-shipping tall stack" concept that utilizes the reference-only concept to say "build these repos, but all of these assets are only used for building later assets, they're not shipped". Also apply the "reference-only" concept to SBRP to remove custom SBRP build logic.

This depends on #18632 (as we need to build dotnet/installer based on the PGO assets to get a PGO-instrumented SDK)

…ited-shipping tall stack" concept that utilizes the reference-only concept to say "build these repos, but all of these assets are only used for building later assets, they're not shipped"
@mmitche
Copy link
Member

mmitche commented Feb 14, 2024

Create a generalized "reference-only" concept for the repo and a "limited-shipping tall stack" concept that utilizes the reference-only concept to say "build these repos, but all of these assets are only used for building later assets, they're not shipped". Also apply the "reference-only" concept to SBRP to remove custom SBRP build logic.

I'm a little confused by this. I definitely see where this makes sense for SBRP, but why apply the concept to PGO builds? We don't really treat these as special today.

@jkoritzinsky
Copy link
Member Author

Today we re-use assets from non-PGO legs in every repo other than dotnet/runtime and dotnet/installer in the PGO legs. I want to make sure that we don't get into a situation where we accidentally ship assets from the PGO legs that we don't want to ship, especially since we end up having to re-build the full stack here. Additionally, if we're building the PGO-instrumented runtime here, we don't even need to ship that artifact as it's only used by dotnet/installer to build the PGO-instrumented SDK.

Basically, I want to make sure that the only artifact we end up with in artifacts/assets is the PGO-instrumented SDK (and no packages or anything else in artifacts/packages).

@mmitche
Copy link
Member

mmitche commented Feb 14, 2024

So we don't rename the PGO assets from runtime?

@jkoritzinsky
Copy link
Member Author

The system today works as follows:

  • The non-PGO-instrumented job in dotnet/runtime ships all the usual assets
  • The PGO-instrumented job in dotnet/runtime only ships the instrumented runtime zip/tarball (with a separate name), nothing else.
  • Every repo other than installer consumes the non-PGO-instrumented assets
  • dotnet/installer's non-PGO job consumes the regular assets.
  • dotnet/installer's PGO job consumes the regular assets from other repos and the PGO assets from the dotnet/runtime repo to produce the pgo-instrumented SDK zip/tarball.

The VMR won't have the non-PGO-instrumented assets available, so my plan was to do something like the following:

  • The non-PGO-instrumented VMR leg builds as usual
  • The PGO-instrumented VMR job builds an instrumented dotnet/runtime with all assets (which will need a little more work to enable). All other repos also build. dotnet/installer consumes these assets to produce the instrumented SDK zip/tarball. All other assets will be tucked away somewhere else (the reference-artifacts feature) so we can use the regular publishing logic for these legs.

@mmitche
Copy link
Member

mmitche commented Feb 14, 2024

The PGO-instrumented VMR job builds an instrumented dotnet/runtime with all assets (which will need a little more work to enable). All other repos also build. dotnet/installer consumes these assets to produce the instrumented SDK zip/tarball. All other assets will be tucked away somewhere else (the reference-artifacts feature) so we can use the regular publishing logic for these legs.

I think we have this problem generally, and so I wouldn't treat them specially for PGO. For instance, you're going to build System.CommandLine 70odd times and make it available to downstream repos. Only one of these "counts" for actually shipping this out the door. This will either be an explicit choice (repos identify which assets go to customers, and only set that as true for specific assets on specific verticals), or an explicit priority-based list. For instance, We start with Windows-x64, and it ships all assets, being the leg that produced rid-agnostic packages. Any non-unique asset produced in another vertical gets ignored. Any unique assets (e.g. your PGO zips) will get selected for the final drop because they were only produced in the PGO leg. Where we have conflicts (X is not produced in Windows x64 but is produced in PGO x86 and non PGO-x86), we error out and force the infra to declare a winner.

@jkoritzinsky
Copy link
Member Author

I'll remove the logic to dump the artifacts elsewhere then.

@jkoritzinsky
Copy link
Member Author

Once we move to the VMR being the only official build of the product, then we can simplify this more in the runtime repo and remove all of the artifact renaming there (as the consumption to build the SDK will be entirely in-pipeline).

@jkoritzinsky
Copy link
Member Author

/azp run installer-unified-build-full

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkoritzinsky
Copy link
Member Author

@mmitche if you want for #18557, we can merge this now and I'll validate that the PGO lanes are hooked up in #18632 once that's green.

@mthalman
Copy link
Member

Can we please wait until #18557 is merged? I've been trying to get that in for 2 weeks now and have had to wrestle with merge conflicts several times. This one will be more conflicts.

@ViktorHofer ViktorHofer enabled auto-merge (squash) February 23, 2024 15:13
@ViktorHofer ViktorHofer merged commit 4b127e3 into dotnet:main Feb 23, 2024
22 checks passed
@jkoritzinsky jkoritzinsky deleted the vmr-pgoinstrument branch February 23, 2024 16:18
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.

Implement Vertical Build for Windows x64 PGO Instrumented
5 participants