RFR: 8329655: Cleanup KlassObj and klassOop names after the PermGen removal [v2]

Stefan Karlsson stefank at openjdk.org
Thu Apr 4 12:18:25 UTC 2024


On Thu, 4 Apr 2024 10:07:11 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Stefan Karlsson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review Roman
>
> src/hotspot/share/memory/heapInspection.cpp line 173:
> 
>> 171: KlassInfoTable::KlassInfoTable(bool add_all_classes) {
>> 172:   _size_of_instances_in_words = 0;
>> 173:   _ref = (uintptr_t) Universe::boolArrayKlass();
> 
> It seems weird (non-obvious) to cast to uintptr_t here. I see it is only used in KlassInfoTable::hash(), which is weird, too. I am not sure that this even does a useful hashing. Might be worth to get rid of the whole thing and use the [fastHash](https://github.com/rkennke/jdk/blob/JDK-8305896/src/hotspot/share/utilities/fastHash.hpp) stuff that @rose00 proposed for Lilliput. Perhaps in a follow-up. I'd probably either cast to void* or Klass*, or cast to uintptr_t as you did and remove the unnecessary cast in ::hash().

I agree. I'll start by removing the redundant cast in `::hash()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18618#discussion_r1551554904


More information about the serviceability-dev mailing list