RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification
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
Hi Christoph, maybe it would be better to check the return code of time_base_to_time, the doc at https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.basetr... says : "The time_base_to_time subroutine returns 0 if the conversion to real time is successful (or not necessary), otherwise -1 is returned." So errors might (at least in theory) occur . (otherwise it looks good to me, not a Reviewer however) Best regards, Matthias From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounces@openjdk.java.net] On Behalf Of Langer, Christoph Sent: Donnerstag, 1. Februar 2018 10:11 To: hotspot-runtime-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Subject: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification 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
Hi Matthias, thanks for looking at this. I updated my coding (webrev in place) to add back the assertion and I also updated the CAS coding to the one that's used on other platforms such as Solaris and BSD/MacOS. Best regards Christoph From: Baesken, Matthias Sent: Donnerstag, 1. Februar 2018 10:27 To: Langer, Christoph <christoph.langer@sap.com>; hotspot-runtime-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Subject: RE: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification Hi Christoph, maybe it would be better to check the return code of time_base_to_time, the doc at https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.aix.basetr... says : "The time_base_to_time subroutine returns 0 if the conversion to real time is successful (or not necessary), otherwise -1 is returned." So errors might (at least in theory) occur . (otherwise it looks good to me, not a Reviewer however) Best regards, Matthias From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounces@openjdk.java.net] On Behalf Of Langer, Christoph Sent: Donnerstag, 1. Februar 2018 10:11 To: hotspot-runtime-dev@openjdk.java.net<mailto:hotspot-runtime-dev@openjdk.java.net>; ppc-aix-port-dev@openjdk.java.net<mailto:ppc-aix-port-dev@openjdk.java.net> Subject: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification 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
Hi Christoph, On 1/02/2018 7:11 PM, Langer, Christoph 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/
I took a look as I was curious :) I recognized the Solaris code for ensuring monotonic time. :) It all seems to do as you describe. One nit - instead of (1000 * 1000 * 1000) you can use: const jlong NANOSECS_PER_SEC = CONST64(1000000000); from ./hotspot/share/utilities/globalDefinitions.hpp Cheers, David
Thanks & Best regards Christoph
Hi David, thanks for looking. I'll change my code to use that constant. One question, as I'm currently looking at the os timing functions: Do you know why in os_solaris.cpp, in os::getTimesSecs(), the process_real_time value is not taken from the result of times() but from getTimeNanos()? Otherwise getTimeSecs would be a good candidate to move to os_posix.cpp ... Best regards Christoph -----Original Message----- From: David Holmes [mailto:david.holmes@oracle.com] Sent: Donnerstag, 1. Februar 2018 12:10 To: Langer, Christoph <christoph.langer@sap.com>; hotspot-runtime-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Subject: Re: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification Hi Christoph, On 1/02/2018 7:11 PM, Langer, Christoph 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/
I took a look as I was curious :) I recognized the Solaris code for ensuring monotonic time. :) It all seems to do as you describe. One nit - instead of (1000 * 1000 * 1000) you can use: const jlong NANOSECS_PER_SEC = CONST64(1000000000); from ./hotspot/share/utilities/globalDefinitions.hpp Cheers, David
Thanks & Best regards Christoph
Hi Christoph, On 1/02/2018 10:06 PM, Langer, Christoph wrote:
Hi David,
thanks for looking. I'll change my code to use that constant.
One question, as I'm currently looking at the os timing functions: Do you know why in os_solaris.cpp, in os::getTimesSecs(), the process_real_time value is not taken from the result of times() but from getTimeNanos()? Otherwise getTimeSecs would be a good candidate to move to os_posix.cpp ...
I can only guess that as per the comment the intent was to ensure that "process_real_time" was consistent with other views of time ie os::elapsedTime and os::elapsedCounter. The code was added by: https://bugs.openjdk.java.net/browse/JDK-6468292 but there are no enlightening comments in that regard. And I don't know if times() would be using a different timebase than gethrtime(). David -----
Best regards Christoph
-----Original Message----- From: David Holmes [mailto:david.holmes@oracle.com] Sent: Donnerstag, 1. Februar 2018 12:10 To: Langer, Christoph <christoph.langer@sap.com>; hotspot-runtime-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Subject: Re: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification
Hi Christoph,
On 1/02/2018 7:11 PM, Langer, Christoph 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/
I took a look as I was curious :) I recognized the Solaris code for ensuring monotonic time. :) It all seems to do as you describe.
One nit - instead of (1000 * 1000 * 1000) you can use:
const jlong NANOSECS_PER_SEC = CONST64(1000000000);
from ./hotspot/share/utilities/globalDefinitions.hpp
Cheers, David
Thanks & Best regards Christoph
On 1/02/2018 11:17 PM, David Holmes wrote:
Hi Christoph,
On 1/02/2018 10:06 PM, Langer, Christoph wrote:
Hi David,
thanks for looking. I'll change my code to use that constant.
One question, as I'm currently looking at the os timing functions: Do you know why in os_solaris.cpp, in os::getTimesSecs(), the process_real_time value is not taken from the result of times() but from getTimeNanos()? Otherwise getTimeSecs would be a good candidate to move to os_posix.cpp ...
I can only guess that as per the comment the intent was to ensure that "process_real_time" was consistent with other views of time ie os::elapsedTime and os::elapsedCounter.
The code was added by:
https://bugs.openjdk.java.net/browse/JDK-6468292
but there are no enlightening comments in that regard. And I don't know if times() would be using a different timebase than gethrtime().
Actually I think it is also because the return value from times() may not be usable due to the potential overflow problem. This is true on other platforms too. Linux has an even bigger disclaimer: On Linux, the "arbitrary point in the past" from which the return value of times() is measured has varied across kernel versions. On Linux 2.4 and earlier this point is the moment the system was booted. Since Linux 2.6, this point is (2^32/HZ) - 300 (i.e., about 429 million) seconds before system boot time. This variability across kernel versions (and across UNIX implementations), combined with the fact that the returned value may overflow the range of clock_t, means that a portable application would be wise to avoid using this value. To measure changes in elapsed time, use gettimeofday(2) instead. -- David
David -----
Best regards Christoph
-----Original Message----- From: David Holmes [mailto:david.holmes@oracle.com] Sent: Donnerstag, 1. Februar 2018 12:10 To: Langer, Christoph <christoph.langer@sap.com>; hotspot-runtime-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Subject: Re: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification
Hi Christoph,
On 1/02/2018 7:11 PM, Langer, Christoph 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/
I took a look as I was curious :) I recognized the Solaris code for ensuring monotonic time. :) It all seems to do as you describe.
One nit - instead of (1000 * 1000 * 1000) you can use:
const jlong NANOSECS_PER_SEC = CONST64(1000000000);
from ./hotspot/share/utilities/globalDefinitions.hpp
Cheers, David
Thanks & Best regards Christoph
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@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
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
Hi Thomas, sorry, I had to do one little in-place upgrade as I incorporated David's suggestion to use NANOSECS_PER_SEC instead of (1000 * 1000 * 1000)... Hope you weren't reviewing at that very moment ;-) Best regards Christoph
-----Original Message----- From: Langer, Christoph Sent: Freitag, 2. Februar 2018 15:17 To: 'Thomas Stüfe' <thomas.stuefe@gmail.com> Cc: hotspot-runtime-dev@openjdk.java.net; ppc-aix-port- dev@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
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@gmail.com> Cc: hotspot-runtime-dev@openjdk.java.net; ppc-aix-port- dev@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
This looks fine. Yes, push it. ..Thomas On Wed, Feb 7, 2018 at 3:45 PM, Langer, Christoph <christoph.langer@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@gmail.com> Cc: hotspot-runtime-dev@openjdk.java.net; ppc-aix-port- dev@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
Thanks, done: http://hg.openjdk.java.net/jdk/hs/rev/45b6aae769cc I forgot to mention that I had added this code to the SAP JVM a few days ago and test showed no problems… From: Thomas Stüfe [mailto:thomas.stuefe@gmail.com] Sent: Mittwoch, 7. Februar 2018 15:53 To: Langer, Christoph <christoph.langer@sap.com> Cc: hotspot-runtime-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net Subject: Re: RFR(S): 8196565: AIX: Clean up os::javaTimeNanos according to AIX/PASE specification This looks fine. Yes, push it. ..Thomas On Wed, Feb 7, 2018 at 3:45 PM, Langer, Christoph <christoph.langer@sap.com<mailto:christoph.langer@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@gmail.com<mailto:thomas.stuefe@gmail.com>> Cc: hotspot-runtime-dev@openjdk.java.net<mailto:hotspot-runtime-dev@openjdk.java.net>; ppc-aix-port- dev@openjdk.java.net<mailto:dev@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
participants (4)
-
Baesken, Matthias
-
David Holmes
-
Langer, Christoph
-
Thomas Stüfe