On Wed, 4 Jun 2025 03:07:52 GMT, Andrei Pangin <apan...@openjdk.org> wrote:
>> Johannes Bechberger has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Rename autoadapt
>
> 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.
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126029865
PR Review Comment: https://git.openjdk.org/jdk/pull/25302#discussion_r2126025355
