RFR (S) 8182848: Some functions misplaced in debug.hpp
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Jun 28 17:25:05 UTC 2017
Hi Coleen,
this is a sensible cleanup!
Some small nits:
vmError.cpp:
- controlled_crash(): I would leave it either a first class global
function, maybe in vmError.hpp, or make it file scope static.
- 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.
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).
Thank you, and Kind Regards, Thomas
On Wed, Jun 28, 2017 at 5:38 PM, <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
> bug link https://bugs.openjdk.java.net/browse/JDK-8182848
>
> Tested with JPRT.
>
> Thanks,
> Coleen
>
>
More information about the hotspot-dev
mailing list