Integrated: 8295303: cleanup debug agent's confusing use of EI_GC_FINISH

[Chris Plummer]

On Thu, 27 Oct 2022 18:06:38 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

> This PR renamed EI_GC_FINISH to EI_CLASS_UNLOAD and does some other minor cleanups related to EI_GC_FINISH.
> 
> The debug agent deals with 3 types of events: JVMTI (per the spec), JDWP (per the spec), and an event indexing that the debug agent refers to as EI (Event Index) events. We have the following EI_GC_FINISH event. The indexing is into the array of jdwp event handlers which are created when the debugger sends an EventRequest command.
> 
> Note there is no EI_CLASS_UNLOAD event that maps to the JDWP CLASS_UNLOAD event, but instead we have:
> 
> `        EI_GC_FINISH = 8,`
> 
> And these are the mappings between EI_GC_FINISH and JDWP and JVMTI events
> 
> 
>   case JVMTI_EVENT_GARBAGE_COLLECTION_FINISH:
>       return EI_GC_FINISH;
> 
>   case JDWP_EVENT(CLASS_UNLOAD):
>       return EI_GC_FINISH;
> 
>   index2jvmti[EI_GC_FINISH - EI_min] = JVMTI_EVENT_GARBAGE_COLLECTION_FINISH;
> 
>   index2jdwp[EI_GC_FINISH - EI_min] = JDWP_EVENT(CLASS_UNLOAD);
> 
> 
> So there is this odd relationship between EI_GC_FINISH and the JDWP CLASS_UNLOAD event. Note that JVMTI does not have a CLASS_UNLOAD (except for unused support in the extension mechanism), and JDWP has no GC_FINISH event.
> 
> This relationship between EI_GC_FINISH and the JDWP CLASS_UNLOAD events stems from the fact that at one point JVMTI_EVENT_GARBAGE_COLLECTION_FINISH was used to trigger the synthesizing all of JDWP CLASS_UNLOAD events for classes that unloaded during the last GC. That is no longer the case, and instead each time a JVMTI OBJECT_FREE event is triggered for a Class instance, the JDWP CLASS_UNLOAD is generated.
> 
> Since JDWP CLASS_UNLOAD maps to EI_GC_FINISH, we have the following:
> 
> `    node = getHandlerChain(EI_GC_FINISH)->first;`
> 
> By looking at this line of code, you would never guess that this is how you fetch the event handler chain for JDWP CLASS_UNLOAD EventRequests, but it is.
> 
> To clean this up I renamed EI_GC_FINISH to EI_CLASS_UNLOAD. However, that still leaves the EI_GC_FINISH to JVMTI_EVENT_GARBAGE_COLLECTION_FINISH mapping to deal with. It's not needed anymore. When we get a JVMTI_EVENT_GARBAGE_COLLECTION_FINISH, this is all we ever do with it:
> 
> 
> static void JNICALL
> cbGarbageCollectionFinish(jvmtiEnv *jvmti_env)
> {
>     LOG_CB(("cbGarbageCollectionFinish"));
>     ++garbageCollected;
>     LOG_MISC(("END cbGarbageCollectionFinish"));
> }
> 
> 
> So the event is not passed around at all, and doesn't trigger other events or mapping to a JDWP event. It is never used as an "event index". This means jvmti2EventIndex should assert if it is ever passed in, since following mapping should never be needed:
> 
> 
>         case JVMTI_EVENT_GARBAGE_COLLECTION_FINISH:
>             return EI_GC_FINISH; 
> 
> 
> This is accomplished by simply deleting the above code, and failing through to the default error handling code.
> 
> Also no longer needed is the following entry:
> 
>     index2jvmti[EI_GC_FINISH          -EI_min] = JVMTI_EVENT_GARBAGE_COLLECTION_FINISH
> 
> For this we just assign the entry to 0, which will result in an error if the entry is ever referenced.

This pull request has now been integrated.

Changeset: 74f2b16a
Author:    Chris Plummer <cjplummer at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/74f2b16a8e3a6ac4fbd81ab5792aa9245ed9b2d4
Stats:     47 lines in 6 files changed: 18 ins; 3 del; 26 mod

8295303: cleanup debug agent's confusing use of EI_GC_FINISH

Reviewed-by: amenkov, sspitsyn

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

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