On Wed, 4 Jun 2025 11:23:57 GMT, Markus Grönlund <mgron...@openjdk.org> wrote:
>> Johannes Bechberger has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 127 commits: >> >> - Merge branch 'master' into parttimenerd_cooperative_cpu_time_sampler >> - Add error message on signal handler install failure >> - Fix signal handler synchronization >> - Improve >> - Rename autoadapt >> - Make process_cpu_time_request private and move up >> - Reorder condition >> - Tiny refactoring >> - Restrict threads for which timers are created >> - Fix tiny mistake >> - ... and 117 more: https://git.openjdk.org/jdk/compare/7838321b...4fd4b673 > > src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 647: > >> 645: // install sig handler for sig >> 646: if ((s8)PosixSignals::install_generic_signal_handler(SIG, >> (void*)::handle_timer_signal) == -1) { >> 647: log_error(jfr)("Failed to install signal handler for CPU thread >> sampling, possibly because another profiler is active: %s", >> os::strerror(os::get_last_error())); > > That we are using a signal handler to provide the user with CPU time > information is an implementation detail. Its good to provide an error > message, but I think it should reflect back on something the user is > expecting. > > Perhaps add a line that says something along the lines of "CPUTimeSample > events will not be recorded." Done ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126360801