[crac] RFR: 8353241: [CRaC] Support ZGC [v3]

[Timofei Pushkin]

On Thu, 10 Apr 2025 21:27:11 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> During my tests with https://github.com/CRaC/example-spring-boot I could not get the image size as low as with G1, but the presented changes improve the image footprint. As as anecdotal data, the image is 177 MB with G1 while 215 MB with ZGC (fastdebug build, `-Xmx1G`).
>
> Radim Vansa has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
> 
>  - Merge branch 'crac' into zgc
>  - fixup
>  - 8353241: CRaC ZGC support

src/hotspot/share/gc/z/zPageAllocator.cpp line 1067:

> 1065:     event.commit(uncommitted);
> 1066:   }
> 1067: }

No newline at the EOF

src/hotspot/share/gc/z/zPageAllocator.hpp line 173:

> 171:   void threads_do(ThreadClosure* tc) const;
> 172: 
> 173:   void cleanup_unused();

This should not be addressed in this PR, but I find "cleanup_unused" name pretty undescriptive, both here and in `CollectedHeap` in general. It wasn't obvious to me that this is something used only by CRaC. I would propose `cleanup_before_checkpoint`, for example.

src/hotspot/share/gc/z/zPageCache.cpp line 33:

> 31: #include "gc/z/zStat.hpp"
> 32: #include "gc/z/zValue.inline.hpp"
> 33: #include "logging/log.hpp"

There is no logging code added so these new imports shouldn't be needed.

src/hotspot/share/gc/z/zPageCache.cpp line 292:

> 290:       _delay(delay) {
> 291:     // Set initial timeout
> 292:     *_timeout = ZUncommitDelay;

Shouldn't this also use `delay`? This shouldn't have any real influence now (the code that has `delay != ZUncommitDelay` doesn't use the timeout) but anyway.

src/hotspot/share/runtime/crac.cpp line 429:

> 427:     MemTracker::final_report(tty);
> 428:   }
> 429: 

Is this left intentionally? If yes, I think nothing will be printed after this on the real VM exit (after restore) since there is `Atomic::cmpxchg(&g_final_report_did_run, false, true) == false` in `MemTracker::final_report()`.

test/jdk/jdk/crac/fileDescriptors/ZGCTest.java line 37:

> 35:  * @requires (os.family == "linux")
> 36:  */
> 37: public class ZGCTest implements CracTest {

A bit unclear why this is in `jdk/crac/fileDescriptors` directory. I guess because of the required `memfd` support but I would still put it in the general `jdk/crac` directory.

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

PR Review Comment: https://git.openjdk.org/crac/pull/219#discussion_r2041490955
PR Review Comment: https://git.openjdk.org/crac/pull/219#discussion_r2041524248
PR Review Comment: https://git.openjdk.org/crac/pull/219#discussion_r2041509158
PR Review Comment: https://git.openjdk.org/crac/pull/219#discussion_r2041503636
PR Review Comment: https://git.openjdk.org/crac/pull/219#discussion_r2041476340
PR Review Comment: https://git.openjdk.org/crac/pull/219#discussion_r2041517297