RFR: 8319650: Improve heap dump performance with class metadata caching [v3]

[Aleksey Shipilev]

On Thu, 9 Nov 2023 13:47:40 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Aleksey Shipilev 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:
>> 
>>  - Limit the table size
>>  - Merge branch 'master' into JDK-8319650-heapdump-cache-meta
>>  - Fix
>
> src/hotspot/share/services/heapDumper.cpp line 830:
> 
>> 828:   }
>> 829: 
>> 830:   typedef ResourceHashtable<void*, DumperClassCacheTableEntry*, TABLE_SIZE, AnyObj::C_HEAP, mtServiceability,
> 
> If the key is InstanceKlass, why not use InstanceKlass* as the key instead of void*.

Right! Fixing.

> src/hotspot/share/services/heapDumper.cpp line 1041:
> 
>> 1039: 
>> 1040: // returns the size of the instance of the given class
>> 1041: u4 DumperSupport::instance_size(Klass* k, DumperClassCacheTableEntry* class_cache_entry) {
> 
> Since you changed the parameters to instance_size(), can you make the argument InstanceKlass* instead of Klass* to remove the cast at 1045.  I believe all the callers already pass InstanceKlass*.

Yes, fixing.

> src/hotspot/share/services/heapDumper.cpp line 1126:
> 
>> 1124: // dump the raw values of the instance fields of the given object
>> 1125: void DumperSupport::dump_instance_fields(AbstractDumpWriter* writer, oop o, DumperClassCacheTableEntry* class_cache_entry) {
>> 1126:   assert(class_cache_entry, "Pre-condition: should be provided");
> 
> Needs explicit check against nullptr.

D'oh, of course.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16545#discussion_r1388096112
PR Review Comment: https://git.openjdk.org/jdk/pull/16545#discussion_r1388096280
PR Review Comment: https://git.openjdk.org/jdk/pull/16545#discussion_r1388096991