(10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems
David Holmes
david.holmes at oracle.com
Thu May 18 21:39:29 UTC 2017
Hi Thomas,
On 19/05/2017 4:39 AM, Thomas Stüfe wrote:
> 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.
Not sure how but as long as it is fixed. :)
> However, the debug output still appears twice: configure: No
> CLOCK_GETTIME_IN_LIBRT
Yes that's a side-effect of the flag helper routine being called twice:
once for build platform and once for target.
> 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.
Right the messages will be gone.
> 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!
Thanks for looking at this in so much detail already.
Cheers,
David
> Kind Regards, Thomas
>
> On Thu, May 18, 2017 at 4:40 PM, Thomas Stüfe <thomas.stuefe at gmail.com
> <mailto: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
> <mailto: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
> <https://bugs.openjdk.java.net/browse/JDK-8174231>
>
> webrevs:
>
> Build-related:
> http://cr.openjdk.java.net/~dholmes/8174231/webrev.top/
> <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/
> <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
>
>
>
>
>
More information about the build-dev
mailing list