RFR: 8355692: Refactor stream indentation [v2]

[Casper Norrbin]

On Mon, 28 Apr 2025 12:59:49 GMT, Joel Sikström <jsikstro at openjdk.org> wrote:

>> Hello,
>> 
>>> To make it easier to review this PR, each commit that is prefixed with "FIX:" has the command/method I've used to verify that the output of prints is not changed/broken.
>> 
>> The goal of this PR is to refactor the usage of streamIndentor and StreamAutoIndentor and combine them into a single StreamIndentor class to make it easier to apply indentation. [JDK-8354362](https://bugs.openjdk.org/browse/JDK-8354362) introduced a partial solution for the refactor I propose, by being able to add indentation with a StreamAutoIndentor.
>> 
>> To use indentation, you currently need to do two separate things: 1) increment indentation and 2) apply indentation when printing. The indentation level can be incremented by using one of: streamIndentor, inc() or inc(int n), and automatic indentation can be enabled by using one of: StreamAutoIndentor, indent(), cr_indent().
>> 
>> The new StreamIndentor has the functionality of both streamIndentor and StreamAutoIndentor, that is, enabling automatic indentation and also applying an optional amount of indentation. This means you only need StreamIndentor to make sure that indentation is incremented and applied.
>> 
>> There are currently four (4) ways that indentation is applied in HotSpot:
>> 
>> 1) The new method of enabling automatic indentation and applying indentation simultaneously (partially implemented already).
>> Only in GC printing code via CollectedHeap.
>> 
>> 2) Initially use a StreamAutoIndentor, then use streamIndentor to temporarily increment indentation. Indentation is automatically applied when printing.
>> nmt/memReporter uses this principle, by having a StreamAutoIndentor as a member variable and applying indentation via streamIndentor when needed.
>> 
>> 3) Use a streamIndentor and manually call indent() (or cr_indent()).
>> Commonly used pattern. No need to manually apply indentation if automatically applied.
>> 
>> 4) Increment/decrement indentation (using inc() and/or dec()) and manually call indent().
>> Only C1's CFGPrinterOutput does this.
>> 
>> I leave the fourth case alone as it is fairly self-contained and requires a more extensive re-write, appropriate for a follow-up patch if that's what we want.
>> 
>> When refactoring, I only found a single case where a streamIndentor couldn't be trivially replaced with the new StreamIndentor. All other streamIndentors, along with manually applied indentation using indent()/cr_indent(), are able to be replaced with the new StreamIndentor without changing the output.
>> 
> ...
>
> Joel Sikström has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make sure indentation is unchanged in ArenaStats::print_on

lgtm 👍

-------------

Marked as reviewed by cnorrbin (Author).

PR Review: https://git.openjdk.org/jdk/pull/24917#pullrequestreview-2803551902