RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification

Thomas Stüfe thomas.stuefe at gmail.com
Thu Feb 1 14:43:53 UTC 2018


Hi Christoph,

thanks for the fix!

But please, could refrain from changing webrevs in place? At least if you
change large portions of the diff? I was reviewing the old one while it
changed under me, this was quite confusing. It also makes understanding the
mail thread difficult.

------

Why do you not handle the return value of mread_real_time() like we did
before on AIX?

.. Oh, I read the IBM documentation now... What is this?

<quote>
Return Values
The read_real_time subroutine returns RTC_POWER if the contents of the
real-time clock are recorded in the timebasestruct, or returns RTC_POWER_PC
if the content of the time base registers is recorded in the timebasestruct.
</quote>

It is assumed, though never explicitly stated, that a return value of
RTC_POWER_PC means we need to convert the returned value with
time_base_to_time(). But then it says:

<quote>
The time_base_to_time subroutine converts time base information to real
time, if necessary. It is **suggested** that applications unconditionally
call the time_base_to_time subroutine rather than conducting a check to see
whether it is necessary.
</quote>

The second quote directly contradicts the first. Either I have to honor the
return code of read_real_time() or I don't, in which case I have to always
convert the result. I also like the "it is suggested" wording, like it is
my choice :)

Then they also never explain the return code of mread_real_time(), but we
just assume the function works the same way as read_real_time() does, yes?
IBM really has the worst manpages :)

Sorry for the rant. But back to your change, we did handle the return code
of mread_real_time() before, and arguably the coding before was more
correct. Well, depending on how we feel about the suggestion :) So I am
hesitant to throw that old coding out. Does your change work on older AIX
releases and did you test on different Power platforms? Did we have an
actual problem on AIX, or is this fix just for PASE?

- I know mread_real_time() is supposed to return monotonic values on AIX,
but I would feel better were we to assert the fact.

- I would leave George's name out of the sources.

- Small nit, could you rename max_nanos to something clearer, e.g.
mread_last_value or similar? And maybe place it physically near that
function? Ideally I would put it inside that function but I am not sure if
the late initialization of then function-scope-static max_nanos would cause
problems.

Thanks! ..

Best Regards, Thomas


On Thu, Feb 1, 2018 at 10:11 AM, Langer, Christoph <christoph.langer at sap.com
> wrote:

> Hi,
>
> please review a fix for the os::javaTimeNanos method on AIX.
>
> It implements the information we got from the AIX documentation and from
> the IBM folks regarding the behavior of mread_real_time on Pase.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8196565
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8196565.0/
>
> Thanks & Best regards
> Christoph
>
>


More information about the hotspot-runtime-dev mailing list