On Fri, 27 Sep 2024 01:28:18 GMT, Alex Menkov <amen...@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
