Skip to content

[Benchmarks] Use combo profiler in UR SubmitKernel scenarios #20295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 14 commits into from
Oct 16, 2025
Merged

[Benchmarks] Use combo profiler in UR SubmitKernel scenarios #20295

merged 14 commits into from
Oct 16, 2025

Conversation

PatKamin
Copy link
Contributor

@PatKamin PatKamin commented Oct 7, 2025 •
edited
Loading

Use the Combo Profiler in all supported Compute Benchmark scenarios. Introduce the --profiler-type parameter for picking one profiler each run. This effectively disables running the same scenarios two times: with time measurement and with CPU instructions retired counter.

Leaving by default CPU counter scenarios enabled in CI. This will result in less scenarios being ran as we won't run time-measuring scenarios where CPU counter is available. In the dashboard, only historic results will be drawn, no new runs in such cases.

The CPU count groups and names suffix can now be safely determined beforehand, without having to parse run logs.

Also, a Compute Benchmarks version bump is made for these changes.

@PatKamin PatKamin requested review from a team as code owners October 7, 2025 08:48
lukaszstolarczuk
lukaszstolarczuk reviewed Oct 7, 2025
View reviewed changes
@PatKamin PatKamin requested a review from lslusarczyk October 7, 2025 11:49
lukaszstolarczuk
lukaszstolarczuk reviewed Oct 7, 2025
View reviewed changes
Copy link
Contributor

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

lslusarczyk
lslusarczyk requested changes Oct 8, 2025
View reviewed changes
@PatKamin PatKamin requested a review from vinser52 October 8, 2025 10:03
@PatKamin
[Benchmarks] Measure cpu instructions optionally
65d85c8 
It makes sense to measure either time elapsed or cpu instructions
retired, not both. Adding an argument to scripts to run Compute
Benchmarks scenarios with only one of two scenarios would lower
the number of benchmark scenarios significantly. This would make
the tests to take less amount of time to complete, giving the user an
option to produce either time or cpu instructions count results,
where applicable.
@PatKamin
More explicit 'CPU count' separator usage
ee9bd83
@PatKamin PatKamin requested a review from lslusarczyk October 8, 2025 10:55
@PatKamin
Copy link
Contributor Author

@vinser52, please, review. Now both time and CPU instructions count are always measured.

@PatKamin PatKamin requested a review from vinser52 October 14, 2025 06:54
@PatKamin
Don't create new names
1aee5e0 
Avoid creating new groups/names as each runtime has only one valid EmulateGraphs value.
lslusarczyk
lslusarczyk reviewed Oct 14, 2025
View reviewed changes
vinser52
vinser52 requested changes Oct 15, 2025
View reviewed changes
@vinser52
Copy link
Contributor

@vinser52, please, review. Now both time and CPU instructions count are always measured.

Not directly related to this PR, but follow-up. Do I understand correctly that ComboProfiler is not supported in L0 versions of the benchmarks? Are you planning to extend L0 versions as well as a next step?

@PatKamin
Copy link
Contributor Author

@vinser52, please, review. Now both time and CPU instructions count are always measured.

Not directly related to this PR, but follow-up. Do I understand correctly that ComboProfiler is not supported in L0 versions of the benchmarks? Are you planning to extend L0 versions as well as a next step?

Yes, currently only timer is supported in SubmitKernel and SubmitGraph Level Zero scenarios in Compute Benchmarks' implementation. We might add the CPU counter in some future PRs, if needed.

lukaszstolarczuk
lukaszstolarczuk reviewed Oct 16, 2025
View reviewed changes
devops/scripts/benchmarks/benches/compute.py Outdated
return self.bench.project.build_dir / "bin" / self.bench_name

def cpu_count_str(self, separator: str = "") -> str:
# Note: SYCL CI currently parses for on this "CPU count" value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you rebase with my PR, pls note there's a slight change in here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, rebased

@PatKamin PatKamin requested a review from vinser52 October 16, 2025 15:36
vinser52
vinser52 reviewed Oct 16, 2025
View reviewed changes
Copy link
Contributor

@vinser52 vinser52 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I am not an expert in this part. I would ask someone else who is more familiar with these scripts to approve. But no objections from my side.

@PatKamin
Copy link
Contributor Author

@intel/llvm-gatekeepers, please merge.

@sarnex sarnex merged commit 93a515d into intel:sycl Oct 16, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukaszstolarczuk lukaszstolarczuk lukaszstolarczuk left review comments

@lslusarczyk lslusarczyk lslusarczyk approved these changes

@sarnex sarnex sarnex approved these changes

@vinser52 vinser52 vinser52 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@PatKamin @lslusarczyk @vinser52 @sarnex @lukaszstolarczuk