RFR (S) 8130039: Move the platform-specific [OS]Semaphore code

[David Holmes]

Hi Coleen,

Overall this seems okay but a few comments ...

On 9/01/2018 8:34 AM, coleen.phillimore at oracle.com wrote:
> Also:
> 8130038: Unify the semaphore usage in os_xxx.cpp
> 
> Tested with hs-tier1-5.  And 
> open/test/jdk/sun/misc/SunMiscSignalTest.java and closed signal tests.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8130039.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8130039

With the removal of os::signal_lookup() the parameter to 
check_pending_signals becomes irrelevant and the "false" path becomes 
dead code. Not sure if it is worth deleting that.

---

os_linux.cpp:

static struct timespec create_timespec(unsigned int sec, int nsec) {
   struct timespec ts;
   // Semaphore's are always associated with CLOCK_REALTIME
   os::Linux::clock_gettime(CLOCK_REALTIME, &ts);
   // see unpackTime for discussion on overflow checking

There's no longer a direct connection between create_timespec and the 
fact it is only used with semaphores - so the initial comment is no 
longer appropriate. But if you rename to create_semaphore_timespec then 
that restores the connection.

The final comment is not correct (my bad!) as unpackTime no longer 
exists since I factored out all the PlatformEvent code. It could say:

// see os_posix.cpp for discussion on overflow checking

but I'm wondering whether the timedwait API should be taking a timespec 
in the first place? If we pass secs and nsecs directly then we only need 
to convert to a timespec inside the low-level implementation that needs 
it ie in semaphore_posix.cpp. I don't recall if there was a reason we 
have to do the timespec conversion in a platform specific manner.

---

semaphore_posix.cpp

   26 #ifndef __APPLE__
   27 #include "runtime/os.hpp"
   28 // POSIX unamed semaphores are not supported on OS X.
   29 #include "semaphore_posix.hpp"
   30 #include <semaphore.h>
   31 #endif
   32
   33
   34 // POSIX unamed semaphores are not supported on OS X.
   35 #ifndef __APPLE__

The whole file contents should be ifndef __APPLE__ so this just reduces to:

   // POSIX unamed semaphores are not supported on OS X.
   #ifndef __APPLE__
   #include "runtime/os.hpp"
   #include "semaphore_posix.hpp"
   #include <semaphore.h>
   ...

---

Thanks,
David

> Thanks,
> Coleen