[UR][CI] add manually triggered benchmark action #17088

pbalcer · 2025-02-20T11:34:22Z

This is a first step towards reenabling UR performance testing CI. This introduces the reusable yml workflow and a way to trigger it manually.

Here's an example how it looks:
pbalcer#2 (comment)

pbalcer · 2025-02-20T11:34:33Z

ping @ianayl

.github/workflows/benchmarks-reusable.yml

.github/workflows/benchmarks.yml

.github/workflows/benchmarks-reusable.yml

lukaszstolarczuk · 2025-02-20T12:44:31Z

also, FYI, python code formatting failed

pbalcer · 2025-02-20T14:03:11Z

This should be ready to review, but we still don't have the runner configured.

lukaszstolarczuk · 2025-02-20T14:39:06Z

LGTM (I can't close my issues)

ianayl · 2025-02-20T15:37:50Z

.github/workflows/benchmarks-reusable.yml

The naming of this file could conform better to what's currently in .github/workflows: how about something like ur-benchmarks-reusable.yml for now, and then it can be mulled upon later?

We can do that. It's not just UR though.

Done, renamed them to ur-benchmarks-reusable.yml.

ianayl · 2025-02-20T15:37:55Z

.github/workflows/benchmarks-reusable.yml

+      env:
+        PR_NO: ${{ inputs.pr_no }}
+      run: |
+        git fetch -- https://github.com/${{github.repository}} +refs/pull/${PR_NO}/*:refs/remotes/origin/pr/${PR_NO}/*


Suggested change

git fetch -- https://github.com/${{github.repository}} +refs/pull/${PR_NO}/*:refs/remotes/origin/pr/${PR_NO}/*

git fetch -- https://github.com/intel/llvm "+refs/pull/${PR_NO}/*:refs/remotes/origin/pr/${PR_NO}/*"

Two quick things:

Shell variables being used should always be quoted to avoid code injection

NIT: Honestly this is way paranoid, but the "best" known security practice right now is to not trust the github context, as it's been used in code injection before. I'll leave this up to your discretion though.

Shell variables being used should always be quoted to avoid code injection

Fixed.

ianayl · 2025-02-20T15:38:18Z

.github/workflows/benchmarks-reusable.yml

+        PR_NO: ${{ inputs.pr_no }}
+      run: |
+        git fetch -- https://github.com/${{github.repository}} +refs/pull/${PR_NO}/*:refs/remotes/origin/pr/${PR_NO}/*
+        git checkout origin/pr/${PR_NO}/merge


Suggested change

git checkout origin/pr/${PR_NO}/merge

git checkout "origin/pr/${PR_NO}/merge"

I removed the PR_NO env var.

ianayl · 2025-02-20T15:38:22Z

.github/workflows/benchmarks-reusable.yml

+      run: |
+        git fetch -- https://github.com/${{github.repository}} +refs/pull/${PR_NO}/*:refs/remotes/origin/pr/${PR_NO}/*
+        git checkout origin/pr/${PR_NO}/merge
+        git rev-parse origin/pr/${PR_NO}/merge


Suggested change

git rev-parse origin/pr/${PR_NO}/merge

git rev-parse "origin/pr/${PR_NO}/merge"

I removed the PR_NO env var.

ianayl · 2025-02-20T15:38:29Z

.github/workflows/benchmarks-reusable.yml

+        ${{matrix.adapter.sycl_config}}
+
+    - name: Build SYCL
+      run: cmake --build ${{github.workspace}}/sycl_build -j $(nproc)


All workloads in intel/llvm use https://github.com/intel/llvm/blob/sycl/.github/workflows/sycl-linux-build.yml for building, which produces an artifact that is then accepted by multiple different workflows in intel/llvm.

I think the goal'd be to get this workflow up as fast as possible, so for the time being this is probably fine. However, we'll want this changed to use sycl-linux-build.yml eventually.

Yup, let's leave that as TODO.

ianayl · 2025-02-20T15:38:34Z

.github/workflows/benchmarks-reusable.yml

+      run: |
+        # Compute the core range for the first NUMA node; second node is for UMF jobs.
+        # Skip the first 4 cores - the kernel is likely to schedule more work on these.
+        CORES=$(lscpu | awk '


Suggested change

CORES=$(lscpu | awk '

CORES="$(lscpu | awk '

ianayl · 2025-02-20T15:38:38Z

.github/workflows/benchmarks-reusable.yml

+            split(a[4], b, ",")
+            sub(/^0/, "4", b[1])
+            print b[1]
+          }')


Suggested change

}')

}')"

ianayl · 2025-02-20T15:38:47Z

.github/workflows/benchmarks-reusable.yml

+      working-directory: ${{ github.workspace }}
+      id: benchmarks
+      run: >
+        taskset -c ${{ env.CORES }} ${{ github.workspace }}/sycl-repo/unified-runtime/scripts/benchmarks/main.py


Suggested change

taskset -c ${{ env.CORES }} ${{ github.workspace }}/sycl-repo/unified-runtime/scripts/benchmarks/main.py

taskset -c "${{ env.CORES }}" ${{ github.workspace }}/sycl-repo/unified-runtime/scripts/benchmarks/main.py

ianayl · 2025-02-20T15:42:30Z

Unfortunately I'm not in dpcpp-devops-reviewers, so final say is with them instead. But:

I see you've your own llvm branch for testing. I think it makes sense but I'm not sure if that's enough: We may want to see testing triggered from intel/llvm as well, in which you'll have to string this up to another existing workflow. This is up to dpcpp-devops-reviewers though however.
Is the runner you wanted UR_DNP_INTEL_05_01? Or is it another machine?

pbalcer · 2025-02-20T15:49:31Z

I see you've your own llvm branch for testing. I think it makes sense but I'm not sure if that's enough: We may want to see testing triggered from intel/llvm as well, in which you'll have to string this up to another existing workflow. This is up to dpcpp-devops-reviewers though however.

I don't believe I have write access to intel/llvm to push the workflow on a branch here.

Is the runner you wanted UR_DNP_INTEL_05_01? Or is it another machine?

It's UR_DNP_INTEL_06_01 I think? I talked with @lukaszstolarczuk and he will setup a runner with PVC_PERF label that matches what's in this PR.

lukaszstolarczuk · 2025-02-20T17:16:52Z

FYI, so the new runner should be up now (it's actually called UR_DNP_INTEL_06_03).

pbalcer · 2025-02-20T17:28:44Z

FYI, so the new runner should be up now (it's actually called UR_DNP_INTEL_06_03).

Thanks!

pbalcer · 2025-02-20T17:33:32Z

@intel/unified-runtime-reviewers @intel/dpcpp-devops-reviewers please review. This is just porting an existing workflow from UR. It's not fully conformant with how other intel/llvm workflows are written, and we plan on addressing that after a merge.

Example comment is here: pbalcer#2 (comment)
Logs here: https://github.com/pbalcer/llvm/actions/runs/13440448856/job/37553283023

From what I understand, it's not possible to test workflow_dispatch directly in intel/llvm prior to a merge. Is my testing on the fork enough or is there anything specific you'd like me to do?

kbenzie

UR LGTM

aelovikov-intel · 2025-02-20T18:01:58Z

From what I understand, it's not possible to test workflow_dispatch directly in intel/llvm prior to a merge. Is my testing on the fork enough or is there anything specific you'd like me to do?

Correct. No, your testing should be enough.

aelovikov-intel

I don't like that it still feels as if SYCL and UR are two completely different projects. Can't we unify the build here and re-use normal artifacts? Why does it have to be different from how we run SYCL E2E tests?

It's not fully conformant with how other intel/llvm workflows are written, and we plan on addressing that after a merge.

The merge has happened, hasn't it?

pbalcer · 2025-02-20T18:19:15Z

I don't like that it still feels as if SYCL and UR are two completely different projects. Can't we unify the build here and re-use normal artifacts? Why does it have to be different from how we run SYCL E2E tests?

This is the quickest way of getting the perf testing functionality back. Right now we have no way to test performance changes in the adapters.
I agree that it's not ideal, and we are committed to improving these workflows iteratively so that they match how the rest of intel/llvm workflows work.

It's not fully conformant with how other intel/llvm workflows are written, and we plan on addressing that after a merge.

The merge has happened, hasn't it?

I was referring to a merge of this patch.

aelovikov-intel · 2025-02-20T18:22:26Z

Still a draft because there's no runner in intel/llvm (WIP).

Please fix PR descrption.

aelovikov-intel · 2025-02-20T18:24:55Z

.github/workflows/ur-benchmarks-reusable.yml

+          github.rest.issues.createComment({
+            issue_number: pr_no,
+            owner: context.repo.owner,
+            repo: context.repo.repo,
+            body: body
+          })


For future, do we want it to be a comment or a summary of this job (like, e.g., https://github.com/intel/llvm/actions/runs/13438348841)?

Good question. I think a comment is more visible. Right now we do not fail a job if the scripts think there's a regression, so if the status is hidden in the job logs, people might forget to check.

In the near future we also plan on uploading a set of html charts per PR (basically this: https://oneapi-src.github.io/unified-runtime/benchmark_results.html but with a PR marked on the chart, so you can easily compare against previous nightly runs). Again, I think people are more likely to use this functionality if it's right there in the comment.

pbalcer · 2025-02-20T18:26:26Z

Still a draft because there's no runner in intel/llvm (WIP).

Please fix PR descrption.

Done.

This is a first step towards reenabling UR performance testing CI. This introduces the reusable yml workflow and a way to trigger it manually.

pbalcer · 2025-02-21T06:00:51Z

@intel/llvm-gatekeepers please merge.
The CI failure is unrelated (this PR doesn't change any runtime code) and is failing in other PRs as well (e.g., https://github.com/intel/llvm/actions/runs/13441609022/job/37557207692?pr=17101).

uditagarwal97 · 2025-02-21T06:04:08Z

Failed Tests (1):
  SYCL :: e2e_test_requirements/no-unsupported-without-info.cpp

was fixed in e4d65e0

pbalcer temporarily deployed to WindowsCILock February 20, 2025 11:34 — with GitHub Actions Inactive

pbalcer temporarily deployed to WindowsCILock February 20, 2025 11:49 — with GitHub Actions Inactive

lukaszstolarczuk reviewed Feb 20, 2025

View reviewed changes

pbalcer force-pushed the bench-workflow-pr branch from f5d0218 to 2eaa1dd Compare February 20, 2025 14:01

pbalcer temporarily deployed to WindowsCILock February 20, 2025 14:01 — with GitHub Actions Inactive

pbalcer temporarily deployed to WindowsCILock February 20, 2025 14:16 — with GitHub Actions Inactive

ianayl reviewed Feb 20, 2025

View reviewed changes

pbalcer force-pushed the bench-workflow-pr branch from 2eaa1dd to ffc93a7 Compare February 20, 2025 17:22

pbalcer had a problem deploying to WindowsCILock February 20, 2025 17:23 — with GitHub Actions Error

pbalcer force-pushed the bench-workflow-pr branch from ffc93a7 to f036d58 Compare February 20, 2025 17:28

pbalcer marked this pull request as ready for review February 20, 2025 17:28

pbalcer requested review from a team as code owners February 20, 2025 17:28

pbalcer had a problem deploying to WindowsCILock February 20, 2025 17:29 — with GitHub Actions Error

kbenzie approved these changes Feb 20, 2025

View reviewed changes

pbalcer force-pushed the bench-workflow-pr branch from f036d58 to 78807df Compare February 20, 2025 18:00

pbalcer temporarily deployed to WindowsCILock February 20, 2025 18:02 — with GitHub Actions Inactive

aelovikov-intel reviewed Feb 20, 2025

View reviewed changes

aelovikov-intel approved these changes Feb 20, 2025

View reviewed changes

[UR][CI] add manually triggered benchmark action

78807df

This is a first step towards reenabling UR performance testing CI. This introduces the reusable yml workflow and a way to trigger it manually.

pbalcer temporarily deployed to WindowsCILock February 20, 2025 19:54 — with GitHub Actions Inactive

uditagarwal97 merged commit 770afbf into intel:sycl Feb 21, 2025
28 of 29 checks passed

kbenzie mentioned this pull request Feb 21, 2025

Pull in intel/llvm changes to main - Fri 21st Feb oneapi-src/unified-runtime#2719

Merged

	git fetch -- https://github.com/${{github.repository}} +refs/pull/${PR_NO}/:refs/remotes/origin/pr/${PR_NO}/
	git fetch -- https://github.com/intel/llvm "+refs/pull/${PR_NO}/:refs/remotes/origin/pr/${PR_NO}/"

	git checkout origin/pr/${PR_NO}/merge
	git checkout "origin/pr/${PR_NO}/merge"

	git rev-parse origin/pr/${PR_NO}/merge
	git rev-parse "origin/pr/${PR_NO}/merge"

	taskset -c ${{ env.CORES }} ${{ github.workspace }}/sycl-repo/unified-runtime/scripts/benchmarks/main.py
	taskset -c "${{ env.CORES }}" ${{ github.workspace }}/sycl-repo/unified-runtime/scripts/benchmarks/main.py

[UR][CI] add manually triggered benchmark action #17088

[UR][CI] add manually triggered benchmark action #17088

Conversation

pbalcer commented Feb 20, 2025 • edited Loading

pbalcer commented Feb 20, 2025

lukaszstolarczuk commented Feb 20, 2025

pbalcer commented Feb 20, 2025

lukaszstolarczuk commented Feb 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbalcer Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianayl commented Feb 20, 2025

pbalcer commented Feb 20, 2025

lukaszstolarczuk commented Feb 20, 2025

pbalcer commented Feb 20, 2025 • edited Loading

pbalcer commented Feb 20, 2025 • edited Loading

kbenzie left a comment

Choose a reason for hiding this comment

aelovikov-intel commented Feb 20, 2025

aelovikov-intel left a comment • edited Loading

Choose a reason for hiding this comment

pbalcer commented Feb 20, 2025

aelovikov-intel commented Feb 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbalcer commented Feb 20, 2025

pbalcer commented Feb 21, 2025

uditagarwal97 commented Feb 21, 2025

pbalcer commented Feb 20, 2025 •

edited

Loading

pbalcer Feb 20, 2025 •

edited

Loading

pbalcer commented Feb 20, 2025 •

edited

Loading

pbalcer commented Feb 20, 2025 •

edited

Loading

aelovikov-intel left a comment •

edited

Loading