Skip to content

nvbench::cpu_timer changed to use steady_clock - #371

Merged
oleksandr-pavlyk merged 1 commit into
NVIDIA:mainfrom
oleksandr-pavlyk:cpu-timer-use-steady-clock
May 20, 2026
Merged

nvbench::cpu_timer changed to use steady_clock#371
oleksandr-pavlyk merged 1 commit into
NVIDIA:mainfrom
oleksandr-pavlyk:cpu-timer-use-steady-clock

Conversation

@oleksandr-pavlyk

Copy link
Copy Markdown
Collaborator

Closes #370

Using std::chrono::steady_clock is more appropriate for timing measurements. It guarantees that duration computed from two time-points will not contain correction deltas (positive or negative).

Using steady_clock is more appropriate for timing measurements.
It guarantees that duration computed from two time-points will not
contain correction deltas.
@oleksandr-pavlyk oleksandr-pavlyk self-assigned this May 19, 2026
@oleksandr-pavlyk oleksandr-pavlyk added the type: bug: functional Does not work as intended. label May 19, 2026
@github-project-automation github-project-automation Bot moved this to Todo in CCCL May 19, 2026
@oleksandr-pavlyk oleksandr-pavlyk moved this from Todo to In Review in CCCL May 19, 2026
@oleksandr-pavlyk

Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7cbb7b8d-c665-4daa-b37b-13a9261eeeba

📥 Commits

Reviewing files that changed from the base of the PR and between 4a33a61 and 9b0c8ee.

📒 Files selected for processing (1)
  • nvbench/cpu_timer.cuh

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes

  • Fixed potential negative or incorrect elapsed times in CPU timer measurements.

Walkthrough

The PR modifies nvbench::cpu_timer to ensure CPU timing uses a monotonic clock. A compile-time type alias selects high_resolution_clock when steady, otherwise falls back to steady_clock. A static assertion enforces that the selected clock is steady. The start() and stop() methods now capture time from the selected clock instead of always using high_resolution_clock.

Changes

Steady clock for CPU timing

Layer / File(s) Summary
Clock type selection and validation
nvbench/cpu_timer.cuh
cpu_timer_clock is conditionally aliased to high_resolution_clock (if steady) or steady_clock (if not), with a compile-time assertion ensuring the selected clock is steady. A <type_traits> include is added to support the conditional type selection.
start() and stop() methods updated
nvbench/cpu_timer.cuh
cpu_timer::start() and cpu_timer::stop() now call cpu_timer_clock::now() instead of std::chrono::high_resolution_clock::now(), and member types are updated to use time_point_t derived from the selected clock.

Assessment against linked issues

Objective Addressed Explanation
Use steady_clock instead of high_resolution_clock for CPU timing [#370]

Comment @coderabbitai help to get the list of available commands and usage tips.

@oleksandr-pavlyk
oleksandr-pavlyk merged commit 84c7952 into NVIDIA:main May 20, 2026
98 of 100 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in CCCL May 20, 2026
@oleksandr-pavlyk
oleksandr-pavlyk deleted the cpu-timer-use-steady-clock branch May 20, 2026 15:22
oleksandr-pavlyk added a commit that referenced this pull request Jul 20, 2026
Use `device_clock = cuda::std::chrono::high_resolution_clock;` for readability
improvement, and a chance for comment to state that using it in device
code for timing purposes is correct and intentional.

I looked at this as a follow-up to #371, which replaced
std::chrono::high_resolution_clock with std::chrono::steady_clock.

Per https://nvidia.github.io/cccl/unstable/libcudacxx/standard_api/time_library.html
type cuda::std::chrono::steady_clock is not provided, and while
high_resolution_clock is not heterogeneously steady, but is steady within device
code and suitable for device-side performance measurement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug: functional Does not work as intended.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Use std::chrono::steady_clock instead of high_resolution_clock for CPU timing measurements

2 participants