RFR: 8136978 Much nearly duplicated code for vmError support

[Sebastian Sickelmann]

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 "format_debug_message" and "start_debugging", respectivly?
>
> -
> http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/src/os/posix/vm/os_posix.hpp.udiff.html
> <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/src/os/posix/vm/os_posix.hpp.udiff.html>
>
> Could you please group
>
>   static int unblock_signals(const sigset_t *set);
>
> with the other signal set functions above and maybe also add a comment?
>
> ---
>
> Also, I would like to see in the name of "unblock_signals" reflected
> that this is thread-specific, e.g. "unblock_thread_signals". 
>
> Or even better, a more generic function which both blocks and unblocks
> signals for the calling thread, e.g.
> "os::Posix::set_thread_signal_mask(const sigset_t *set, bool do_block)". 
>
> ---
>
> 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(x86_64) 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
>     ucontext_get_pc and
>     ucontext_set_pc 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::show_message_box 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 show_message. The two messages
>     are named formatDebugMessage
>     and startDebugging. The os-independet implemetation of
>     show_message 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.
>
>
>