RFR: 8341060: Cleanup statics in HeapDumper [v2]

[Alex Menkov]

On Fri, 27 Sep 2024 09:50:21 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

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

AFAIU they were introduced to simplify access to currently active dumper (and its writer - HeapDumper had a single writer that days) from callback functions (static methods like `do_load_class` updated by this fix).

> 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`?

the map is updated by the class, so it can't be `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:

Fixed

> 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?

Merged.

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

PR Comment: https://git.openjdk.org/jdk/pull/21216#issuecomment-2379944243
PR Review Comment: https://git.openjdk.org/jdk/pull/21216#discussion_r1779080190
PR Review Comment: https://git.openjdk.org/jdk/pull/21216#discussion_r1779080374
PR Review Comment: https://git.openjdk.org/jdk/pull/21216#discussion_r1779081395