RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v16]

Tyler Steele duke at openjdk.java.net
Mon Jan 31 23:10:14 UTC 2022


On Sat, 29 Jan 2022 06:30:01 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Tyler Steele has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Changes macoss -> macosx in problem list
>>  - Refactors loadlib_aix: Removes redundant c++ class
>
> src/hotspot/os/aix/os_perf_aix.cpp line 72:
> 
>> 70: 
>> 71: static bool initialize_libperfstat() {
>> 72:   static bool is_libperfstat_loaded = false;
> 
> I don't think this is needed. libperfstat is initialized as part of os::init in os_aix.cpp.
> 
> https://github.com/openjdk/jdk/blob/d366d15d67a08833d93a5806edef8145cb7803e5/src/hotspot/os/aix/os_aix.cpp#L2999-L3008
> https://github.com/openjdk/jdk/blob/d366d15d67a08833d93a5806edef8145cb7803e5/src/hotspot/os/aix/os_aix.cpp#L2329-L2333
> 
> Should be already active at this point. Its functions are stubbed out in case the library cannot be loaded, so you should be able to call them in here, without an additional init check. You just have to handle the errors on the individual API calls, as if the real APIs returned errors. But you do this already I think.
> 
> Long term, if you think libperfstat should be always there, we can simplify things and treat libperfstat load errors as hard VM errors in release builds too. We already assert in `os::Aix::initialize_libperfstat()`. (Note: "assert" fires in debug builds (ASSERT macro), and "guarantee" fires in all builds).

Aha! I saw the definition of initialize_libperfstat(), but not that it was later called in os::init(). It did always feel strange to manually initialize libperfstat as I was doing. Thanks for pointing this out.

> src/hotspot/os/aix/os_perf_aix.cpp line 611:
> 
>> 609: 
>> 610:   // calling perfstat_<subsystem>(NULL, NULL, _, 0) returns number of available records
>> 611:   if ((n_records = libperfstat::perfstat_netinterface(NULL, NULL, sizeof(perfstat_netinterface_t), 0)) <= 0) {
> 
> is n=0 an error? Could there conceivably be zero interfaces?

Good point. I agree it's not reasonable to return an error if there are simply no network interfaces.

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

PR: https://git.openjdk.java.net/jdk/pull/6885



More information about the build-dev mailing list