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

Thu May 18 18:39:47 UTC 2017

Okay, I regenerated the generated-configure.sh and the double definitions
for -DSUPPORTS_CLOCK_MONOTONIC disappeared. So the generated-configure.sh
you posted was just outdated.

However, the debug output still appears twice: configure: No
CLOCK_GETTIME_IN_LIBRT

AC_MSG_NOTICE([No CLOCK_GETTIME_IN_LIBRT]) expands to two invocations of
ac_echo(..). I am out of my depth here, not sure what the background is.
But as you want to remove the debug output anyway, I think this is not an
issue.

I will take more time for a full review later. Just adding that I really
like this fix, it takes a lot of coding off our (platform specific) backs,
which is a good thing!

Kind Regards, Thomas

On Thu, May 18, 2017 at 4:40 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

> Hi David, Magnus,
>
> compiles and works fine on AIX, but as mentioned before off-list to David
> I see this stdout:
>
> configure: No CLOCK_GETTIME_IN_LIBRT
> configure: No CLOCK_GETTIME_IN_LIBRT
>
> Also, the -DSUPPORTS_CLOCK_MONOTONIC appears twice on the command line.
> Full compile command looks like this:
>
> /bin/xlC_r -q64 -qpic -D_REENTRANT -D__STDC_FORMAT_MACROS
> -DSUPPORTS_CLOCK_MONOTONIC -DSUPPORTS_CLOCK_MONOTONIC -DAIX -qtune=balanced
> -qalias=noansi -qstrict -qtls=default -qlanglvl=c99vla -qlanglvl=noredefmac
> -qnortti -qnoeh -qignerrno -qarch=ppc64 -DASSERT -DTARGET_ARCH_ppc
> -DINCLUDE_SUFFIX_OS=_aix -DINCLUDE_SUFFIX_CPU=_ppc -DTARGET_COMPILER_xlc
> -DPPC64 -DHOTSPOT_LIB_ARCH='"ppc64"' -DCOMPILER1 -DCOMPILER2
> -DINCLUDE_JVMCI=0 -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/share/vm
> -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/os/aix/vm
> -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/os/posix/vm
> -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/cpu/ppc/vm
> -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/os_cpu/aix_ppc/vm
> -I/priv/d031900/openjdk/jdk10-hs/output/hotspot/variant-server/gensrc
> -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/share/vm/precompiled
> -I/priv/d031900/openjdk/jdk10-hs/source/hotspot/src/share/vm/prims
> -DDONT_USE_PRECOMPILED_HEADER -g -qsuppress=1540-0216 -qsuppress=1540-0198
> -qsuppress=1540-1090 -qsuppress=1540-1639 -qsuppress=1540-1088
> -qsuppress=1500-010 -O3 -qhot=level=1 -qinline -qinlglue
> -DTHIS_FILE='"os_posix.cpp"' -c -qmakedep=gcc -MF
> /priv/d031900/openjdk/jdk10-hs/output/hotspot/variant-server/libjvm/objs/os_posix.d
> -o /priv/d031900/openjdk/jdk10-hs/output/hotspot/variant-server/libjvm/objs/os_posix.o
> /priv/d031900/openjdk/jdk10-hs/source/hotspot/src/os/posix/vm/os_posix.cpp
>
>  -DSUPPORTS_CLOCK_MONOTONIC is the only switch appearing twice. I'm
> baffled. Do you have any idea?
>
> Regards, Thomas
>
>
> On Thu, May 18, 2017 at 12:06 PM, Magnus Ihse Bursie <
> magnus.ihse.bursie at oracle.com> 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.
>>
>> 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
>>>>>
>>>>
>>>>
>>
>