RFR: 8039244: Don't use UINT32_FORMAT and INT32_FORMAT when printing uints and ints in the GC code

[Stefan Karlsson]

On 2014-04-08 18:42, Vladimir Kozlov wrote:
> So *_FORMAT are not broken (when used correctly). 
> It is GC group's decision to use %d and %u in GC code. Right?
> It is bad that different components of JVM will be inconsistent in 
> this but it is your decision.
Just take a look at the compiler code. There are only ten usages of 
*INT32_FORMAT:

src//share/vm/code/codeCache.cpp:    st->print_cr(" total_blobs=" 
UINT32_FORMAT " nmethods=" UINT32_FORMAT
src//share/vm/code/codeCache.cpp:                 " adapters=" 
UINT32_FORMAT,
src//share/vm/code/codeCache.cpp:  st->print(" total_blobs='" 
UINT32_FORMAT "' nmethods='" UINT32_FORMAT "'"
src//share/vm/code/codeCache.cpp:            " adapters='" UINT32_FORMAT 
"' free_code_cache='" SIZE_FORMAT "'",

src//share/vm/code/nmethod.cpp:    tty->print_cr("*flushing nmethod 
%3d/" INTPTR_FORMAT ". Live blobs:" UINT32_FORMAT "/Free CodeCache:" 
SIZE_FORMAT "Kb",

src//share/vm/opto/callnode.cpp:    st->print(" %s%d]=#ScObj" 
INT32_FORMAT, msg, i, sco_n);
src//share/vm/opto/callnode.cpp:      st->print(" 
%s%d]=#"INT32_FORMAT,msg,i,t->is_int()->get_con());
src//share/vm/opto/callnode.cpp:      st->print("        # ScObj" 
INT32_FORMAT " ", i);

src//share/vm/runtime/deoptimization.cpp: tty->print_cr("  %40s: " 
UINT32_FORMAT " (%.1f%%)", name, r, (r * 100.0) / total);

and there is about 600 lines in the compiler code that use %d. To me 
these ten lines are what's inconsistent, not that the GC team has 
removed its few usages of *INT32_FORMAT.


For reference, these are _all_ usages of INT32_FORMAT and UINT32_FORMAT 
in the HotSpot code base.

$ grep -r "INT32_FORMAT" src/
src//os_cpu/bsd_zero/vm/os_bsd_zero.cpp:      "caught unhandled signal " 
INT32_FORMAT " at address " PTR_FORMAT;
src//os_cpu/bsd_zero/vm/os_bsd_zero.cpp: 
fatal(err_msg("pthread_stackseg_np failed with err = " INT32_FORMAT,
src//os_cpu/bsd_zero/vm/os_bsd_zero.cpp: 
fatal(err_msg("pthread_attr_init failed with err = " INT32_FORMAT, rslt));
src//os_cpu/bsd_zero/vm/os_bsd_zero.cpp: 
fatal(err_msg("pthread_attr_get_np failed with err = " INT32_FORMAT,
src//share/vm/code/codeCache.cpp:    st->print_cr(" total_blobs=" 
UINT32_FORMAT " nmethods=" UINT32_FORMAT
src//share/vm/code/codeCache.cpp:                 " adapters=" 
UINT32_FORMAT,
src//share/vm/code/codeCache.cpp:  st->print(" total_blobs='" 
UINT32_FORMAT "' nmethods='" UINT32_FORMAT "'"
src//share/vm/code/codeCache.cpp:            " adapters='" UINT32_FORMAT 
"' free_code_cache='" SIZE_FORMAT "'",
src//share/vm/code/nmethod.cpp:    tty->print_cr("*flushing nmethod 
%3d/" INTPTR_FORMAT ". Live blobs:" UINT32_FORMAT "/Free CodeCache:" 
SIZE_FORMAT "Kb",
src//share/vm/interpreter/bytecodeTracer.cpp:    st->print_cr(" " 
INT32_FORMAT, constants->int_at(i));
src//share/vm/interpreter/bytecodeTracer.cpp:      st->print_cr(" " 
INT32_FORMAT, get_byte());
src//share/vm/interpreter/bytecodeTracer.cpp:      st->print_cr(" " 
INT32_FORMAT, get_short());
src//share/vm/interpreter/bytecodeTracer.cpp: st->print_cr(" #%d " 
INT32_FORMAT, index, offset);
src//share/vm/interpreter/bytecodeTracer.cpp:        st->print(" %d " 
INT32_FORMAT " " INT32_FORMAT " ",
src//share/vm/interpreter/bytecodeTracer.cpp:          const char 
*format = first ? " %d:" INT32_FORMAT " (delta: %d)" :
src//share/vm/interpreter/bytecodeTracer.cpp: ", %d:" INT32_FORMAT " 
(delta: %d)";
src//share/vm/interpreter/bytecodeTracer.cpp:          const char 
*format = first ? " " INT32_FORMAT ":" INT32_FORMAT :
src//share/vm/interpreter/bytecodeTracer.cpp: ", " INT32_FORMAT ":" 
INT32_FORMAT ;
src//share/vm/opto/callnode.cpp:    st->print(" %s%d]=#ScObj" 
INT32_FORMAT, msg, i, sco_n);
src//share/vm/opto/callnode.cpp:      st->print(" 
%s%d]=#"INT32_FORMAT,msg,i,t->is_int()->get_con());
src//share/vm/opto/callnode.cpp:      st->print("        # ScObj" 
INT32_FORMAT " ", i);
src//share/vm/opto/type.cpp:    sprintf(buf, "min+" INT32_FORMAT, n - 
min_jint);
src//share/vm/opto/type.cpp:    sprintf(buf, "max-" INT32_FORMAT, 
max_jint - n);
src//share/vm/opto/type.cpp:    sprintf(buf, INT32_FORMAT, n);
src//share/vm/runtime/arguments.cpp:  jio_snprintf(buffer, bufsz, "1." 
UINT32_FORMAT, spec_version);
src//share/vm/runtime/deoptimization.cpp: tty->print_cr("  %40s: " 
UINT32_FORMAT " (%.1f%%)", name, r, (r * 100.0) / total);
src//share/vm/runtime/perfData.cpp:              " length = " 
INT32_FORMAT ","
src//share/vm/runtime/perfData.cpp:              " 
PerfMaxStringConstLength = " INT32_FORMAT "\n",
src//share/vm/runtime/perfData.cpp:   jio_snprintf(intbuf, 40, 
UINT32_FORMAT, instance);
src//share/vm/runtime/perfData.cpp:   jio_snprintf(intbuf, 40, 
UINT32_FORMAT, instance);
src//share/vm/runtime/perfMemory.cpp:              "  Overflow = " 
INT32_FORMAT " bytes"
src//share/vm/runtime/safepoint.cpp: 
INT32_FORMAT_W(8)INT32_FORMAT_W(11)INT32_FORMAT_W(15)
src//share/vm/runtime/safepoint.cpp: tty->print(INT32_FORMAT"         ", 
sstats->_page_armed);
src//share/vm/runtime/safepoint.cpp: tty->print_cr(INT32_FORMAT"   ", 
sstats->_nof_threads_hit_page_trap);
src//share/vm/trace/traceStream.hpp:    _st.print("%s = "UINT32_FORMAT, 
label, val);
src//share/vm/trace/traceStream.hpp:    _st.print("%s = "UINT32_FORMAT, 
label, val);
src//share/vm/trace/traceStream.hpp:    _st.print("%s = "INT32_FORMAT, 
label, val);
src//share/vm/trace/traceStream.hpp:    _st.print("%s = "UINT32_FORMAT, 
label, val);
src//share/vm/trace/traceStream.hpp:    _st.print("%s = "INT32_FORMAT, 
label, val);


I don't understand why you insist that I'm making things more inconsistent.


StefanK

>
> Thanks,
> Vladimir
>
> On 4/8/14 1:15 AM, Stefan Karlsson wrote:
>>
>> On 2014-04-04 09:59, Stefan Karlsson wrote:
>>> Please, review this patch to the GC code to change usages of 
>>> UINT32_FORMAT and INT32_FORMAT to %u and %d when uints
>>> and ints are used.
>>>
>>> While doing this change I found, and changed, a couple of places 
>>> where we used UINT32_FORMAT to print variables that
>>> were less than 4 bytes.
>>
>> This change is not strictly needed, since varargs methods will widen 
>> the types of the arguments. Thanks David Holmes for
>> pointing this out.
>>
>>>
>>> webrev: http://cr.openjdk.java.net/~stefank/8039244/webrev.00/
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8039244
>>
>> I'm going to push the webrev.02 version now. I haven't heard back 
>> from all that commented on this thread, so if someone
>> feels that this needs to be discussed more we can start a new 
>> discussion or open a new RFE.
>>
>> thanks,
>> StefanK
>>
>>>
>>> thanks,
>>> StefanK
>>