Energy profiling tools: Add Daemon class for periodic task execution by ethan-puyaubreau · Pull Request #300 · kokkos/kokkos-tools

ethan-puyaubreau · 2025-08-15T15:56:43Z

This PR introduces a generic Daemon class for background task execution in the energy profiler infrastructure:

Thread-safe periodic execution with configurable intervals
Clean start/stop lifecycle management
Reusable foundation for background monitoring tasks

The daemon system will enable energy providers to perform periodic power sampling at specified intervals, supporting the energy measurement capabilities planned in #301.

JBludau · 2025-08-28T20:55:31Z

+  void run();
+  void stop();
+  bool is_running() const { return running_; }
+  std::thread& get_thread() { return thread_; }


do we actually want this to be modifiable?

JBludau

Description and title could be updated, but the rest is ok for now

JBludau · 2025-08-29T15:39:58Z

If we go to c++20 we can use jthread for thread safety, but that is for kokkos 5

maartenarnst · 2025-09-01T08:47:54Z

+
+class Daemon {
+ public:
+  Daemon(std::function<void()> func, int interval_ms)


Should we pass in a std::chrono::duration directly, instead of an int? What about the type int; e.g. what is the default arithmetic type that std::chrono::milliseconds encodes?

yes, that is a good idea.

done in 646bf35

maartenarnst · 2025-09-01T08:49:14Z

+  std::thread& get_thread() { return thread_; }
+
+ private:
+  std::chrono::milliseconds interval_;


Should this be chosen as the smallest possible tick period? Thinking that "milliseconds" may be quite long as a sampling interval.

hmm ... we could use the smallest to make it as general as possible. Nevertheless, the question that arises is if there is anything useful below the ms threshold. Do you have an example in mind?

changed in 646bf35

changed in 646bf35

Sounds good though at least for interactions with performance monitoring systems (including for #291 for LDMS), I don't think there is any benefit (and it may actually be detrimental) to go below the millisecond level in production scenarios.

maartenarnst · 2025-09-01T08:49:25Z

+
+ private:
+  std::chrono::milliseconds interval_;
+  bool running_{false};


Is there an issue of thread safety?

yes. As stated in a previous comment, we could just switch to jthread and https://en.cppreference.com/w/cpp/thread/jthread/request_stop.html once we have c++20

maartenarnst · 2025-09-01T08:49:46Z

+#include <functional>
+#include <thread>
+#include <chrono>


Best to order the includes alphabetically.

done in 626f777

maartenarnst · 2025-09-01T08:50:44Z

+//
+//@HEADER
+
+#pragma once


We should probably agree at the level of kokkos-tools on using consistently either preprocessor conditionals or this pragma to protect our includes.

I agree that consistency would be a good thing to aim for. There are a lot more things in Tools that could be made consistent though.

switched to classic ifdef in 67a8030

maartenarnst · 2025-09-01T08:52:58Z

+  } else {
+    throw std::runtime_error("Daemon already started");


We may want to think about how to be consistent in handling errors. In your tool, it appears you're sometimes using exceptions, and sometimes using a boolean to handle errors. I'm not sure if there's a policy at the kokkos-tools level to avoid exceptions. Probably best to check e.g. with @JBludau.

I would even say we don't want to throw when the daemon is already running. This is not an exception but should just do nothing if the daemon is started.

removed the exceptions in 4502851

Yeah, exceptions aren't really used for Kokkos Tools libraries - and I think not throwing here is the right choice.

maartenarnst

This PR would also benefit from tests and an example.

maartenarnst · 2025-09-18T13:08:07Z

Hey @JBludau. I've just gone over the changes. You'll find a few suggestions. Sill thinking that it would be good to add a small test. Perhaps there could be a test with a callback function that just waits for a given duration. The test could use durations that would be typical for the use case of the energy profiler.

Co-authored-by: Maarten Arnst <maarten.arnst@ulg.ac.be>

JBludau · 2025-09-18T13:42:18Z

Hey @JBludau. I've just gone over the changes. You'll find a few suggestions. Sill thinking that it would be good to add a small test. Perhaps there could be a test with a callback function that just waits for a given duration. The test could use durations that would be typical for the use case of the energy profiler.

Yeah, I asked @ethan-puyaubreau if he could add his test.

aprokop

This looks good to me. I only have a couple question.
We need this capability as soon as we can for one of my projects.

aprokop · 2025-10-10T17:54:29Z

+
+namespace KokkosTools::EnergyProfiler {
+void Daemon::start() {
+  if (!running_) {


What are the semantics of start() if it's already running? Should it break? Should it be restarted?

I think it should just do nothing if it is already running. Multiple start calls should be fine since in our current use case the global system time is recorded. If someone wants to use it differently I would opt we add a restart() functionality.

but as described below ... this is not threadsafe anyway

aprokop · 2025-10-10T17:55:28Z

+}
+
+void Daemon::stop() {
+  if (running_) {


What are the semantics here? Should we require that it's been running?

I mean, it is not threadsafe, so we are limited with the current impl. But if we care we can either go to jthread or at least use atomics for the status

FWIW, I think C++20 std::jthread is a great idea here. I have a slight inclination to use C++ atomics for now since it seems more straightforward - I am wondering how portable/available jthreads would be on various computing systems currently?

Currently the tools don't have a minimum requirement for 20, right? Since most of the other tools are not threadsafe, I would say that is a larger discussion.
But if we want it to be std::jthread now, we can update. I am happy for any contribution

Co-authored-by: Andrey Prokopenko <andrey.prok@gmail.com>

vlkale · 2025-12-02T17:15:30Z

Hi all, I was wondering about the status on this. I think this is a good PR that would be valuable for a lot of profiling data beyond energy information, which is relevant to #291 - albeit I would say the energy profiling use case is the one that is probably of most value currently to production Kokkos applications.

Do you know the rough timeline of this PR by chance? I know that @ethan-puyaubreau is done with his internship, so there may be timing constraints from his end.

I think this PR is on the right track overall.

JBludau · 2025-12-02T17:50:59Z

The main open thing is a desire to have an example for the daemon. So far I was not able to come up with a good one.
Personally I think it is fine without an example as the energy profiling pr's that depend on this are basically going to be example usages of this.

vlkale · 2025-12-02T23:54:53Z

The main open thing is a desire to have an example for the daemon. So far I was not able to come up with a good one. Personally I think it is fine without an example as the energy profiling pr's that depend on this are basically going to be example usages of this.

Yes, I definitely agree just considering the energy profiling use case already shows an example usage. It would certainly help to have a small example since it can make clear assumptions being made on its use, but I think it doesn't need to be a blocker. I do think some of the error handling and/or tests for corner cases (e.g., interval being longer than time for kernel) that are discussed by @maartenarnst and you in this PR are important to think about.

Except for making sure no critical error checks are missing, I think the code logic of this PR makes sense to me.

Doing some sort of beta release of this Daemon class, especially so, e.g., @aprokop along with #291 can use this would be good to get early feedback on real-world production use cases, as that could directly provide a lot of insight for improvements. Maybe it could be merged in as a beta feature early next year?

Just my thoughts on this now - I will look and see if I can make a few more improvements to the code and error checks in the PR myself till then.

vlkale

LGTM overall, just a suggestion to consider a default interval

vlkale · 2025-12-03T00:13:20Z

+namespace KokkosTools::EnergyProfiler {
+class Daemon {
+ public:
+  Daemon(std::function<void()> func, const std::chrono::duration& interval)


One quick note is I don't see a default value for the time interval of the Daemon but maybe you could have one. If you do, I think a reasonable one would be 1 second. This would at least makes sense from the point of view of #291. Maybe there are other use cases and situations where you want it to be shorter (or longer).

Hmm, I think the user explicitly specifying a duration is a good practice here. It is a core setting of the daemon that should be thought about explicitly.

Ah, got it - yes, I agree. We want this Daemon class to be generic beyond the energy profiling use case and/or LDMS use case, and it should indeed be thought out by users explicitly based on the scenario.

JBludau · 2025-12-05T20:46:02Z

@maartenarnst do you want us to investigate the use case of a longer return time than the frequency the daemon is given? If so, do you have a specific case in mind that would need this?

JBludau · 2026-01-05T21:52:03Z

@vlkale I think we can merge this. Maarten did not reply and we still can look at making the daemon able to deal with runtimes longer than its frequency later if we ever have a usecase for this

vlkale · 2026-01-06T04:46:41Z

@vlkale I think we can merge this. Maarten did not reply and we still can look at making the daemon able to deal with runtimes longer than its frequency later if we ever have a usecase for this

@JBludau I am good with merging it. Yes, allowing for runtimes longer than its frequency would be good but it's not needed right now for immediate/priority use cases.

JBludau · 2026-02-27T19:03:03Z

@vlkale could you merge this?

vlkale · 2026-03-04T22:07:13Z

@dalg24 @crtrott Can you please review and merge if it looks good to you?

This is a simple but important addition to Kokkos Tools for integrating profiling of system-level metrics like energy consumption.

ethan-puyaubreau changed the title ~~Feature/energy profiler daemon~~ Energy profiling tools: Deamon system Aug 15, 2025

ethan-puyaubreau changed the title ~~Energy profiling tools: Deamon system~~ Energy profiling tools: Daemon system Aug 15, 2025

ethan-puyaubreau mentioned this pull request Aug 15, 2025

Energy profiling tools: NVML-based measurement tool #301

Open

ethan-puyaubreau force-pushed the feature/energy-profiler-daemon branch from b757df7 to 84a0ef5 Compare August 15, 2025 17:14

ethan-puyaubreau mentioned this pull request Aug 15, 2025

Energy profiling tools: Variorum-based measurement tool #302

Draft

ethan-puyaubreau force-pushed the feature/energy-profiler-daemon branch from 4039559 to 10c87de Compare August 18, 2025 13:30

ethan-puyaubreau mentioned this pull request Aug 18, 2025

NVML and Variorum based energy measurement tool for Kokkos #295

Draft

ethan-puyaubreau marked this pull request as ready for review August 19, 2025 18:16

ethan-puyaubreau mentioned this pull request Aug 19, 2025

Energy profiling tools: Core infrastructure with timing tool and export capabilities #299

Open

ethan-puyaubreau force-pushed the feature/energy-profiler-daemon branch 4 times, most recently from 2c20e1f to 08e5d24 Compare August 28, 2025 19:01

JBludau reviewed Aug 28, 2025

View reviewed changes

JBludau approved these changes Aug 29, 2025

View reviewed changes

ethan-puyaubreau changed the title ~~Energy profiling tools: Daemon system~~ Energy profiling tools: Add Daemon class for periodic task execution Aug 29, 2025

DaKerboul added 4 commits August 29, 2025 14:26

Add Daemon class for managing periodic task execution

3b8efed

Refactor Daemon::tick

d47feef

Rename variable Daemon::run method

5b58534

Move files to upper folder

9458838

ethan-puyaubreau force-pushed the feature/energy-profiler-daemon branch from 180df12 to 9458838 Compare August 29, 2025 18:26

maartenarnst reviewed Sep 1, 2025

View reviewed changes

Comment thread profiling/energy-profiler/daemon.cpp

maartenarnst reviewed Sep 1, 2025

View reviewed changes

JBludau added 2 commits September 17, 2025 14:11

alphabetical order of includes

626f777

switch to chrono::duration

646bf35

maartenarnst reviewed Sep 18, 2025

View reviewed changes

Comment thread profiling/energy-profiler/daemon.cpp Outdated

maartenarnst reviewed Sep 18, 2025

View reviewed changes

Comment thread profiling/energy-profiler/daemon.hpp Outdated

maartenarnst reviewed Sep 18, 2025

View reviewed changes

Comment thread profiling/energy-profiler/daemon.hpp Outdated

maartenarnst reviewed Sep 18, 2025

View reviewed changes

Comment thread profiling/energy-profiler/daemon.hpp Outdated

JBludau and others added 4 commits September 18, 2025 09:41

Update profiling/energy-profiler/daemon.hpp

d26e43d

Co-authored-by: Maarten Arnst <maarten.arnst@ulg.ac.be>

Update profiling/energy-profiler/daemon.hpp

008bfcb

Co-authored-by: Maarten Arnst <maarten.arnst@ulg.ac.be>

Update profiling/energy-profiler/daemon.hpp

a25f2b7

Co-authored-by: Maarten Arnst <maarten.arnst@ulg.ac.be>

Update profiling/energy-profiler/daemon.cpp

9dd32a0

Co-authored-by: Maarten Arnst <maarten.arnst@ulg.ac.be>

aprokop reviewed Oct 10, 2025

View reviewed changes

Update profiling/energy-profiler/daemon.hpp

da422c5

Co-authored-by: Andrey Prokopenko <andrey.prok@gmail.com>

aprokop approved these changes Oct 10, 2025

View reviewed changes

fix formatting

04e6724

vlkale reviewed Dec 3, 2025

View reviewed changes

vlkale added the feature Needed feature but software still is correct on its own label Jan 6, 2026

vlkale requested review from crtrott and dalg24 February 27, 2026 19:35

dalg24 approved these changes Mar 5, 2026

View reviewed changes

dalg24 merged commit 49ffd61 into kokkos:develop Mar 5, 2026
7 checks passed

Conversation

ethan-puyaubreau commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JBludau left a comment

Choose a reason for hiding this comment

Uh oh!

JBludau commented Aug 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vlkale Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maartenarnst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maartenarnst commented Sep 18, 2025

Uh oh!

JBludau commented Sep 18, 2025

Uh oh!

aprokop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vlkale commented Dec 2, 2025

ethan-puyaubreau commented Aug 15, 2025 •

edited

Loading

vlkale Dec 3, 2025 •

edited

Loading