RFR: 8087322: Implement a Semaphore utility class

[Kim Barrett]

On Jun 19, 2015, at 4:43 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> On 19/06/2015 4:50 PM, Kim Barrett wrote:
>> On Jun 18, 2015, at 9:27 PM, David Holmes <david.holmes at oracle.com> wrote:
>>> One thing I am tempted to clean up a little more is the timedwait call.
>> 
>> I think that can likely be made common to all non-Apple posix systems, […].  All those posix variants appear to support clock_gettime of CLOCK_REALTIME, at least according to their documentation.  […]
> 
> Not that although clock_getime is supported by all POSIX systems:
> 
> a) we don't use it anywhere in Solaris code (conversion to POSIX APIs is a long standing wishlist item)

Right, though I'm hoping it isn't a problem to use clock_gettime in
(hopefully) shared POSIX code that happens to be used by the Solaris
ports.

> b) we dynamically lookup clock_gettime on Linux. Again this is historical and unnecessary given the supported platforms, but a cleanup that is yet to be implemented.

Yes, I saw that.  I'm hoping that problem is sufficiently ancient that
it can be ignored.  Tracing through the bug reports, that doesn't seem
unreasonable: 6348968 -> 6336247 -> 4885046 leads to a discussion
about it being a bug in Redhat 9 when using NPTL, and that it had been
fixed in upstream NPTL but that fix hadn't yet made it's way into RH9
(and apparently not into RH AS4 either at that point).

Just in general, there seems to be a lot of accumulated workarounds
and possibly unnecessary code duplication in the unixy os_xxx.cpp
variants (and possibly other files nearby that I haven't looked at).
I was talking to Coleen yesterday about this, and she said she's been
thinking it might be time to send a cleanup crew in there.  Merging
the semaphores would help with that; it's something that would
probably have been done as part of such a cleanup anyway.

>>> The sr_semaphore appears to be being initialized by C++ static initialization, which is a concern for me, primarily because Semaphore construction requires allocation support (it's a CHeapObj) and utilizes NMT. I get very nervous when NMT gets involved prior to VM initialization. Can we make sr_semaphore a pointer instead and initialize it in one of the os::init methods?
>> 
>> I think that as long as we’re not allocating any CHeapObj subobjects, that isn’t a problem.  The fact that it is a CHeapObj permits, but does not require, allocation using the CHeapObj allocator.  At least, I think that’s true.  Certainly it won’t be allocated using its associated operator new, so I don’t see how NMT could get involved.
> 
> OK. I'm not a fan of using C++ based initialization for any non-trivial classes we define.

You of course mean "static initialization".  And I agree; static
initialization can cause a lot of problems.  I don't think we were
planning on changing anything other than the type of the
sr_semaphore's unless we had to though, leaving that sort of cleanup
to other tasks.  But that's a good reminder; at least one of the
variants Stefan came up would probably run into problems here.

>>> Can we not implement this:
>>> 
>>> 53   void signal();
>>> 54   void signal(uint count);
>>> 
>>> as simply:
>>> 
>>>  void signal(uint count = 1);
>> 
>> I think it ends up being more convenient in the implementation to have them be separate.  If that ultimately turns out not to be the case then we can change it.  It doesn’t affect the client API at all (unless someone starts using pointers to member functions).
> 
> Ah yes I see that now = signal(n) is n calls to signal().

Except that for Windows the natural implementation is the other way
around.  But for the POSIX variants, we can have one signal(n) that
calls the platform-specific (Apple semaphore_t or POSIX sem_t)
signal().