RFR(XL): 8181449: Fix debug.hpp / globalDefinitions.hpp dependency inversion
Kim Barrett
kim.barrett at oracle.com
Thu Jun 22 20:54:05 UTC 2017
> On Jun 22, 2017, at 2:49 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>
> Hi Kim,
>
> On 2017-06-22 02:29, Kim Barrett wrote:
>>>> (3) http://cr.openjdk.java.net/~kbarrett/8181449/move_pns/
>>>> a. Moved print_native_stack to VMError class.
>>>> b. Removed unused and undefined pd_obfuscate_location.
>>>> c. Simplified #ifdef PRODUCT guard in ps().
>>>
>>> You removed the call to p->trace_stack(). Was that intentional?
>>>
>>> if (p->has_last_Java_frame()) {
>>> // If the last_Java_fp is set we are in C land and
>>> // can call the standard stack_trace function.
>>> -#ifdef PRODUCT
>>> p->print_stack();
>>> } else {
>>> +#ifdef PRODUCT
>>> tty->print_cr("Cannot find the last Java frame, printing stack disabled.");
>>> #else // !PRODUCT
>>> - p->trace_stack();
>>> - } else {
>> I kept getting confused by this code while trying to figure out what
>> to do with pd_ps, which is why I changed it. Seems I was even more
>> confused than I thought, since I failed to see the difference between
>> p->print_stack() and p->trace_stack(). Well spotted. I've reverted
>> this change.
>
> OK. I wonder if this code would have been easier to read with to #ifdef PRODUCT sections.
I looked at that before simply reverting, and it was still pretty ugly. And I’d have
needed to figure out how to test. I shouldn’t have touched this code as part of
this set of changes in the first place; I’m just glad you spotted the mistake.
>>> http://cr.openjdk.java.net/~kbarrett/8181449/target_macros/src/share/vm/utilities/globalDefinitions_gcc.hpp.udiff.html
>>>
>>> Maybe also get rid of the following line:
>>> //----------------------------------------------------------------------------------------------------
>>> -// Debugging
>> Gone now.
>
> I wasn't particularly clear on this request, and you cut out the following part of my mail:
> "or get rid of the following stray new line so that the code in the different globalDefinitions files are more consistent.”
Yeah, I misunderstood what you were asking for.
What I've ended up doing is deleting all the "//--- ... ---" lines in
the globalDefinitions_xxx.hpp files. There weren't very many, all in
this general vicinity, and just making the one near the nanness comment
consistent would have left other similarly odd file to file variations.
> http://cr.openjdk.java.net/~kbarrett/8181449/hotspot.01.inc/src/share/vm/utilities/globalDefinitions.hpp.udiff.html
>
> 31
> 32 #include TARGET_COMPILER_HEADER(utilities/globalDefinitions)
> 33 // Defaults for macros that might be defined per compiler.
> 34 #ifndef NOINLINE
> 35 #define NOINLINE
> 36 #endif
>
> Would you mind adding a newline between 32 and 32 before pushing?
Done.
I also updated the copyrights for globalDefinitions.hpp and
macros.hpp, which got lost somewhere in the patch splitting.
More information about the hotspot-dev
mailing list