From a60aab1817d80dc220702cc213a68f47a95caa61 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 6 Sep 2023 10:08:13 +0200 Subject: [PATCH] SPMI: Optimize collection download, increase diffs timeout (#91637) With the latest collections we are frequently hitting timeouts during linux asmdiffs runs. This PR applies two changes to improve the situation: - Optimize the download/extraction of collections by extracting directly to the target location. The Helix machines actually seem to spend many times longer to extract and copy the collections than they do downloading them. - Increase the timeout of superpmi-diffs by one hour. --- .../templates/run-superpmi-diffs-job.yml | 4 +-- .../coreclr/templates/superpmi-diffs-job.yml | 2 +- src/coreclr/scripts/jitutil.py | 32 +++++++++---------- src/coreclr/scripts/superpmi-diffs.proj | 2 +- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/eng/pipelines/coreclr/templates/run-superpmi-diffs-job.yml b/eng/pipelines/coreclr/templates/run-superpmi-diffs-job.yml index b987c25cb89a2..da61351842a58 100644 --- a/eng/pipelines/coreclr/templates/run-superpmi-diffs-job.yml +++ b/eng/pipelines/coreclr/templates/run-superpmi-diffs-job.yml @@ -11,7 +11,7 @@ parameters: osSubgroup: '' # optional -- operating system subgroup continueOnError: 'false' # optional -- determines whether to continue the build if the step errors dependsOn: '' # optional -- dependencies of the job - timeoutInMinutes: 120 # optional -- timeout for the job + timeoutInMinutes: 180 # optional -- timeout for the job enableTelemetry: false # optional -- enable for telemetry liveLibrariesBuildConfig: '' # optional -- live-live libraries configuration to use for the run helixQueues: '' # required -- Helix queues @@ -119,7 +119,7 @@ jobs: helixType: 'build/tests/' helixQueues: ${{ join(',', parameters.helixQueues) }} creator: dotnet-bot - WorkItemTimeout: 2:00 # 2 hours + WorkItemTimeout: 3:00 # 3 hours WorkItemDirectory: '$(WorkItemDirectory)' CorrelationPayloadDirectory: '$(CorrelationPayloadDirectory)' helixProjectArguments: '$(Build.SourcesDirectory)/src/coreclr/scripts/superpmi-diffs.proj' diff --git a/eng/pipelines/coreclr/templates/superpmi-diffs-job.yml b/eng/pipelines/coreclr/templates/superpmi-diffs-job.yml index aace201eb9661..4380be02352b7 100644 --- a/eng/pipelines/coreclr/templates/superpmi-diffs-job.yml +++ b/eng/pipelines/coreclr/templates/superpmi-diffs-job.yml @@ -5,7 +5,7 @@ parameters: osSubgroup: '' # optional -- operating system subgroup condition: true pool: '' - timeoutInMinutes: 180 # build timeout + timeoutInMinutes: 240 # build timeout variables: {} helixQueues: '' dependOnEvaluatePaths: false diff --git a/src/coreclr/scripts/jitutil.py b/src/coreclr/scripts/jitutil.py index 5a6b4ddc6c107..9ba23d15bb752 100644 --- a/src/coreclr/scripts/jitutil.py +++ b/src/coreclr/scripts/jitutil.py @@ -756,25 +756,23 @@ def download_files(paths, target_dir, verbose=True, fail_if_not_found=True, is_a logging.info("Uncompress %s", download_path) if item_path.lower().endswith(".zip"): - with zipfile.ZipFile(download_path, "r") as file_handle: - file_handle.extractall(temp_location) + with zipfile.ZipFile(download_path, "r") as zip: + zip.extractall(target_dir) + archive_names = zip.namelist() else: - with tarfile.open(download_path, "r") as file_handle: - file_handle.extractall(temp_location) - - # Copy everything that was extracted to the target directory. - copy_directory(temp_location, target_dir, verbose_copy=verbose, - match_func=lambda path: not path.endswith(".zip") and not path.endswith(".tar.gz")) - - # The caller wants to know where all the files ended up, so compute that. - for dirpath, _, files in os.walk(temp_location, topdown=True): - for file_name in files: - if not file_name.endswith(".zip") and not file_name.endswith(".tar.gz"): - full_file_path = os.path.join(dirpath, file_name) - target_path = full_file_path.replace(temp_location, target_dir) - local_paths.append(target_path) + with tarfile.open(download_path, "r") as tar: + tar.extractall(target_dir) + archive_names = tar.getnames() + + for archive_name in archive_names: + if archive_name.endswith("/"): + # Directory + continue + + target_path = os.path.join(target_dir, archive_name.replace("/", os.path.sep)) + local_paths.append(target_path) else: - # Not a zip file; download directory to target directory + # Not an archive download_path = os.path.join(target_dir, item_name) if is_item_url: ok = download_one_url(item_path, download_path, fail_if_not_found=fail_if_not_found, is_azure_storage=is_azure_storage, display_progress=display_progress) diff --git a/src/coreclr/scripts/superpmi-diffs.proj b/src/coreclr/scripts/superpmi-diffs.proj index 8b7aae5803de8..1d7b5fd0597f3 100644 --- a/src/coreclr/scripts/superpmi-diffs.proj +++ b/src/coreclr/scripts/superpmi-diffs.proj @@ -57,7 +57,7 @@ $(Python) $(ProductDirectory)/superpmi_diffs.py -type $(SuperPmiDiffType) -base_jit_directory $(ProductDirectory)/base -diff_jit_directory $(ProductDirectory)/diff $(SuperPmiBaseJitOptionsArg) $(SuperPmiDiffJitOptionsArg) -log_directory $(SuperpmiLogsLocation) - 2:00 + 3:00