Properly load repo/commit information in CI

Author: ianayl

devops/actions/run-tests/benchmark_v2/action.yml

@@ -110,6 +110,19 @@ runs:
       pip install --user --break-system-packages -r ./devops/scripts/benchmarks/requirements.txt
       echo "-----"
 
+      # clang builds have git repo / commit hashes in their --version output,
+      # same goes for dpcpp. Obtain git repo / commit hash info this way:
+
+      # First line of --version is formatted 'clang version ... (<repo> <commit>)'
+      # thus we parse for (<repo> <commit>):
+      sycl_git_info="$(clang++ --version | head -n 1 | grep -oE '\([^ ]+ [a-f0-9]+\)$')" | tr -d '()'
+      if [ -z "$sycl_git_info" ]; then
+        echo "Error: Unable to deduce SYCL build source repo/commit: Are you sure dpcpp variable is in PATH?"
+        exit 1
+      fi
+      sycl_git_repo="$(printf "$sycl_git_info" | cut -d' ' -f1)"
+      sycl_git_commit="$(printf "$sycl_git_info" | cut -d' ' -f2)"
+
       case "$ONEAPI_DEVICE_SELECTOR" in
         level_zero:*) SAVE_SUFFIX="L0" ;;
         level_zero_v2:*) SAVE_SUFFIX="L0v2" ;;
@@ -128,10 +141,13 @@ runs:
         --results-dir "./llvm-ci-perf-results/" \
         --output-dir "./llvm-ci-perf-results/" \
         --preset "$PRESET" \
-        --timestamp-override "$SAVE_TIMESTAMP"
+        # CI options:
+        --timestamp-override "$SAVE_TIMESTAMP" \
+        --sycl-github-repo "$sycl_git_repo" \ 
+        --sycl-commit "$sycl_git_commit"
       echo "-----"
       python3 ./devops/scripts/benchmarks/compare.py to_hist \
-        --name Baseline_PVC_L0 \
+        --name "$SAVE_NAME" \
         --compare-file "./llvm-ci-perf-results/results/${SAVE_NAME}_${SAVE_TIMESTAMP}.json" \
         --results-dir "./llvm-ci-perf-results/results/"
 

devops/scripts/benchmarks/history.py

@@ -58,45 +58,58 @@ def extract_timestamp(file_path: Path) -> str:
         self.runs = benchmark_runs
 
     def create_run(self, name: str, results: list[Result]) -> BenchmarkRun:
-        try:
-            script_dir = os.path.dirname(os.path.abspath(__file__))
-            result = run("git rev-parse --short HEAD", cwd=script_dir)
-            git_hash = result.stdout.decode().strip()
-
-            # Get the GitHub repo URL from git remote
-            remote_result = run("git remote get-url origin", cwd=script_dir)
-            remote_url = remote_result.stdout.decode().strip()
-
-            # Convert SSH or HTTPS URL to owner/repo format
-            if remote_url.startswith("[email protected]:"):
-                # SSH format: [email protected]:owner/repo.git
-                github_repo = remote_url.split("[email protected]:")[1].rstrip(".git")
-            elif remote_url.startswith("https://github.com/"):
-                # HTTPS format: https://github.com/owner/repo.git
-                github_repo = remote_url.split("https://github.com/")[1].rstrip(".git")
-            else:
+
+        def git_info_from_path(path: Path) -> (str, str):
+            """
+            Derives git repo, commit information from git repo located in path.
+
+            Returns:
+                (str, str): git_hash, github_repo
+            """
+            try:
+                result = run("git rev-parse --short HEAD", cwd=path)
+                git_hash = result.stdout.decode().strip()
+
+                # Get the GitHub repo URL from git remote
+                remote_result = run("git remote get-url origin", cwd=path)
+                remote_url = remote_result.stdout.decode().strip()
+
+                # Convert SSH or HTTPS URL to owner/repo format
+                if remote_url.startswith("[email protected]:"):
+                    # SSH format: [email protected]:owner/repo.git
+                    github_repo = remote_url.split("[email protected]:")[1].rstrip(".git")
+                elif remote_url.startswith("https://github.com/"):
+                    # HTTPS format: https://github.com/owner/repo.git
+                    github_repo = remote_url.split("https://github.com/")[1].rstrip(".git")
+                else:
+                    github_repo = None
+
+            except:
+                git_hash = "unknown"
                 github_repo = None
+            
+            return git_hash, github_repo
 
-        except:
-            git_hash = "unknown"
-            github_repo = None
+        if options.sycl_commit is None or options.sycl_github_repo is None:
+            git_hash, github_repo = git_info_from_path(os.path.dirname(os.path.abspath(__file__)))
+        else:
+            git_hash, github_repo = options.sycl_commit, options.sycl_github_repo
 
         # Check if RUNNER_NAME environment variable has been declared.
         #
-        # RUNNER_NAME is always present in github runner environments. Because
-        # github runners obfusicate hostnames, using socket.gethostname()
-        # produces different hostnames when ran on the same machine multiple
-        # times. Thus, we rely on the RUNNER_NAME variable when running on
-        # github runners.
+        # Github runners obfusicate hostnames, thus running socket.gethostname()
+        # twice produces two different hostnames. Since github runners always
+        # define a RUNNER_NAME variable, use RUNNER_NAME instead if it exists:
         hostname = os.getenv("RUNNER_NAME")
         if hostname is None:
             hostname = socket.gethostname()
-        elif not Validate.runner_name(hostname):
-            # However, nothing stops github runner env variables (including
-            # RUNNER_NAME) from being modified by external actors. Ensure
-            # RUNNER_NAME contains nothing malicious:
+        else:
+            # Ensure RUNNER_NAME has not been tampered with:
             # TODO is this overkill?
-            raise ValueError("Illegal characters found in specified RUNNER_NAME.")
+            Validate.runner_name(
+                hostname,
+                throw=ValueError("Illegal characters found in specified RUNNER_NAME.")
+            )
 
         return BenchmarkRun(
             name=name,

devops/scripts/benchmarks/main.py

@@ -476,29 +476,52 @@ def validate_and_parse_env_args(env_args):
         help="Benchmark preset to run",
         default=options.preset,
     )
-    parser.add_argument(
-        "--results-dir",
-        type=str,
-        help="Specify a custom directory to load/store (historical) results from",
-        default=options.results_directory_override,
-    )
     parser.add_argument(
         "--build-jobs",
         type=int,
         help="Number of build jobs to run simultaneously",
         default=options.build_jobs,
     )
     parser.add_argument(
-        "--timestamp-override",
+        "--hip-arch",
         type=str,
-        help="Used in CI to enforce use of same timestamp across scripts",
+        help="HIP device architecture",
         default=None,
     )
+
+    # Options intended for CI:
     parser.add_argument(
-        "--hip-arch",
+        "--results-dir",
         type=str,
-        help="HIP device architecture",
-        default=None,
+        help="Specify a custom directory to load/store (historical) results from",
+        default=options.results_directory_override,
+    )
+    parser.add_argument(
+        "--timestamp-override",
+        type=lambda ts: Validate.timestamp(
+            ts,
+            throw=argparse.ArgumentTypeError("Specified timestamp not in YYYYMMDD_HHMMSS format.")
+        ),
+        help="Manually specify timestamp used in metadata",
+        default=options.timestamp_override,
+    )
+    parser.add_argument(
+        "--sycl-github-repo",
+        type=lambda gh_repo: Validate.github_repo(
+            gh_repo,
+            throw=argparse.ArgumentTypeError("Specified SYCL github repo not in <owner>/<repo> format.")
+        ),
+        help="Manually specify SYCL github repo used in metadata",
+        default=options.sycl_github_repo,
+    )
+    parser.add_argument(
+        "--sycl-commit",
+        type=lambda commit: Validate.commit_hash(
+            commit,
+            throw=argparse.ArgumentTypeError("Specified SYCL commit is not a valid commit hash.")
+        ),
+        help="Manually specify commit hash used to build SYCL in metadata",
+        default=options.sycl_commit,
     )
 
     args = parser.parse_args()
@@ -539,14 +562,18 @@ def validate_and_parse_env_args(env_args):
         if not os.path.isdir(args.output_dir):
             parser.error("Specified --output-dir is not a valid path")
         options.output_directory = os.path.abspath(args.output_dir)
-    if args.timestamp_override is not None:
-        if not Validate.timestamp(args.timestamp_override):
-            parser.error("--timestamp_override is not a valid timestamp")
-        options.timestamp_override = args.timestamp_override
+
+    # Options intended for CI:
+    options.timestamp_override = args.timestamp_override
     if args.results_dir is not None:
         if not os.path.isdir(args.results_dir):
             parser.error("Specified --results-dir is not a valid path")
         options.results_directory_override = os.path.abspath(args.results_dir)
+    if args.sycl_github_repo is not None or args.sycl_commit is not None:
+        if args.sycl_github_repo is None or args.sycl_commit is None:
+            parser.error("--sycl-github-repo and --sycl-commit must both be defined together")
+        options.sycl_github_repo = args.sycl_github_repo
+        options.sycl_commit = args.sycl_commit
 
     benchmark_filter = re.compile(args.filter) if args.filter else None
 

devops/scripts/benchmarks/options.py

@@ -46,20 +46,23 @@ class Options:
     preset: str = "Full"
     build_jobs: int = multiprocessing.cpu_count()
 
-    # Options applicable to CI only:
+    # Options intended for CI:
     regression_threshold: float = 0.05
-    # In CI, it may be necessary to e.g. compare or redo benchmark runs.
-    # A timestamp is generated at the beginning of the CI run and used through
-    # the entire CI process, instead of scripts generating their own timestamps
-    # every time a script runs (default behavior).
+    # It's necessary in CI to compare or redo benchmark runs. Instead of
+    # generating a new timestamp each run by default, specify a single timestamp
+    # to use across the entire CI run.
     timestamp_override: str = None
-    # By default, the directory to fetch results from is the benchmark working
-    # directory specified in the CLI args, hence a default value of "None" as
-    # the value is decided via runtime.
+    # The default directory to fetch results from is args.benchmark_directory,
+    # hence a default value of "None" as the value is decided during runtime.
     #
     # However, sometimes you may want to fetch results from a different
     # directory, i.e. in CI when you clone the results directory elsewhere.
     results_directory_override: str = None
+    # By default, we fetch SYCL commit info from the folder where main.py is
+    # located. This doesn't work right when CI uses different commits for e.g.
+    # CI scripts vs SYCl build source.
+    sycl_github_repo: str = None
+    sycl_commit: str = None
 
 
 options = Options()

devops/scripts/benchmarks/utils/validate.py

@@ -1,22 +1,68 @@
 import re
 
+def validate_on_re(val: str, regex: re.Pattern, throw: Exception = None):
+    """
+    Returns True if val is matched by pattern defined by regex, otherwise False.
+
+    If `throw` argument is not None: return val as-is if val matches regex,
+    otherwise raise error defined by throw.
+    """
+    is_matching: bool = re.compile(regex).match(val) is not None
+
+    if throw is None: return is_matching
+    elif not is_matching: raise throw
+    else: return val
+
+
 class Validate:
     """Static class containing methods for validating various fields"""
 
     @staticmethod
-    def runner_name(runner_name: str) -> bool:
+    def runner_name(runner_name: str, throw: Exception = None):
         """
         Returns True if runner_name is clean (no illegal characters).
         """
-        runner_name_re = re.compile(r"[a-zA-Z0-9_]+")
-        return runner_name_re.match(runner_name) is not None
+        return validate_on_re(runner_name, r"^[a-zA-Z0-9_]+$", throw=throw)
 
     @staticmethod
-    def timestamp(t: str) -> bool:
+    def timestamp(t: str, throw: Exception = None):
         """
         Returns True if t is in form YYYYMMDD_HHMMSS, otherwise False.
+
+        If throw argument is specified: return t as-is if t is in aforementioned
+        format, otherwise raise error defined by throw.
+        """
+        return validate_on_re(
+            t,
+            r"^\d{4}(0[1-9]|1[0-2])([0-2][0-9]|3[01])_([01][0-9]|2[0-3])[0-5][0-9][0-5][0-9]$",
+            throw=throw
+        )
+
+    @staticmethod
+    def github_repo(repo: str, throw: Exception = None):
+        """
+        Returns True if repo is of form <owner>/<repo name>
+
+        If throw argument is specified: return repo as-is if repo is in
+        aforementioned format, otherwise raise error defined by throw.
         """
-        timestamp_re = re.compile(
-            r"^\d{4}(0[1-9]|1[0-2])([0-2][0-9]|3[01])_([01][0-9]|2[0-3])[0-5][0-9][0-5][0-9]$"
+        return validate_on_re(
+            re.sub(r"^https?://github.com/", "", repo),
+            r"^[a-zA-Z0-9_-]{1,39}/[a-zA-Z0-9_.-]{1,100}$",
+            throw=throw
         )
-        return timestamp_re.match(t) is not None
+
+    @staticmethod
+    def commit_hash(commit: str, throw: Exception = None, trunc: int = 40):
+        """
+        Returns True if commit is a valid git commit hash.
+
+        If throw argument is specified: return commit hash (truncated to trunc
+        chars long) if commit is a valid commit hash, otherwise raise error
+        defined by throw.
+        """
+        commit_re = r"^[a-f0-9]{7,40}$"
+        if throw is None:
+            return validate_on_re(commit, commit_re)
+        else:
+            return validate_on_re(commit, commit_re, throw=throw)[:trunc]