Skip to content

Conversation

@CongMa13
Copy link
Collaborator

@CongMa13 CongMa13 commented Jan 16, 2026

Proposed changes

I did some more digging and found that chrono is overkill for what we need. Including is notoriously slow because it is a heavyweight header. It doesn't just define a few functions; it brings in a massive web of templates, type traits, and extensive timezone and calendar support.

What we need is just a simple timer.

I replace it chrono with some C function. ( for windows and linux)

Tracking issue: #3575

Checklist

Please put an x into the boxes that apply. You can also fill these out after creating the PR. If you're not sure, please don't hesitate to ask.

  • I have added tests relevant to the introduced functionality, and the unit tests are passing locally
  • I have added the test to REGRESSION_TESTS list defined at the top of CMakeLists.txt in tests/CMakeLists.txt, IF the test takes more than 30 seconds to run.
  • I have added inline documentation which enables the maintainers with understanding the motivation
  • I have removed the stale documentation which is no longer relevant after this pull request
  • (If this change is user-facing) I have added release notes which provide the end users with a brief summary of the improvement from this pull request
  • I have run clang-format on all changed files
  • Any dependent changes have been merged

Discussion

If this is a relatively large or complex change, feel free to start a discussion by explaining why you chose the solution you did and what alternatives you considered

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the dependency on <chrono> from the CK TILE library's timer implementation to improve compilation times. The heavyweight <chrono> header is replaced with lightweight platform-specific C functions for Windows (QueryPerformanceCounter) and Linux/Unix (clock_gettime).

Changes:

  • Replaced std::chrono timer implementation with custom high-resolution clock functions
  • Created new high_res_cpu_clock.hpp header with platform-specific timing implementations
  • Updated cpu_timer struct to use the new timing functions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
include/ck_tile/host/timer.hpp Replaced chrono-based timer with custom high_res_now() functions and timepoint_t type
include/ck_tile/host/high_res_cpu_clock.hpp Added new header implementing platform-specific high-resolution timing using C APIs
include/ck_tile/host.hpp Added include for the new high_res_cpu_clock.hpp header

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@CongMa13 CongMa13 force-pushed the congma13/ck_tile/reduce_building_time branch from edbac19 to f0ae519 Compare January 17, 2026 00:17
Copy link
Contributor

@tenpercent tenpercent left a comment

Choose a reason for hiding this comment

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

since we're replacing some stdlib functionality with our custom one, should we also add tests for a sanity check?

@illsilin
Copy link
Collaborator

illsilin commented Jan 19, 2026

since we're replacing some stdlib functionality with our custom one, should we also add tests for a sanity check?

it's still using standard timing functions, just the ones defined in <time.h> or <windows.h> headers instead of <chrono.h> . those are pretty common and tested functions, so i'm sure we can trust them.

@illsilin
Copy link
Collaborator

not sure why the miopen fp32 convolution failed. just restarted TheRock CI build.

@illsilin illsilin merged commit 0517d43 into develop Jan 19, 2026
28 of 30 checks passed
@illsilin illsilin deleted the congma13/ck_tile/reduce_building_time branch January 19, 2026 23:31
kensclin pushed a commit that referenced this pull request Jan 20, 2026
* [CK TILE] remove dependency on std chrono

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants