On Tue, 7 Jan 2025 12:51:33 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> There are a lot of format modifiers that are noisy and unnecessary in the
>> code. This change removes the INTX variants. It's not that disruptive even
>> for backporting because %z modifier has been available for a long time so
>> should backport fine. This was mostly done with a sed script plus some hand
>> fixups.
>>
>> Testing mach5 and other platform cross compilations in progress. Opening
>> this for GHA testing.
>
> Coleen Phillimore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Restore copyright and macro.
>From what I've looked at so far it looks good! I noticed there are several
>cases where you mix format specifiers with macros. I understand that replacing
>other macros may not be in the scope of this change but I find it inconsistent
>in places where we have both.
I listed out some of the cases below, but if you don't believe this to be
necessary you can ignore me.
src/hotspot/os/bsd/os_bsd.cpp line 2527:
> 2525: "\n\n"
> 2526: "Do you want to debug the problem?\n\n"
> 2527: "To debug, run 'gdb /proc/%d/exe %d'; then switch to
> thread %zd (" INTPTR_FORMAT ")\n"
There is both `%zd` and `INTPTR_FORMAT` in this line. I think it would be more
consistent to convert both to format specifiers here.
src/hotspot/os/linux/os_linux.cpp line 5276:
> 5274: "\n\n"
> 5275: "Do you want to debug the problem?\n\n"
> 5276: "To debug, run 'gdb /proc/%d/exe %d'; then switch to
> thread %zu (" INTPTR_FORMAT ")\n"
Same as above
src/hotspot/os/windows/os_windows.cpp line 533:
> 531: }
> 532:
> 533: log_info(os, thread)("Thread is alive (tid: %zu, stacksize: "
> SIZE_FORMAT "k).", os::current_thread_id(), thread->stack_size() / K);
Same as above, this time with `SIZE_FORMAT`
src/hotspot/os/windows/os_windows.cpp line 618:
> 616: thread->set_osthread(osthread);
> 617:
> 618: log_info(os, thread)("Thread attached (tid: %zu, stack: "
This line also mixes format specifiers and macros
src/hotspot/os/windows/os_windows.cpp line 3340:
> 3338: if (Verbose && PrintMiscellaneous) {
> 3339: reserveTimer.stop();
> 3340: tty->print_cr("reserve_memory of %zx bytes took " JLONG_FORMAT "
> ms (" JLONG_FORMAT " ticks)", bytes,
Here too
src/hotspot/share/classfile/classLoaderStats.cpp line 115:
> 113: Klass* parent_klass = (cls._parent == nullptr ? nullptr :
> cls._parent->klass());
> 114:
> 115: _out->print(INTPTR_FORMAT " " INTPTR_FORMAT " " INTPTR_FORMAT "
> %6zu " SIZE_FORMAT_W(8) " " SIZE_FORMAT_W(8) " ",
Here too
src/hotspot/share/classfile/classLoaderStats.cpp line 126:
> 124: _out->cr();
> 125: if (cls._hidden_classes_count > 0) {
> 126: _out->print_cr(SPACE SPACE SPACE "
> %6zu " SIZE_FORMAT_W(8) " " SIZE_FORMAT_W(8) " + hidden classes",
And here
src/hotspot/share/classfile/classLoaderStats.cpp line 140:
> 138: _out->print("Total = %-6zu", _total_loaders);
> 139: _out->print(SPACE SPACE SPACE " ", "", "", "");
> 140: _out->print_cr("%6zu " SIZE_FORMAT_W(8) " " SIZE_FORMAT_W(8) " ",
And here
src/hotspot/share/code/vtableStubs.cpp line 82:
> 80:
> 81: void VtableStub::print_on(outputStream* st) const {
> 82: st->print("vtable stub (index = %d, receiver_location = %zd, code = ["
> INTPTR_FORMAT ", " INTPTR_FORMAT "])",
And here
-------------
Changes requested by matsaave (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22916#pullrequestreview-2540706941
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909299619
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909300550
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909300883
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909301552
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909301678
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909303066
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909303216
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909303480
PR Review Comment: https://git.openjdk.org/jdk/pull/22916#discussion_r1909303991
