RFR: 8355692: Refactor stream indentation

Joel Sikström jsikstro at openjdk.org
Mon Apr 28 11:12:56 UTC 2025


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().

I propose to make it easier to use and apply indentation by combining streamIndentor and StreamAutoIndentor into a new class called StreamIndentor. 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 to do one thing to make sure that indentation is incremented and applied, making it easier to use indentation.

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.

In the third case, there is a possibility that there are cases where applying indentation automatically is undesirable. However, when refactoring and looking at the code I have not found any place where this is the case. All cases when using a streamIndentor and applying indentation manually using indent()/cr_indent() are able to be replaced by the new StreamIndentor without changing the output.

After the changes in this PR there are only two ways that indentation is applied:
1) The new method of enabling automatic indentation and applying indentation simultaneously using StreamIndentor.
2) Increment/decrement indentation (using inc() and/or dec()) and manually call indent().
Only C1's CFGPrinterOutput does this.

Some practical changes:
 * `outputStream::cr_indent()` has been removed since it is no longer used/needed.
 * `outputStream::set_autoindent()` is now private and StreamIndentor is a friend of outputStream to access this.
 * Made the indentation argument of StreamIndentor explicit, instead of the default of 2 which streamIndentor had. Sure, there are many cases which use an indentation of two, but I don't feel it's necessary to have this default only because it is common. If anything, making the argument explicit makes it easier to understand the code, in my opinion.
 * Renamed all uses of a StreamIndentor to `si[level]`.

Testing:
* GHA, Oracle's tier 1-4
* Manual inspection of printed content. See the commit message of commits prefixed with "FIX:".

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

Commit messages:
 - Remove cr_indent()
 - Adapt gtest
 - Make outputStream::set_autoindent private
 - Rename to StreamIndentor si[level]
 - Combine streamIndentor and StreamAutoIndentor
 - FIX: memReporter
 - FIX: compilationMemoryStatistic.cpp
 - FIX: Trivial changes in compilationMemoryStatistic.cpp
 - FIX: metaspaceStatistics.cpp
 - FIX: printCLDMetaspaceInfoClosure.cpp
 - ... and 6 more: https://git.openjdk.org/jdk/compare/b41e0b17...a0c122ca

Changes: https://git.openjdk.org/jdk/pull/24917/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24917&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8355692
  Stats: 195 lines in 34 files changed: 27 ins; 50 del; 118 mod
  Patch: https://git.openjdk.org/jdk/pull/24917.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24917/head:pull/24917

PR: https://git.openjdk.org/jdk/pull/24917


More information about the hotspot-dev mailing list