RFR: 8039244: Don't use UINT32_FORMAT and INT32_FORMAT when printing uints and ints in the GC code
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Apr 8 22:16:21 UTC 2014
Sorry, I had impression that we mostly use *INT32_FORMAT formats.
You are right and I am fine with your changes.
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
>>>
>
More information about the hotspot-gc-dev
mailing list