-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Spmi replay pipeline #56871
Spmi replay pipeline #56871
Conversation
0443b0a
to
a09bac7
Compare
@dotnet/jit-contrib |
Can someone review this? Want to make sure that this goes in before we freeze the main. |
displayName: Upload JIT to Azure Storage | ||
env: | ||
CLRJIT_AZ_KEY: $(clrjit_key1) # secret key stored as variable in pipeline | ||
- ${{ if eq(parameters.uploadAsArtifacts, false) }}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- ${{ if eq(parameters.uploadAsArtifacts, false) }}: | |
- ${{ if not(parameters.uploadAsArtifacts) }}: |
a09bac7
to
c8dc88a
Compare
Hello @kunalspathak! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good to Me
@kunalspathak Love this work! Are you planning to move the pipeline from internal to public, so it can be auto-triggered or manually triggered on JIT PR's? Or is there some reason why it must remain on "internal" and run post-merge? |
We could definitely make it public but there is one scenario in which it doesn't work - If the PR changes the JITEE guid, then we don't get new collection for new guid because we haven't collected it. What should ideally happen in such cases is that collection pipeline should trigger the replay pipeline. But other than that, this can be moved to public (let me know if you plan on doing it). |
|
Currently, the partitions are hardcoded in runtime/src/coreclr/scripts/superpmi-replay.proj Lines 45 to 55 in 7994e59
I didn't see any assert yet. The pipeline failed once and reported when we didn't had .mch files to run on. |
Introduce
superpmi-replay
public pipeline that will be triggered after JIT changes. Here is how it works:windowx-x64
build to cross-compile following jit binaries:clrjit_win_x64_x64.dll
clrjit_win_arm64_x64.dll
clrjit_unix_x64_x64.dll
clrjit_unix_arm64_x64.dll
windowx-x86
build to cross-compile following jit binaries:clrjit_win_x86_x86.dll
clrjit_unix_arm_x86.dll
superpmi replay
for below environment variables:superpmi__arch.log
as an artifact. Currently the pipeline fails because of a bug in helix tracked by https://github.com/dotnet/core-eng/issues/13983.Changes:
--no_progress
in "superpmi download" so we do not see the progress logs during mch downloading.download-specific-artifacts.yml
.Since I have introduced superpmi-replay** files, I will submit a follow-up PR to rename existing superpmi* files that does the collection to superpmi-collect**.
Also, as a follow-up work, we would trigger this pipeline after
superpmi-collect
pipeline completes so the collection are fresh and we don't get "missing key errors".Fixes: #52392