Replying to all this time :)
On 19/05/2017 7:15 PM, Magnus Ihse Bursie wrote:
On 2017-05-19 09:15, David Holmes wrote:
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.
Code looks good!
Thanks for the re-review.
In the future, I very much prefer if you do not update webrevs in place.
It's hopeless if you start reading a thread after some updates have
occured, the mails don't make any sense, and it's hard to follow
after-the-fact how the patch evolved.
Sorry. Point taken.
David
-----
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!
That's good to know.
/Magnus
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
