On Fri, 13 Dec 2024 12:18:13 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:
>> We should output more information about the JVMTI agents in the hserr file.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> simplify coding
Looks fine, two bikeshedding nits.
I approve now, up to you if you take my suggestions.
src/hotspot/share/runtime/os.cpp line 1133:
> 1131: const JvmtiAgent* agent = it.next();
> 1132: if (agent != nullptr) {
> 1133: if (first_agent) st->print_cr("JVMTI agents:");
if no agents are loaded, I would print "JVMTI agents: none" unconditionally.
Makes it more obvious than just a missing entry; that could be also an error.
src/hotspot/share/runtime/os.cpp line 1138:
> 1136: const char* instrumentinfo = agent->is_instrument_lib() ?
> "instrumentlib" : "";
> 1137: const char* loadinfo = agent->is_loaded() ? "loaded" : "not
> loaded";
> 1138: const char* initinfo = agent->is_initialized() ? "initialized" :
> "not initialized";
I would just print these as flags, its clearer and simpler to print. E.g.
"myagent path: mylib.so dynamic:1 loaded:1 instrument_lib:0 initialized:1 xxx
options xxx"
-------------
Marked as reviewed by stuefe (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22706#pullrequestreview-2502573370
PR Review Comment: https://git.openjdk.org/jdk/pull/22706#discussion_r1884116576
PR Review Comment: https://git.openjdk.org/jdk/pull/22706#discussion_r1884113414
