RFR: 8292680: Convert Dictionary to ConcurrentHashTable [v4]

Coleen Phillimore coleenp at openjdk.org
Wed Aug 24 18:10:32 UTC 2022


On Wed, 24 Aug 2022 16:03:06 GMT, Johan Sjölén <duke at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
>> 
>>  - Merge branch 'master' into cht-dictionary
>>  - Conditionalize scanning function call for safepoint.
>>  - Use ConcurrentHashtable for Dictionary
>
> src/hotspot/share/classfile/dictionary.cpp line 411:
> 
>> 409: // During class loading we may have cached a protection domain that has
>> 410: // since been unreferenced, so this entry should be cleared.
>> 411: void Dictionary::clean_cached_protection_domains(GrowableArray<ProtectionDomainEntry*>* delete_list) {
> 
> Only mention of `GrowableArray` is here and it's a pointer, so do we really need to include the `growableArray.hpp` header? A forward declaration should be sufficient. Not entirely sure, maybe the templates screw it up.

We do operations on _delete_list like push below, so we need the whole header file here.

> src/hotspot/share/classfile/dictionary.cpp line 412:
> 
>> 410: // since been unreferenced, so this entry should be cleared.
>> 411: void Dictionary::clean_cached_protection_domains(GrowableArray<ProtectionDomainEntry*>* delete_list) {
>> 412:   assert(JavaThread::current()->is_Java_thread(), "only called by JavaThread");
> 
> `JavaThread::current` already does this check, so just leave it as `Thread::current()`.
> 
> Edit, to be clear: JavaThread::current() will fail its assertion before `->is_Java_thread()` is ever called on this line.

Okay, yes, you're right, it's a bit redundant.

> src/hotspot/share/classfile/dictionary.hpp line 49:
> 
>> 47:   class Config {
>> 48:    public:
>> 49:     typedef DictionaryEntry* Value;
> 
> Nit: Is there a reason that we're mixing `using` and `typedef` (see line 55)?

The Config class is defined as needing a typedef for 'Value'.  I'll see if using also does the same thing.

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

PR: https://git.openjdk.org/jdk/pull/9886


More information about the hotspot-dev mailing list