(10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems

[David Holmes]

Hi Magnus,

On 18/05/2017 8:06 PM, Magnus Ihse Bursie wrote:
> 
> 
> On 2017-05-18 09:35, David Holmes wrote:
>> On 18/05/2017 5:32 PM, Magnus Ihse Bursie wrote:
>>> On 2017-05-18 08:25, David Holmes wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8174231
>>>>
>>>> webrevs:
>>>>
>>>> Build-related: http://cr.openjdk.java.net/~dholmes/8174231/webrev.top/
>>>
>>> Build changes look  good.
>>
>> Thanks Magnus! I just realized I left in the AC_MSG_NOTICE debugging 
>> prints outs - do you want me to remove them? I suppose they may be 
>> useful if something goes wrong on some platform.
> 
> I didn't even notice them. :-/
> 
> It's a bit unfortunate we don't have a debug level on the logging from 
> configure. :-( Otherwise they would have clearly belonged there.
> 
> The AC_MSG_NOTICE messages stands out much from the rest of the 
> configure log, so maybe it's better that you remove them. The logic 
> itself is very simple, if the -D flags are missing then we can surely 
> tell what happened. So yes, please remove them.

Webrev updated in place.

I have removed them to avoid noise - particularly as they get executed 
twice.

I also made an adjustment to AC_SEARCH_LIBS as I don't want to pass the 
saved_LIBS value in explicitly, but want it to use the LIBS value - 
which is no longer cleared before the call. I've verified all platforms 
are okay - except AIX which I'll need Thomas to recheck when he can.

I also discovered an oddity in that our ARM64 builds seem to use 
different system libraries in that librt.so is not needed for 
clock_gettime. This still seems to work ok. Of more of a concern if we 
were expand this kind of function-existence check is that libc seems to 
contain "dummy" (or at least dysfunctional) versions of a number of the 
core pthread APIs!

Thanks,
David

> Alternatively, rewrite them as CHECKING/RESULT, if you want to keep the 
> logging. That way they match better the rest of the configure log (and 
> also describes what you're doing). Just check if AC_SEARCH_LIBS prints 
> some output (likely so, I think), then you can't do it in the middle of 
> a CHECKING/RESULT pair, but have to do the CHECKING part after 
> AC_SEARCH_LIBS.
> 
> /Magnus
> 
>>
>> David
>>
>>> /Magnus
>>>
>>>>       hotspot:
>>>> http://cr.openjdk.java.net/~dholmes/8174231/webrev.hotspot/
>>>>
>>>> First a big thank you to Thomas Stuefe for testing various versions of
>>>> this on AIX.
>>>>
>>>> This is primarily a refactoring and cleanup exercise (ie lots of
>>>> deleted duplicated code!).
>>>>
>>>> I have taken the PlatformEvent, PlatformParker and Parker::* code, out
>>>> of os_linux and moved it into os_posix for use by Linux, OSX, BSD, AIX
>>>> and perhaps one day Solaris (more on that later).
>>>>
>>>> The Linux code was the most functionally complete, dealing with
>>>> correct use of CLOCK_MONOTONIC for relative timed waits, and the
>>>> default wall-clock for absolute timed waits. That functionality is
>>>> not, unfortunately, supported by all our POSIX platforms so there are
>>>> some configure time build checks to set some #defines, and then some
>>>> dynamic lookup at runtime**. We allow for the runtime environment to
>>>> be less capable than the build environment, but not the other way
>>>> around (without build time support we don't know the runtime types
>>>> needed to make library calls).
>>>>
>>>> ** There is some duplication of dynamic lookup code on Linux but this
>>>> can be cleaned up in future work if we refactor the time/clock code
>>>> into os_posix as well.
>>>>
>>>> The cleanup covers a number of things:
>>>> - removal of linux anachronisms that got "ported" into the other
>>>> platforms
>>>>   - eg EINTR can not be returned from the wait methods
>>>> - removal of solaris anachronisms that got ported into the linux code
>>>> and then on to other platforms
>>>>   - eg ETIMEDOUT is what we expect never ETIME
>>>> - removal of the ancient/obsolete os::*::allowdebug_blocked_signals()
>>>> from the Parker methods
>>>> - consolidation of unpackTime and compute_abstime into one utility
>>>> function
>>>> - use statics for things completely private to the implementation
>>>> rather than making them part of the os* API (eg access to condAttr
>>>> objects)
>>>> - cleanup up commentary and style within methods of the same class
>>>> - clean up coding style in places eg not using Names that start with
>>>> capitals.
>>>>
>>>> I have not tried to cleanup every single oddity, nor tried to
>>>> reconcile differences between the very similar in places PlatformEvent
>>>> and Park methods. For example PlatformEvent still examines the
>>>> FilterSpuriousWakeups** flag, and Parker still ignores it.
>>>>
>>>> ** Perhaps a candidate for deprecation and future removal.
>>>>
>>>> There is one mini "enhancement" slipped in this. I now explicitly
>>>> initialize mutexes with a mutexAttr object with its type set to
>>>> PTHREAD_MUTEX_NORMAL, instead of relying on the definition of
>>>> PTHREAD_MUTEX_DEFAULT. On FreesBSD the default is not "normal" but
>>>> "error checking" and so is slow. On all other current platforms there
>>>> is no effective change.
>>>>
>>>> Finally, Solaris is excluded from all this (other than the debug
>>>> signal blocking cleanup) because it potentially supports three
>>>> different low-level sync subsystems: UI thr*, Pthread, and direct LWP
>>>> sync. Solaris cleanup would be a separate RFE.
>>>>
>>>> No doubt I've overlooked mentioning something that someone will 
>>>> spot. :)
>>>>
>>>> Thanks,
>>>> David
>>>
>