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

Langer, Christoph christoph.langer at sap.com
Fri Feb 2 14:16:35 UTC 2018


Hi Thomas,

thanks for the review.

Here is an update: http://cr.openjdk.java.net/~clanger/webrevs/8196565.1/

> 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.

Ok, sorry. This time I have an updated webrev in a new location ;-)

> 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?
> 
> Sorry for the rant.

Yes, the IBM doc is a bit contradicting. Maybe you are right, we should do it like before for AIX...

> 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?

The main motivation was to fix PASE and to get this aligned with OpenJDK.

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

Hm, the only thing way to assert monotonicity was to do CAS operations on AIX as well...
We had some code like that in the SAP JVM before but I rather decided to throw it out
there because I would like to trust mread_real_time specs for AIX.
Additional CAS operations could mean a performance impact.

> - I would leave George's name out of the sources.
You're right - I did so already in my in-place update :)

> - 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.

I compared to the code for BSD and solaris and want to keep it similar.
So I rename it to max_real_time. But I would like to keep the variable
declaration in the beginning of the file where local variables are declared.

I'm submitting the updated version of the code to our SAP JVM depot and
then we'll get test results for various AIX and PASE runtimes and can see if
we hit one of the asserts.

Thanks and best regards
Christoph




More information about the hotspot-runtime-dev mailing list