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

[Thomas Stüfe]

This looks fine. Yes, push it.

..Thomas

On Wed, Feb 7, 2018 at 3:45 PM, Langer, Christoph <christoph.langer at sap.com>
wrote:

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