On Tue, 24 Feb 2026 11:48:35 GMT, r1viollet <[email protected]> wrote:

>> For more information. see:
>> 
>> https://bugs.openjdk.org/browse/JDK-8372760
>> 
>> Testing: tier1 + test/jdk/jdk/jfr + manual testing
>> 
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> src/hotspot/share/jfr/periodic/jfrPeriodic.cpp line 103:
> 
>> 101: 
>> 102: TRACE_REQUEST_FUNC(JVMInformation) {
>> 103:   ResourceMark rm;
> 
> I see we had a ResourceMark in previous implementation, do we still need this 
> or none of the new function calls use this type of allocation ?

ResourceMark can be removed. Everything for the event is now heap-allocated.

> src/hotspot/share/jfr/periodic/jfrPeriodic.cpp line 259:
> 
>> 257:     while (processes != nullptr) {
>> 258:       SystemProcess* tmp = processes;
>> 259:       const char* info = processes->command_line();
> 
> The intent of the change here is to remove command line arguments for other 
> processes.
> I understand that these are likely to contain sensitive data, though I did 
> not see a mention of this change in the 
> [issue](https://bugs.openjdk.org/browse/JDK-8372760)

Plan is to make that change separately and integrate before this PR and JEP.

> src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp line 376:
> 
>> 374:   delete flags_args;
>> 375: 
>> 376:   _initialized = true;
> 
> 💭 does the `_initialized` flag need to be an atomic ?

We synchronize JVM.emit(...) before calling into native code to emit an event, 
so it should be fine.

> src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp line 589:
> 
>> 587:     return false;
>> 588:   }
>> 589:   char buffer[512];
> 
> 💭 We could add a log if we exceed the size of the buffer

I will use a stringstream instead and avoid the limit.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2849682254
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2849682510
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2849682042
PR Review Comment: https://git.openjdk.org/jdk/pull/29736#discussion_r2849686136

Reply via email to