On Wed, 4 Jun 2025 08:40:34 GMT, Markus Grönlund <mgron...@openjdk.org> wrote:
>> src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.cpp line 606:
>>
>>> 604: void JfrCPUTimeThreadSampler::init_timers() {
>>> 605: // install sig handler for sig
>>> 606: PosixSignals::install_generic_signal_handler(SIG,
>>> (void*)::handle_timer_signal);
>>
>> SIGPROF is also used by external profilers. Need to check if SIGPROF handler
>> is already installed and warn user.
>
> This is *very* important to have a robust failure mechanism when existing
> handlers are already installed. Why? JFR can be turned on dynamically from
> the outside, at any time, during runtime. A lot of agents could have
> installed their handlers by then.
>
> Please describe how you intend to handle the case where someone starts JFR
> late during runtime and the signal handler cannot be installed.
I added a log_error to tell the user
>> src/hotspot/share/jfr/periodic/sampling/jfrCPUTimeThreadSampler.hpp line 128:
>>
>>> 126: static void send_lost_event(const JfrTicks& time, traceid tid, s4
>>> lost_samples);
>>> 127:
>>> 128: // Trigger sampling while a thread is not in a safepoint, from a
>>> seperate thread
>>
>> typo: separate
>
> And again, its not "sampling" that is triggered. It is async processing of
> the queue holding existing samples.
I removed the comment, as the method name itself is pretty self-explanatory.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126330036
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126332382
