RFR: 8333211: NMT Reports: replace manual indentation handling with auto indent [v2]

Thomas Stuefe stuefe at openjdk.org
Tue Jun 4 11:47:29 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, an...

Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:

 - restore indentation() accessor
 - Merge branch 'master' into nmt-reporting-better-indentation
 - NMT reporting, better indentation

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/19461/files
  - new: https://git.openjdk.org/jdk/pull/19461/files/29e71ff7..14b4f589

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19461&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19461&range=00-01

  Stats: 6619 lines in 220 files changed: 4136 ins; 1877 del; 606 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