RFR: 8333211: NMT Reports: replace manual indentation handling with auto indent
Thomas Stuefe
stuefe at openjdk.org
Sat Jun 1 05:25:19 UTC 2024
Hi,
this is the first of a few patches I plan to reduce NMT code complexity.
In NMT reporting, indentations are manually hard-coded. That makes the code brittle and frustrating to maintain. One cannot, e.g., re-use a printing subfunction in differently aligned portions without some awkward gymnastics (e.g. handing indentation as argument to the sub routine).
This patch adds a simple automatic indentation feature to `outputStream` and then uses that feature to simplify NMT reporting. NMT reporting becomes more malleable and easier to change.
I plan to reuse this feature for a couple of non-NMT things.
---
The new "autoindent" mode in `outputStream` is off by default. If enabled, the stream automatically adds indentations at new lines.
The added code is small and minimally invasive. It builds atop the existing manual indentation. It trades completeness for simplicity:
- it only works for a selected set of APIs (`print`, `print_cr`, `print_raw`, `print_raw_cr`).
- it does not work for multiline output: `print("line\nline\nline")` needs to be broken up into individual `print` calls
Despite these limitations, the feature works fine and I expect covers 99% of all use cases.
In the NMT reports, I replaced the many manual indentations with a few incarnations of `streamIndentor`. The changes in detail:
- to prevent backward compatibility issues, I am careful to limit the autoindent mode to NMT reporting.
- I replaced manual indentation with a few `streamIndentor` placements
- I removed the now useless `print_(malloc|virtual_memory|arena)_line`
- Some minor cleanups:
- merged the `outputStream` constructors into one
- moved indentation level and autoindent flag to the private section of `outputStream`
- removed some unused accessors in `outputStream`
- tightened printing code in NMT reporting, e.g. replaced `print_cr(" ")` with `cr()`
- removed the now unused `NativeCallStack::print_on(outputStream* out, int indent)` which hands in indentation.
The last point I see as the real benefit of autoindent; we now don't have to manually hand indentation levels to printing subroutines. Subroutines can be oblivious to indentation - their printouts will be correctly indented regardless of indentation level.
-----
Testing:
- GHAs
- I ran NMT jtreg tests manually
- I was careful to preserve the formatting of the existing reports. Therefore I tested all four variants: summary, summary.diff, detail, detail.diff , compared with outputs from mainline, and found no regressions.
- x86 errors unrelated
-------------
Commit messages:
- NMT reporting, better indentation
Changes: https://git.openjdk.org/jdk/pull/19461/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19461&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8333211
Stats: 240 lines in 7 files changed: 112 ins; 40 del; 88 mod
Patch: https://git.openjdk.org/jdk/pull/19461.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19461/head:pull/19461
PR: https://git.openjdk.org/jdk/pull/19461
More information about the hotspot-runtime-dev
mailing list