[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR: 8203290: [PPC64, s390] Check functionality of JDK-8199712 (Flight Recorder) [v16]
From:       Tyler Steele <duke () openjdk ! java ! net>
Date:       2022-01-31 23:10:14
Message-ID: JqNKLgTRv6jNQMN1Ae9JHJNebmzt0rPql6JU9L6izs0=.e130d249-51dc-44f0-a147-2385bd81c996 () github ! com
[Download RAW message or body]

On Sat, 29 Jan 2022 06:30:01 GMT, Thomas Stuefe <stuefe@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


[prev in list] [next in list] [prev in thread] [next in thread]