On Wed, 3 May 2023 09:04:50 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
> Hi all, > > Please review the implementation of Generational ZGC, which can be turned on > by adding -XX:+ZGenerational in addition to using -XX:+UseZGC. Generational > ZGC is a major rewrite of the non-generational ZGC version that exists in the > openjdk/jdk repository. It splits the heap into two generations; the young > generation where newly allocated objects are born, and the old generation > where long-lived objects get promoted to. The motivation for introducing > generations is to allow ZGC to reclaim memory faster by not having to walk > the entire object graph every time a garbage collection is run. This should > make Generational ZGC suitable for more workloads. In particular workloads > that previously hit allocation stalls because of high allocation rates, large > live sets, or limited spare machine resources, have the potential to work > better with Generational ZGC. For an in-depth description of Generational > ZGC, see https://openjdk.org/jeps/439. > > The development of Generational ZGC started around the same time as the > development of JDK 17. At that point we forked off the Generational ZGC > development into its own branch and let non-generational live unaffected in > openjdk/jdk. This safe-guarded non-generational ZGC and allowed Generational > ZGC to move unhindered, without the shackles of having to fit into another GC > implementation's design and quirks. Since then, almost all of the ZGC files > have been changed. Moving forward to today, when it's ready for us to > upstream Generational ZGC, we now need to deliver Generational ZGC without > disrupting our current user-base. We have therefore opted to initially > include both versions of ZGC in the code base, but with the intention to > deprecate non-generational ZGC in a future release. Existing users running > with only -XX:+UseZGC will get the non-generational ZGC, and users that want > the new Generational ZGC need to run with -XX:+ZGenerational in addition to > -XX:+UseZGC. The intention i s to give the users time to validate and deploy their workloads with the new GC implementation. > > Including both the new evolution of a GC and its legacy predecessor poses a > few challenges for us GC developers. The first reaction could be to try to > mash the two implementations together and sprinkle the GC code with > conditional statements or dynamic dispatches. We have done similar > experiments before. When ZGC was first born, we started an experiment where > we converted G1 into getting the same features as the evolving ZGC. It was > quite clear to us how time consuming and complex things end up being when we > tried to keep both the original G1 working, and at the same time implemented > the ZGC-alike G1. Given this experience, we don't see that as a viable > solution to deliver a maintainable and evolving Generational ZGC. Our > pragmatic suggestion to these challenges is to let Generational ZGC live > under the current gc/z directories and let the legacy, non-generational ZGC > be completely separated in its own directories. This way we can continue to > move quickly with the continued develop ment of Generational ZGC and let the non-generational ZGC be mostly untouched until it gets deprecated, and eventually removed. The non-generational ZGC directory will be gc/x and all the classes of non-generational have been prefixed with X instead of Z. An alternative to this rename could be to namespace out non-generational ZGC. We experimented with that, but it was too easy to accidentally cross-compile Generational ZGC code into non-generational ZGC, so we didn't like that approach. > > Most of the stand-alone cleanups and enhancements outside of the ZGC code > have already been upstreamed to openjdk/jdk. There are still a few patches > that could/should be pushed separately, but they will be easier to understand > by also looking at the Generational ZGC code, so they will be sent out after > this PR has been published. The patches that could be published separately > are: > > * 59d1e96af6a UPSTREAM: Introduce check_oop infrastructure to check oops in > the oop class > * ca9edf8aa79 UPSTREAM: RISCV tmp reg cleanup resolve_jobject > * 4bec9c69b67 CLEANUP: barrierSetNMethod_aarch64.cpp > * b67d03a3f04 UPSTREAM: Add relaxed add&fetch for aarch64 atomics > * a2824734d23 UPSTREAM: lir_xchg > * 36cd39c0126 UPSTREAM: assembler_ppc CMPLI > * 447259cea42 UPSTREAM: assembler_ppc ANDI > * 9417323499a UPSTREAM: Add VMErrorCallback infrastructure > > Regarding all the changesets you see in this PR, they form the history of the > development of Generational ZGC. It might look a bit unconventional to what > you are used to see in openjdk development. What we have done is to use > merges with the 'ours' strategy to ignore the previous Generational ZGC > patches, and then rebased and flattened the changes on top of the merge. This > effectively gives us the upsides of having a rebased repository and the > upsides of retaining the history in the repository. The downside could be > that GitHub now lists all those changesets in the PR. Given that this patch > is so big, and that you likely only want to see a part of it, I suggest that > you pull down the PR branch and then compare it to the openjdk/jdk changeset > this PR is based against: > > > git fetch https://github.com/openjdk/zgc zgc_master > git diff zgc_master... <sub-directory of interest> > > > There have been many contributors of this patch over the years. I'll do my > best to poke Skara into listing you all, but if you see that I've missed your > name please reach out to me and I'll fix it. > > Testing: we have been continuously running Generational ZGC through Oracle's > tier1-8 testing. I have obviously stared at this code since its inception. To me it doesn't just look good, it looks fantastic. ------------- Marked as reviewed by eosterlund (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13771#pullrequestreview-1410554817
