RFR: 8354362: Use automatic indentation in CollectedHeap printing [v11]
Leo Korinth
lkorinth at openjdk.org
Wed Apr 23 16:02:40 UTC 2025
On Tue, 22 Apr 2025 15:05:08 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:
>> Hello,
>>
>>> This PR only focuses on fixing indentation and re-arranging some callsites. It does *not* change the contents of any output, apart from some (IMO relevant) indentation/whitespace additions.
>>
>>> Update: With some suggestions from @stefank, I've renamed print_on to print_heap_on and print_on_error to print_gc_on to better reflect their purpose where they're called. With this I've also renamed other instances of print_on_error to better reflect their purpose. Printing heap information and printing gc information is now two distinct steps in vmError.cpp.
>>
>> Currently, the CollectedHeap printing code (print_on and print_on_error, with calls "below") prepends spaces in messages in a way that only makes sense if you write the code and then check the output to see if you've done everything correctly. To make writing and maintaining printing code easy, I propose we move to a system where each printing method, starting at callers of print_on and print_on_error, uses the indentation API in outputStream and does not rely on prepending spaces like is done right now.
>>
>> What I propose is that any (GC) printing method should not make any assumptions of the indentation level of its caller(s). This means that each function shall:
>> 1. Not prepend any spaces to its printing, and instead expect that the caller(s) should handle any indentation before calling this function.
>> 2. Enforce its own indentation, by enabling auto indentation in its own context and for its "lower level" calls (which is often the desired outcome).
>>
>> Combining these two rules means that *any* (GC) printing method can be called from anywhere and give sensible output, without (seemingly random) indentation of expectations elsewhere.
>>
>> I have aggregated calls that print on the same indentation level to the same callsite. This makes it clear where to look in the code and also makes it easier to add/enforce indendation. To this end, I have re-arranged print_on_error so that it never includes print_on. The new system I propose is that print_on and print_on_error can be called separately for different information, which aligns well with having the same callsite for the same indentation. See changes in vmError.cpp for how this is implemented.
>>
>> Instead of prepending spaces, I use StreamAutoIndentor, defined in ostream.hpp. To make using automatic indentation easier, I've made some changes to StreamAutoIndentor so that it inherits from streamIndentor and also add an *optional* argu...
>
> Joel Sikström has updated the pull request incrementally with one additional commit since the last revision:
>
> Use FormatBuffer instead of local char buf
src/hotspot/share/utilities/vmError.cpp line 1395:
> 1393: // Take heap lock over both heap and GC printing so that information is
> 1394: // consistent.
> 1395: Heap_lock->lock();
I would prefer using `MutexLocker` even though it changes indentation (of the source code).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24593#discussion_r2056384316
More information about the hotspot-gc-dev
mailing list