RFR: 8194860: Cleanup Semaphore timed-wait time calculations
Kim Barrett
kim.barrett at oracle.com
Mon Jan 21 08:42:31 UTC 2019
> On Jan 20, 2019, at 9:29 PM, David Holmes <david.holmes at oracle.com> wrote:
>
> Here's the updated webrev:
>
> http://cr.openjdk.java.net/~dholmes/8194860/webrev.v2
>
> Only changes are in os_posix.cpp:
>
> - As Coleen requested I statically initialized the various static variables to simplify the conditional compilation for Solaris and removed the dynamic initialization to NULL/false.
>
> - I had a bug (exposed on old OS X versions) where I was using clock_gettime even though it might not be available.
Looks good.
A few questions / comments about mostly pre-existing code.
------------------------------------------------------------------------------
src/hotspot/os/posix/os_posix.cpp
1630 static pthread_condattr_t _condAttr[1];
1634 static pthread_mutexattr_t _mutexAttr[1];
Pre-existing. Why are these one element arrays, rather than just
single objects?
------------------------------------------------------------------------------
src/hotspot/os/posix/os_posix.cpp
1735 int (*condattr_setclock_func)(pthread_condattr_t*, clockid_t) =
1736 (int (*)(pthread_condattr_t*, clockid_t))dlsym(RTLD_DEFAULT,
1737 "pthread_condattr_setclock");
1738 if (condattr_setclock_func != NULL) {
1739 _pthread_condattr_setclock = condattr_setclock_func;
1740 }
Pre-existing. Why the local condattr_setclock_func? Why not just
directly set _pthread_condattr_setclock from the dlsym lookup? This
isn't like the earlier case with _clock_gettime &etc, where there are
additional checks to be performed before accepting the library
function.
------------------------------------------------------------------------------
src/hotspot/os/posix/os_posix.hpp
136 static void to_RTC_abstime(timespec* abstime, int64_t millis);
I thought about suggesting returning a timespec rather than taking a
pointer to one as an argument. That would be more usual C++ style, and
would avoid any possibility of accidentally passing in a NULL pointer.
But it would require doing some research on the state of copy elision
in some compilers that I'm less familiar with. Doing so would also
suggest doing the same for to_abstime.
------------------------------------------------------------------------------
More information about the hotspot-runtime-dev
mailing list