On Thu, 23 Jun 2022 12:34:59 GMT, Zhengyu Gu <z...@openjdk.org> wrote:
>> Currently, jdi only check and process class unloading event when it detects >> a new GC cycle. >> >> After [JDK-8212879](https://bugs.openjdk.org/browse/JDK-8212879), posting >> class events can overlap with GC finish event, that results, sometimes, it >> only captures partial or even empty unloaded class list. The pending list >> usually can be flushed out at next GC cycle. But for the classes unloaded >> during the last GC cycle, the class unloading events may lost forever. >> >> This patch checks and processes class unloading events unconditionally, >> suggested by @kbarrett, the last pending unloaded class list can be flushed >> by other events, such as `VM_DEATH`. >> >> It also performs `commonRef_compact()` only when there are classes unloaded. >> >> New test failed about 20% without patch, none with patch. >> >> **Update** >> There are significant changes from early patch. >> >> The new approach: >> No longer removing dead objects and post events on VM thread. I believe it >> was implemented this way to workaround the following issues: >> - JDI event handler uses JVMTI raw monitor, it requires thread in >> `_in_native` state >> - The thread can not hold lock, which is needed to protect `JvmtiTagMap` >> while walking, when transition to `_in_native` state >> >> The new solution breaks up into two steps: >> - Collect all dead object tags with lock >> - Transition to `_in_native` state and post object free events in one batch >> >> This way, JDI event handler can process object free events upon arrivals >> without delay. > > Zhengyu Gu has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 18 additional commits since > the last revision: > > - Improve naming and cleanup > - Merge branch 'master' into JDK-8256811-jdi-missing-class-unloading-event > - v4 > - v3 > - v2 > - Merge branch 'master' into JDK-8256811-jdi-missing-class-unloading-event > - Merge branch 'jdi_tmp' into JDK-8256811-jdi-missing-class-unloading-event > - v0 > - v2 > - v1 > - ... and 8 more: https://git.openjdk.org/jdk/compare/8ac4830c...559b4bf1 It would be good if the test was testing that if there are two `ClassUnloadRequest`, then there should be two `ClassUnloadEvents` in each `EventSet`, one for each `ClassUnloadRequest`. To make it even more interesting, give each `ClassUnloadRequest` a class filter, but have one be more restrictive. You currently use `CLASS_NAME_PREFIX` for each class and then use that as a filter. Maybe you could also have `ALT_CLASS_NAME_PREFIX` that adds a bit more to the prefix, and maybe about half the classes use the ALT prefix. static final String CLASS_NAME_PREFIX = "SampleClass__"; static final String ALT_CLASS_NAME_PREFIX = CLASS_NAME_PREFIX + "ALT__"; Use `CLASS_NAME_PREFIX` as a class filter for one of the `ClassUnloadRequest`s like you are now, and use `ALT_CLASS_NAME_PREFIX` for the other. Then when you get a `ClassUnloadEvent` it should always match `CLASS_NAME_PREFIX`, but if (and only if) it also matches `ALT_CLASS_NAME_PREFIX`, then there should be a second `ClassUnloadEvent` in the `EventSet` that also matches `ALT_CLASS_NAME_PREFIX` ------------- PR: https://git.openjdk.org/jdk/pull/9168
