On Tue, 30 Aug 2022 18:20:26 GMT, Ioi Lam <ik...@openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> David's comments
>
> src/hotspot/share/classfile/protectionDomainCache.cpp line 162:
>
>> 160: // Purge any deleted entries outside of the SystemDictionary_lock.
>> 161: purge_deleted_entries();
>> 162: int oops_removed = purge_entries_from_table(); // reacquires SD lock
>
> It's confusing to have two similarly named functions (purge_deleted_entries
> and purge_entries_from_table). Maybe the two functions should be combined
> into a single `purge()` function that performs the two steps?
I'll put purge_entries_from_table back in the mainline of this function where
it used to be. I'd separated it because it was long. purge_deleted_entries
should be its own function because it goes to a safepoint so it's special.
> src/hotspot/share/classfile/protectionDomainCache.hpp line 35:
>
>> 33: // The amount of different protection domains used is typically
>> magnitudes smaller
>> 34: // than the number of system dictionary entries (loaded classes).
>> 35: class ProtectionDomainCacheTable : public AllStatic {
>
> How about adding a comment to say what this table maps from/to? Something
> like:
>
> // The ProtectionDomainCacheTable maps all java.security.ProtectionDomain
> objects that are
> // registered by DictionaryEntry::add_protection_domain() to a unique
> WeakHandle.
Okay, I added your comment.
-------------
PR: https://git.openjdk.org/jdk/pull/10043
