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

[Stefan Karlsson]

On 2014-04-09 00:16, Vladimir Kozlov wrote:
> Sorry, I had impression that we mostly use *INT32_FORMAT formats.
> You are right and I am fine with your changes.

Thanks, Vladimir.

StefanK

>
> Thanks,
> Vladimir
>
> On 4/8/14 10:43 AM, Stefan Karlsson wrote:
>> 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
>>>>
>>