RFR: 8295303: cleanup debug agent's confusing use of EI_GC_FINISH [v2]

[Chris Plummer]

> 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.

Chris Plummer has updated the pull request incrementally with one additional commit since the last revision:

  Fixed ei range check logic errors.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/10887/files
  - new: https://git.openjdk.org/jdk/pull/10887/files/ce7a283c..ec44b9dd

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10887&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10887&range=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/10887.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10887/head:pull/10887

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