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

[Langer, Christoph]

Hi Thomas,

are you ok with the change as it is right now?

http://cr.openjdk.java.net/~clanger/webrevs/8196565.1/

If yes, then I'd push it...

Best regards
Christoph

> -----Original Message-----
> From: Langer, Christoph
> Sent: Freitag, 2. Februar 2018 15:17
> To: 'Thomas Stüfe' <thomas.stuefe at gmail.com>
> Cc: hotspot-runtime-dev at openjdk.java.net; ppc-aix-port-
> dev at openjdk.java.net
> Subject: RE: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to
> AIX/PASE specification
> 
> 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
>