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

G1: Investigate refactoring aged out retained region dropping logic

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Unresolved
    • Icon: P4 P4
    • tbd
    • 23
    • hotspot
    • gc

      Currently the logic to age out retained regions is embedded in the collection set selection code.

      There has been the suggestion that it might be cleaner to move this logic out of the collection set selection code into a separate phase as it only tangentially selects regions for the collection set.

      This came up during implementation and review of JDK-8329528. In the end this refactoring has not been implemented in that change because of complexity it introduces in the current code with the following reasons:

        * extra loop over the collection set candidates, where that loop is at a different place in the code, called from `pre_evacuate_collection_set`. This creates an extra additional dependency when tracking dependencies there. One could imagine to add time tracking for this phase as well, resulting in more extra code.
        * logging of this filtering will become disjointed from collection set selection. Dropping a region from the collection set candidates implies not selecting it, and reflecting this properly in the selection process takes additional effort unless one agrees with having separate log messages. (E.g. it is a bit surprising that when you know that there were pinned regions, the cset selection may actually tell you that there were none).
        * this is very little extra code piggybacking it on the collection set selection

      I think I considered this extra step for filtering when implementing this the first time, rejected it for all the extra effort (probably in the range of 100 LOC, see the diff for the https://github.com/openjdk/jdk/pull/18692/commits/0c46d6a653365472421123f021cda414834fac15 change - which is incomplete btw as it does not adjust the regression test and potentially add proper timing/logging) just to basically avoid this single `if` clause there.

            Unassigned Unassigned
            tschatzl Thomas Schatzl
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: