diff --git a/eng/pipelines/coreclr/templates/run-superpmi-asmdiffs-job.yml b/eng/pipelines/coreclr/templates/run-superpmi-asmdiffs-job.yml index 46535d20e8680..6bf5a3e8d0191 100644 --- a/eng/pipelines/coreclr/templates/run-superpmi-asmdiffs-job.yml +++ b/eng/pipelines/coreclr/templates/run-superpmi-asmdiffs-job.yml @@ -111,6 +111,14 @@ jobs: targetFolder: '$(SpmiAsmdiffsLocation)' condition: always() + - task: CopyFiles@2 + displayName: Copying dasm files of all partitions + inputs: + sourceFolder: '$(HelixResultLocation)' + contents: '**/Asmdiffs_*.zip' + targetFolder: '$(SpmiAsmdiffsLocation)' + condition: always() + - script: $(PythonScript) $(Build.SourcesDirectory)/src/coreclr/scripts/superpmi_asmdiffs_summarize.py -diff_summary_dir $(SpmiAsmdiffsLocation) -arch $(archType) displayName: ${{ format('Summarize ({0})', parameters.archType) }} condition: always() diff --git a/src/coreclr/scripts/superpmi-asmdiffs.proj b/src/coreclr/scripts/superpmi-asmdiffs.proj index 170b306202224..aef782d769cb9 100644 --- a/src/coreclr/scripts/superpmi-asmdiffs.proj +++ b/src/coreclr/scripts/superpmi-asmdiffs.proj @@ -67,7 +67,7 @@ $(WorkItemCommand) -arch %(HelixWorkItem.Architecture) -platform %(HelixWorkItem.Platform) $(WorkItemTimeout) - superpmi_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_download_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_diff_summary_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).md + superpmi_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_download_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_diff_summary_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).md;Asmdiffs_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).zip diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index f59a00fb42582..86ab04a9b4d92 100755 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -318,6 +318,7 @@ asm_diff_parser.add_argument("-diff_jit_option", action="append", help="Option to pass to the diff JIT. Format is key=value, where key is the option name without leading COMPlus_...") asm_diff_parser.add_argument("-tag", help="Specify a word to add to the directory name where the asm diffs will be placed") asm_diff_parser.add_argument("-metrics", action="append", help="Metrics option to pass to jit-analyze. Can be specified multiple times, or pass comma-separated values.") +asm_diff_parser.add_argument("-retainOnlyTopFiles", action="store_true", help="Retain only top .dasm files with largest improvements or regressions and delete remaining files.") # subparser for upload upload_parser = subparsers.add_parser("upload", description=upload_description, parents=[core_root_parser, target_parser]) @@ -1628,6 +1629,8 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: summary_file_info = ( mch_file, md_summary_file ) all_md_summary_files.append(summary_file_info) command = [ jit_analyze_path, "--md", md_summary_file, "-r", "--base", base_asm_location, "--diff", diff_asm_location ] + if self.coreclr_args.retainOnlyTopFiles: + command += [ "--retainOnlyTopFiles" ] if self.coreclr_args.metrics: command += [ "--metrics", ",".join(self.coreclr_args.metrics) ] elif base_bytes is not None and diff_bytes is not None: @@ -3261,6 +3264,11 @@ def verify_replay_common_args(): lambda unused: True, "Unable to set metrics.") + coreclr_args.verify(args, + "retainOnlyTopFiles", + lambda unused: True, + "Unable to set retainOnlyTopFiles.") + process_base_jit_path_arg(coreclr_args) jit_in_product_location = False diff --git a/src/coreclr/scripts/superpmi_asmdiffs.py b/src/coreclr/scripts/superpmi_asmdiffs.py index aca3fe959f363..ffd1fd9424b2d 100644 --- a/src/coreclr/scripts/superpmi_asmdiffs.py +++ b/src/coreclr/scripts/superpmi_asmdiffs.py @@ -13,10 +13,10 @@ ################################################################################ import argparse -import os +from os import walk, path import shutil from coreclr_arguments import * -from jitutil import run_command +from jitutil import run_command, TempDir parser = argparse.ArgumentParser(description="description") @@ -66,6 +66,54 @@ def setup_args(args): return coreclr_args +def copy_dasm_files(spmi_location, upload_directory, tag_name): + """Copies .dasm files to a tempDirectory, zip it, and copy the compressed file to the upload directory. + + Args: + spmi_location (string): Location where .dasm files are present + upload_directory (string): Upload directory + tag_name (string): tag_name used in zip file name. + """ + + print("Copy .dasm files") + + # Create upload_directory + if not os.path.isdir(upload_directory): + os.makedirs(upload_directory) + + # Create temp directory to copy all issues to upload. We don't want to create a sub-folder + # inside issues_directory because that will also get included twice. + with TempDir() as prep_directory: + for file_path, dirs, files in walk(spmi_location, topdown=True): + # Credit: https://stackoverflow.com/a/19859907 + dirs[:] = [d for d in dirs] + for name in files: + if not name.lower().endswith(".dasm"): + continue + + dasm_src_file = path.join(file_path, name) + dasm_dst_file = dasm_src_file.replace(spmi_location, prep_directory) + dst_directory = path.dirname(dasm_dst_file) + if not os.path.exists(dst_directory): + os.makedirs(dst_directory) + try: + shutil.copy2(dasm_src_file, dasm_dst_file) + except PermissionError as pe_error: + print('Ignoring PermissionError: {0}'.format(pe_error)) + + # Zip compress the files we will upload + zip_path = os.path.join(prep_directory, "Asmdiffs_" + tag_name) + print("Creating archive: " + zip_path) + shutil.make_archive(zip_path, 'zip', prep_directory) + + zip_path += ".zip" + dst_zip_path = os.path.join(upload_directory, "Asmdiffs_" + tag_name + ".zip") + print("Copying {} to {}".format(zip_path, dst_zip_path)) + try: + shutil.copy2(zip_path, dst_zip_path) + except PermissionError as pe_error: + print('Ignoring PermissionError: {0}'.format(pe_error)) + def main(main_args): """ Run superpmi asmdiffs process on the Helix machines. @@ -156,7 +204,8 @@ def main(main_args): "-spmi_location", spmi_location, "-error_limit", "100", "-log_level", "debug", - "-log_file", log_file]) + "-log_file", log_file, + "-retainOnlyTopFiles"]) # If there are asm diffs, and jit-analyze ran, we'll get a diff_summary.md file in the spmi_location directory. # We make sure the file doesn't exist before we run diffs, so we don't need to worry about superpmi.py creating @@ -169,6 +218,8 @@ def main(main_args): shutil.copy2(overall_md_summary_file, overall_md_summary_file_target) except PermissionError as pe_error: print('Ignoring PermissionError: {0}'.format(pe_error)) + + copy_dasm_files(spmi_location, log_directory, "{}_{}".format(platform_name, arch_name)) else: # Write a basic summary file. Ideally, we should not generate a summary.md file. However, currently I'm seeing # errors where the Helix work item fails to upload this specified file if it doesn't exist. We should change the @@ -178,9 +229,6 @@ def main(main_args): No diffs found """) - # TODO: the superpmi.py asmdiffs command returns a failure code if there are MISSING data even if there are - # no asm diffs. We should probably only fail if there are actual failures (not MISSING or asm diffs). - if return_code != 0: print("Failure in {}".format(log_file)) return 1