Skip to content

Conversation

@BruceForstall
Copy link
Contributor

@BruceForstall BruceForstall commented Sep 10, 2020

Update from Kunal:

Added runtime superpmi pipeline that does the following:

  • Builds CoreClr/Libraries/Core_Root
  • Runs SuperPMI setup script that:
    • Clones and builds jitutils to produce pmi.dll.
    • Prepare correlation payload directory. This is the folder that contains Core_Root, pmi.dll, superpmi scripts that is needed to run SPMI collection. This is a common directory that is sent to all the helix machines that will do the collection.
    • Scans the folder on which SPMI need to be done (in this case, it is just Core_Root, but in future will be TestArtifacts) and partition the .dlls/.exe in chunks of 50MB. These partition constitutes “payload” directory that is sent to individual helix machines to do the collection in parallel. For e.g. Core_Root folder is 150 MB (just managed .dll and .exe) and is divided in 3 partitions so we can do SPMI collection on 3 machines in parallel, each doing it on payload of 50 MB.
  • Sends payload/correlation directories to helix machine and performs the SPMI collection.
  • Once helix machines are done with collection, there will be multiple .mch files produced from each partition (libraries.0.mch, libraries.1.mch, libraries.2.mch in this case). We bring them back on azure machine and merge and de-duplicate all the .mch files into a single libraries.mch file.
  • Upload the merged libraries.mch file to azure blob storage.

I also had to update superpmi.py to make it Python3.5 compliant. I didn't remove the test run part but commented the portion of tests collection with TODO intentionally so when I have to enable it, I will know what to enable.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Sep 10, 2020
@runfoapp runfoapp bot mentioned this pull request Sep 23, 2020
@kunalspathak kunalspathak force-pushed the AddSuperpmiPipeline branch 2 times, most recently from bb642b7 to 383f0c5 Compare September 25, 2020 00:36
Note that shutil.copy2 is very slow specially on Linux as compared to rsync.
But I think we are ok to have slowness in return of simpler code.
- Tests run times out on Windows
- Some of the partition's collection failed for tests because reply was not clean
- Here are the details when libraries/tests were run:
  https://dev.azure.com/dnceng/internal/_build/results?buildId=832814&view=results
- Hence, disable tests run and just have libraries run for now.
- I didn't remove the test run part but commented the portion of tests collection with
  TODO intentionally so when I have to enable it, I will know what to enable.
@kunalspathak kunalspathak marked this pull request as ready for review September 28, 2020 18:58
@kunalspathak
Copy link
Contributor

@dotnet/jit-contrib

@kunalspathak
Copy link
Contributor

@safern - Thanks a lot for helping me through this. If you get a chance, please take a look at the overall pipeline structure.

Copy link
Contributor Author

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

Looks good.

(I can't approve this because it's technically "my" PR)

@kunalspathak
Copy link
Contributor

Looks good.

(I can't approve this because it's technically "my" PR)

Thanks Bruce for reviewing. I will incorporate the feedback and approve it! :)

Copy link
Contributor

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM

@kunalspathak kunalspathak merged commit 6b33e41 into dotnet:master Oct 1, 2020
<FileSeparatorChar>\</FileSeparatorChar>
</PropertyGroup>
<PropertyGroup Condition="'$(AGENT_OS)' != 'Windows_NT'">
<FileSeparatorChar>/</FileSeparatorChar>
Copy link
Member

Choose a reason for hiding this comment

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

You don't need this. You can use MSBuild::NormalizeDirectory or MSBuild::NormalizePath apis. We have a bunch of usages and examples in the runtime repo.

Copy link
Member

Choose a reason for hiding this comment

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

Note that this APIs need to be used with full paths, it doesn't work with relative paths.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you do need it... because you're using it to calculate the commands you're going to run on helix.

# Run superpmi collection in helix
- template: /eng/common/templates/steps/superpmi-send-to-helix.yml
parameters:
HelixSource: '$(HelixSourcePrefix)/$(Build.Repository.Name)/$(Build.SourceBranch)' # sources must start with pr/, official/, prodcon/, or agent/
Copy link
Member

Choose a reason for hiding this comment

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

I beliueve this is no longer needed, this is automatically set by the Helix SDK nowadays.

kunalspathak added a commit to kunalspathak/runtime that referenced this pull request Oct 5, 2020
In dotnet#42053, I forgot to push a change to enable the superpmi pipeline.
This change will trigger it every Sunday at 9:00 AM PST.
kunalspathak added a commit that referenced this pull request Oct 5, 2020
* enable superpmi collection pipeline

In #42053, I forgot to push a change to enable the superpmi pipeline.
This change will trigger it every Sunday at 9:00 AM PST.

* fix space changes
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@BruceForstall BruceForstall deleted the AddSuperpmiPipeline branch February 28, 2021 03:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants