RFR: 8259242: vmTestbase/vm/mlvm/mixed/stress/java/findDeadlock/TestDescription.java timed out [v2]

[Patricio Chilano Mateo]

On Wed, 7 Apr 2021 12:39:19 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change makes the DictionaryEntry pd_set reads lock free, because when I set a low SafepointTimeout on the findDeadlock test, these threads were waiting on the ProtectionDomainSet_lock.  This lock is a singleton that's taken for every dictionary entry.
>> 
>> I don't know if this change will make the test never timeout again, because removing this lock caused us to hit a low SafepointTimeout on another _no_safepoint_check lock that is not so easy to eradictate.
>> 
>> Tested with tiers 1-3.  There's a test that exercises this code in runtime/Dictionary/ProtectionDomainCacheTest.java.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add acquire/release and update names.

Hi Coleen,

Changes LGTM. Some comments below:

src/hotspot/share/classfile/dictionary.cpp line 92:

> 90:   // This doesn't require a lock because nothing is reading this
> 91:   // entry anymore.  The ClassLoader is dead.
> 92:   if (entry->pd_set_acquire() != NULL) {

I think this should still be a 'while' to remove the whole list.

src/hotspot/share/classfile/protectionDomainCache.cpp line 93:

> 91:   // with the caller class and class loader, which if still alive will keep this
> 92:   // protection domain entry alive.
> 93:   if (delete_list->length() != 0) {

If you want you could also use a threshold to decide when to issue the handshake, to avoid triggering it for very few entries (given removals seem to be rare already). See is_async_deflation_needed() for example.

src/hotspot/share/classfile/protectionDomainCache.cpp line 211:

> 209: }
> 210: 
> 211: 

extra line?

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

PR: https://git.openjdk.java.net/jdk/pull/3362