Integrated: 8355692: Refactor stream indentation
Joel Sikström
jsikstro at openjdk.org
Mon May 12 15:52:00 UTC 2025
On Mon, 28 Apr 2025 11:07:44 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.
>
> After the changes in this PR there are only two ways th...
This pull request has now been integrated.
Changeset: 8128f638
Author: Joel Sikström <jsikstro at openjdk.org>
URL: https://git.openjdk.org/jdk/commit/8128f638fac39f6874c13364cbf742493745d845
Stats: 196 lines in 34 files changed: 28 ins; 51 del; 117 mod
8355692: Refactor stream indentation
Reviewed-by: stefank, cnorrbin
-------------
PR: https://git.openjdk.org/jdk/pull/24917
More information about the hotspot-dev
mailing list