RFR: 8292446: Make TableRateStatistics optional in CHT [v2]
Coleen Phillimore
coleenp at openjdk.org
Wed Aug 17 15:24:21 UTC 2022
On Wed, 17 Aug 2022 12:46:43 GMT, Johan Sjölén <duke at openjdk.org> wrote:
>> Right now the footprint of disabling statistics for tables where they're not used is minimal as they're single instance and static. We're working on using CHT in more contexts, where it will be dynamically allocated, and then these changes will add up to something more substantial.
>>
>> Passes tier1.
>
> Johan Sjölén has updated the pull request incrementally with two additional commits since the last revision:
>
> - Uninitialized pointers do not default to nullptr
> - Fixes
Thank you for doing this. I have a couple small comments.
src/hotspot/share/gc/g1/g1CardSet.cpp line 37:
> 35: #include "runtime/java.hpp"
> 36: #include "utilities/bitMap.inline.hpp"
> 37: #include "utilities/concurrentHashTable.hpp"
Do you needs this? Doesn't concurrentHashTable.inline.hpp include it already?
src/hotspot/share/utilities/concurrentHashTable.hpp line 46:
> 44: private:
> 45: // _stats_rate is null if statistics are not enabled.
> 46: TableRateStatistics* _stats_rate = nullptr;
Don't you have to initialize this in the constructor?
src/hotspot/share/utilities/concurrentHashTable.hpp line 399:
> 397: size_t grow_hint = DEFAULT_GROW_HINT,
> 398: void* context = nullptr,
> 399: bool enable_statistics = DEFAULT_ENABLE_STATISTICS);
I wonder if it would be better to put context last since it's only used by one hashtable?
-------------
PR: https://git.openjdk.org/jdk/pull/9899
More information about the hotspot-dev
mailing list