RFR: 8212879: Make JVMTI TagMap table concurrent [v11]

[Serguei Spitsyn]

On Thu, 19 Nov 2020 00:39:52 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Hi Coleen,
>> It looks good to me.
>> Just a couple of nits below.
>> 
>> src/hotspot/share/prims/jvmtiTagMap.cpp:
>> 
>> There is a double-check for _needs_cleaning, so the one at line 136 can be removed:
>>  136   if (_needs_cleaning &&
>>  137       post_events &&
>>  138       env()->is_enabled(JVMTI_EVENT_OBJECT_FREE)) {
>>  139     remove_dead_entries(true /* post_object_free */);
>> 1158 void JvmtiTagMap::remove_dead_entries(bool post_object_free) {
>> 1159   assert(is_locked(), "precondition");
>> 1160   if (_needs_cleaning) {
>> 1161     log_info(jvmti, table)("TagMap table needs cleaning%s",
>> 1162                            (post_object_free ? " and posting" : ""));
>> 1163     hashmap()->remove_dead_entries(env(), post_object_free);
>> 1164     _needs_cleaning = false;
>> 1165   }
>> 1166 }
>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/AttachOnDemand/attach021/attach021Agent00.cpp:
>> 
>> The change below is not needed as the call to nsk_jvmti_aod_disableEventAndFinish() does exactly the same:
>> -    nsk_jvmti_aod_disableEventAndFinish(agentName, JVMTI_EVENT_OBJECT_FREE, success, jvmti, jni);
>> +
>> +    /* Flush any pending ObjectFree events, which may set success to 1 */
>> +    if (jvmti->SetEventNotificationMode(JVMTI_DISABLE,
>> +                                        JVMTI_EVENT_OBJECT_FREE,
>> +                                        NULL) != JVMTI_ERROR_NONE) {
>> +        success = 0;
>> +    }
>> +
>> +    nsk_aod_agentFinished(jni, agentName, success);
>>  }
>
>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/AttachOnDemand/attach021/attach021Agent00.cpp:
>> 
>> The change below is not needed as the call to nsk_jvmti_aod_disableEventAndFinish() does exactly the same:
>> 
>> ```
>> -    nsk_jvmti_aod_disableEventAndFinish(agentName, JVMTI_EVENT_OBJECT_FREE, success, jvmti, jni);
>> +
>> +    /* Flush any pending ObjectFree events, which may set success to 1 */
>> +    if (jvmti->SetEventNotificationMode(JVMTI_DISABLE,
>> +                                        JVMTI_EVENT_OBJECT_FREE,
>> +                                        NULL) != JVMTI_ERROR_NONE) {
>> +        success = 0;
>> +    }
>> +
>> +    nsk_aod_agentFinished(jni, agentName, success);
>>  }
>> ```
> 
> This change really is needed.
> 
> The success variable in the test is a global, initially 0, set to 1 by the
> ObjectFree handler.
> 
> In the old code, if the ObjectFree event hasn't been posted yet, we pass the
> initial 0 value of success to nsk_jvmti_aod_disabledEventAndFinish, where
> it's a local variable (so unaffected by any changes to the variable in the
> test), so stays 0 through to the call to nsk_aod_agentFinished.  So the test
> fails. 
> 
> The split in the change causes the updated post-ObjectFree event success
> value of 1 to be passed to agentFinished.  So the test passes.
> 
> That required some head scratching to find at the time.  That's the point of
> the comment about flushing pending events.  Maybe the comment should be
> improved.

@kimbarrett
Okay, thank you for explanation.
I agree, it'd make sense to improve the comment a little bit.
Thanks,
Serguei

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

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