Clean up SuperPMI collection scripts #74598
Conversation
The SuperPMI collection scripts were initially written to do PMI-based collections, and crossgen2 and benchmarks were added later. This change cleans up the scripts to only do what is required for a particular collection type. E.g., don't clone and build jitutils to get pmi.dll for crossgen2 collections, where it is not needed. Additional documentation is added and things are renamed to be more clear, if possible. Also, don't pass PMI-specific arguments to superpmi.py for crossgen2 collections (where they are ignored, but might be confusing to readers). In addition, the superpmi_collect_setup.py script gets more argument validation. This isn't terribly necessary since it's only called in one place in the CI scripts, but it is useful for documentation, and helps when you are calling the script manually as part of testing changes. I added a `-payload_directory` argument that specifies where the correlation and work item payloads should be placed, instead of assuming they should be in "well-known" directories in the source tree. Once again, this is useful for testing.
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThe SuperPMI collection scripts were initially written to do In addition, the superpmi_collect_setup.py script gets more argument validation.
|
|
A test collection was done to validate these changes: https://dev.azure.com/dnceng/internal/_build/results?buildId=1965888&view=results |
|
@kunalspathak @dotnet/jit-contrib PTAL |
kunalspathak
left a comment
kunalspathak
left a comment
There was a problem hiding this comment.
LGTM with nit comments.
|
|
||
| parser = argparse.ArgumentParser(description="description") | ||
|
|
||
| parser.add_argument("-source_directory", help="path to source directory") |
There was a problem hiding this comment.
In addition to introducing -payload_directory, did you just move these around or are there any real updates too that I am not spotting?
| # payload | ||
| pmiassemblies_directory = os.path.join(workitem_directory, "pmiAssembliesDirectory") | ||
| input_artifacts = os.path.join(pmiassemblies_directory, coreclr_args.collection_name) | ||
| input_artifacts = os.path.join(workitem_payload_directory, "collectAssembliesDirectory", coreclr_args.collection_name) |
There was a problem hiding this comment.
Consider setting variable for "collectAssembliesDirectory" in yml which can be used in python script as well as proj file.
| # Title : superpmi_collect_setup.py | ||
| # | ||
| # Notes: | ||
| # |
There was a problem hiding this comment.
@BruceForstall, can we delete run-pmi-diffs.py and other unused scripts in this directory? More than one scripts in this directory have stale references: 4-6 years old docker containers, Jenkins CI which is unused since 2019, downloading emulators from 2016 blobs which return 404-Not-Found etc.
There was a problem hiding this comment.
Makes sense. Yes, we should do that. (But not in this change)
|
This change was subsumed by #74961 |
4 participants
The SuperPMI collection scripts were initially written to do
PMI-based collections, and crossgen2 and benchmarks were added
later. This change cleans up the scripts to only do what is
required for a particular collection type. E.g., don't clone and
build jitutils to get pmi.dll for crossgen2 collections, where it
is not needed. Additional documentation is added and things are
renamed to be more clear, if possible. Also, don't pass PMI-specific
arguments to superpmi.py for crossgen2 collections (where they are
ignored, but might be confusing to readers).
In addition, the superpmi_collect_setup.py script gets more argument validation.
This isn't terribly necessary since it's only called in one place in the CI
scripts, but it is useful for documentation, and helps when you are calling
the script manually as part of testing changes. I added a
-payload_directoryargument that specifies where the correlation and work item payloads should
be placed, instead of assuming they should be in "well-known" directories
in the source tree. Once again, this is useful for testing.