Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8322150

[GenShen] TestPeriodicGC fails with "Generational mode: Should have periodic young GC in logs"

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Duplicate
    • Icon: P4 P4
    • None
    • repo-shenandoah
    • hotspot

      I stumbled upon this failure when testing the changes for JDK-8321939.

      The subtest of TestPeriodicGC run in generational mode with periodic old and young gc intervals set to non-zero values fails with no periodic gcs in the log.

      The test appears to have started failing after a recent change in upstream JDK tip to remove the ShenandoahUnloadClassesFrequency flag, but a cursory visual review of differences didn't reveal the main issue; see JDK-8320877.

      However, looking at the GC logs, it's clear that in its current state, we end up with the ShenandoahRegulatorThread causing essentially back-to-back global gc's on account of `should_unload_classes()` answering `yes`, in response to which it starts a global cycle. Since global cycles occur in quick succession, the periodic gc's never need to occur, causing the test to fail.

      In fact the test failure revealed that the ShenandoahRegulatorThread's logic may be a bit too aggressive in precipitating global collections because of `should_unload_classes` typically answering `yes` in its current form, even when there is no need to actually precipitate a collection:

      ```
      bool ShenandoahHeuristics::should_unload_classes() {
        if (!can_unload_classes()) return false;
        if (has_metaspace_oom()) return true;
        return ClassUnloadingWithConcurrentMark;
      }
      ```

      since ClassUnloadingWithConcurrentMark now defaults to true (for a while):

      ```
        product(bool, ClassUnloading, true, \
                "Do unloading of classes") \
                                                                                  \
        product(bool, ClassUnloadingWithConcurrentMark, true, \
                "Do unloading of classes with a concurrent marking cycle") \
                                                                                  \
      ```

      (and is not turned off by default in GenShen).

      This is further confirmed by running the subtest with either of the above class unloading flags disabled so as to throttle back the class unloading triggered global collections, allowing the periodic gcs to then occur. (The main test just sleeps without any allocations, so no GCs would otherwise be triggered.)

      I suspect the issue stems from a possible misinterpretation (or at least some confusion) in the semantics of `should_unload_classes` and therefore the use of it by (some of) its callers.

      I believe the test is correct, and so is the code in the upstream as well as the change, but this may have serendiptously uncovered a small issue in GenShen's regulator thread work loop logic and possible confusion in the semantics of `should _unload_classes`.

      This ticket is to clean out that confusion and get this test to pass.

            ysr Y. Ramakrishna
            ysr Y. Ramakrishna
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: