Skip to content

Fix, build and test the energy profiler daemon - #350

Open
ethan-puyaubreau wants to merge 7 commits into
kokkos:developfrom
ethan-puyaubreau:fix/energy-profiler-daemon
Open

ethan-puyaubreau wants to merge 7 commits into
kokkos:developfrom
ethan-puyaubreau:fix/energy-profiler-daemon

Conversation

@ethan-puyaubreau

@ethan-puyaubreau ethan-puyaubreau commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #300: no target builds the daemon, and it does not compile (std::chrono::duration needs template arguments).

  • Take the interval as std::chrono::nanoseconds, so milliseconds, microseconds or seconds convert to it implicitly.
  • Build the daemon as a static library, kp_energy_profiler_daemon.
  • Run the daemon on a std::jthread: stop() requests a stop that wakes the thread instead of waiting up to a full interval, and destroying a running daemon stops it instead of calling std::terminate. ThreadSanitizer reported a data race on running_ before.
  • Schedule runs on steady_clock: high_resolution_clock is system_clock in libstdc++.
  • Add the unit tests asked for in the review of Energy profiling tools: Add Daemon class for periodic task execution #300.

Tested on my fork against Kokkos 5.0.2 and develop.

@JBludau

JBludau commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

you can even use c++20 and jthread now

@ethan-puyaubreau

Copy link
Copy Markdown
Contributor Author

Indeed, that was the plan from #300 once we had C++20! Switched to std::jthread in 8f4af23: request_stop now wakes and stops the thread, and the jthread joins itself, so the hand-written stop signaling and the destructor are gone.

@JBludau JBludau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Ethan!

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.

2 participants