RFR: 8341060: Cleanup statics in HeapDumper

Aleksey Shipilev shade at openjdk.org
Fri Sep 27 09:52:38 UTC 2024


On Fri, 27 Sep 2024 01:28:18 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

> The fix cleans up static variables/methods in HeapDumper.
> See jira for details.
> 
> Testing:
>   heap dump-related tests (test/hotspot/jtreg/serviceability, test/hotspot/jtreg/compiler/c2/TestReduceAllocationAndHeapDump.java, test/hotspot/jtreg/runtime/ErrorHandling, test/hotspot/jtreg/gc/epsilon, test/jdk/sun/tools/jhsdb, test/jdk/sun/tools/jmap, test/jdk/com/sun/management/HotSpotDiagnosticMXBean)

I have only cosmetic comments. Do you know why these were statics to begin with?

src/hotspot/share/services/heapDumper.cpp line 1520:

> 1518:  private:
> 1519:   AbstractDumpWriter* _writer;
> 1520:   GrowableArray<Klass*>* _klass_map;

While we are at it, turn these `const`?

src/hotspot/share/services/heapDumper.cpp line 1526:

> 1524:       _klass_map->at_put_grow(serial_num, k);
> 1525:   }
> 1526:  public:

Suggestion:

  void add_class_serial_number(Klass* k, int serial_num) {
    _klass_map->at_put_grow(serial_num, k);
  }

 public:

src/hotspot/share/services/heapDumper.cpp line 1532:

> 1530: };
> 1531: 
> 1532: void LoadedClassDumper::do_klass(Klass* k) {

Any reason not to merge this definition straight in declaration?

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

PR Review: https://git.openjdk.org/jdk/pull/21216#pullrequestreview-2333075710
PR Review Comment: https://git.openjdk.org/jdk/pull/21216#discussion_r1778216275
PR Review Comment: https://git.openjdk.org/jdk/pull/21216#discussion_r1778347171
PR Review Comment: https://git.openjdk.org/jdk/pull/21216#discussion_r1778348342


More information about the serviceability-dev mailing list