RFR (S) 8182848: Some functions misplaced in debug.hpp

[coleen.phillimore at oracle.com]

On 6/28/17 1:25 PM, Thomas Stüfe wrote:
> Hi Coleen,
>
> this is a sensible cleanup!
>
> Some small nits:

:)  It's the kind of change that invites such things, but that's ok.
>
> vmError.cpp:
>
> - controlled_crash(): I would leave it either a first class global 
> function, maybe in vmError.hpp, or make it file scope static.

I could add controlled_crash to class VMError like the other functions, 
although it violates the new implied rule that we only declare functions 
in header files that are used externally.   But this seems reasonable to 
be in the header, I guess.

>
> - can we move #include <signal.h> up to the general includes?
>
> Not a part of your patch, but looking at the includes at the 
> beginning, I see:
>   ...
>   25 #include <fcntl.h>
>   26 #include "precompiled.hpp"
>   27 #include "code/codeCache.hpp"
>   ...
>
> which is probably wrong, I am sure fcntl.h is never picked up because 
> of the precompiled header line following it. So, maybe move the system 
> headers past the hotspot headers? I always thought that was the way we 
> do this.

I'll put the system headers (signal.h and this one) below the hotspot 
header #includes.  I think this is how they are supposed to be.   If 
it's not using fnctl.h, then I'll remove it.

>
> debug.hpp
>
> While looking at debug.hpp, I saw both report_out_of_shared_space() 
> and report_insufficient_metaspace(). Both seem awfully specific for a 
> general purpose assert file. The implementation for 
> report_insufficient_metaspace() in particular needs to know 
> "MaxMetaspaceSize", so I think it would be better placed in 
> metaspace.cpp (it is also only called from that one file, so it does 
> not even have to be exposed beyond metaspace.cpp).

This metaspace change may be removed with some work that Ioi is doing.  
I'll tell him about it.

Thanks,
Coleen
>
> Thank you, and Kind Regards, Thomas
>
>
>
>
>
>
> On Wed, Jun 28, 2017 at 5:38 PM, <coleen.phillimore at oracle.com 
> <mailto:coleen.phillimore at oracle.com>> wrote:
>
>     Summary: moved to vmError.hpp,cpp where they seemed more appropriate
>
>     Moved the function pd_ps() into frame_sparc.cpp eliminating
>     debug_cpu.cpp files.   You can pick a better name for this if you
>     want.
>
>     open webrev at
>     http://cr.openjdk.java.net/~coleenp/8182848.01/webrev
>     <http://cr.openjdk.java.net/%7Ecoleenp/8182848.01/webrev>
>     bug link https://bugs.openjdk.java.net/browse/JDK-8182848
>     <https://bugs.openjdk.java.net/browse/JDK-8182848>
>
>     Tested with JPRT.
>
>     Thanks,
>     Coleen
>
>