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

[Magnus Ihse Bursie]

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.

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
>>