RFR: 8136978 Much nearly duplicated code for vmError support (original) (raw)

Sebastian Sickelmann sebastian.sickelmann at gmx.de
Wed Nov 18 10:02:56 UTC 2015

Hi Thomas,

thanks for heaving a look at this patch. I will try to work in your suggestions and rebase it on an actual tip.

What repository is the right one? I think the hs-rt ?!??!

-- Sebastian

On 11/18/2015 10:52 AM, Thomas Stüfe wrote:

Hi Sebastian,

Your patch fails to apply on my repository. I tested against a freshly pulled jdk9/dev. I think it may conflict with "8080775: Better argument formatting for assert() and friends". -------------- From looking at it, this change is a nice cleanup! Here are some few cosmetic issues: - os::formatDebugMessage() and void os::startDebugging(): Standard naming scheme would be lowercase with underscores. Could you rename them "formatdebugmessage" and "startdebugging", respectivly? - http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/src/os/posix/vm/osposix.hpp.udiff.html <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/src/os/posix/vm/osposix.hpp.udiff.html> Could you please group static int unblocksignals(const sigsett *set); with the other signal set functions above and maybe also add a comment? --- Also, I would like to see in the name of "unblocksignals" reflected that this is thread-specific, e.g. "unblockthreadsignals". Or even better, a more generic function which both blocks and unblocks signals for the calling thread, e.g. "os::Posix::setthreadsignalmask(const sigsett *set, bool doblock)". --- Kind Regards, Thomas

On Fri, Oct 16, 2015 at 6:33 AM, Sebastian Sickelmann <sebastian.sickelmann at gmx.de <mailto:sebastian.sickelmann at gmx.de>> wrote: Hi, i have looked at the enhancement JDK-8136978, please find my first suggestion at [0] http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/ <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/>. I first looked also at another solution but don't liked it. For some details on this and comments of Coleen and Kim to this, see the thread at hotspot-dev[1]. Right now i only compile-"tested" it on my linux(x8664) machine, so there might be some error in aix,bsd,solaris. I am not sure if i can setup my machine to compile those as well. For the windows-part i am actually also not able to compile or test it. I think there is a good change to optimize the use of #include's in the changed files, but I am not sure how i can effectively work out where i can reduce some imports. There are no additional tests for this. What is the best way to really test such a change on all platforms. Do you use your development-machine for this, or is there some infrastructure that can test such multi-platform changes for you? Here is a short description of the suggested change: Nearly identically implementations of VMError moved from the os/[linux|aix|bsd|solaris] to a os/posix. The parts that are different were refactored and are now in the os-specific implementations of the os class. The two os specific methods ucontextgetpc and ucontextsetpc are moved to the declaration of the os::Posix class. The implementations of those remain in the os[linux|aix|bsd|solaris].cpp implementations but are renamed acordingly. All uses of these methods are replaces to use the "os::Poxis prefix". For the method VMError::showmessagebox also the windows implementation is changed. Now there are two methods in the declarartion of the class os that are used to help the os-independent implementation of showmessage. The two messages are named formatDebugMessage and startDebugging. The os-independet implemetation of showmessage can be found in share/vm/utilities/vmError.cpp -- Sebastian [0] http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/ <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/> [1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-October/020249.html suggestion to this.

More information about the hotspot-runtime-dev mailing list