Upload dasm files as artifacts for "asmdiffs pipeline"

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()

src/coreclr/scripts/superpmi-asmdiffs.proj

@@ -67,7 +67,7 @@
     <HelixWorkItem Include="@(SPMI_Partition)">
       <Command>$(WorkItemCommand) -arch %(HelixWorkItem.Architecture) -platform %(HelixWorkItem.Platform)</Command>
       <Timeout>$(WorkItemTimeout)</Timeout>
-      <DownloadFilesFromResults>superpmi_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_download_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).log;superpmi_diff_summary_%(HelixWorkItem.Platform)_%(HelixWorkItem.Architecture).md</DownloadFilesFromResults>
+      <DownloadFilesFromResults>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</DownloadFilesFromResults>
     </HelixWorkItem>
   </ItemGroup>
   </Project>

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

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