[crac] RFR: Persist memory in-JVM [v13]

[Anton Kozlov]

On Mon, 16 Oct 2023 11:59:13 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> This is a WIP for persisting various portions of JVM memory from within the process, rather than leaving that up to C/R engine (such as CRIU). In the future this will enable us to optimize loading (theoretically leading to faster startup), compress and encrypt the data.
>> 
>> At this moment the implementation of persisting thread stacks is in proof-of-concept shape, ~especially waking up the primordial thread includes some hacks. This could be improved by using a custom (global, ideally robust) futex instead of the internal futex used by `pthread_join`.~ Fix already implemented.
>> 
>> ~One of the concerns related to thread stacks is rseq used by glibc; without disabling this CRIU would attempt to 'fix' the rseqs (see `fixup_thread_rseq`) and touch the unmapped memory. CRIU uses special ptrace commands to check the status; I am not aware if it is possible to access this information from within the process using any public API.~ Solved. The JVM forks and the child ptraces JVM, recording the rseq info. Once we have the we can unregister the rseq before checkpoint and register it afterwards (here we have the advantage that we know the threads won't be in any critical section as we're in a safepoint).
>> ~Currently this works with `/proc/sys/kernel/yama/ptrace_scope` set to `0`; we should make it work with `1` (default), too.~ Fixed.
>> 
>> Regarding persistence implementation, currently we store the memory in multiple files; first block (page-size aligned) contains some validation data and index of memory address - file offsets. The way this is implemented requires the size of index to be known before dumping memory. It might be more convenient (and portable for e.g. network-based storage) to use single file, and either keep the index in the 'fundamental' memory (C heap), put it at the end of file or to another index file.
>
> Radim Vansa has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Revert changes not needed anymore in vm_version
>  - Windows build fix
>  - Refactored to fix OSX

Incomplete first round of review

src/hotspot/os/linux/crac_linux.cpp line 481:

> 479: 
> 480:   Atomic::add(&persist_waiters, 1);
> 481:   // From now on the code must not use stack variables!

Does it work in slowdebug? Suppose Atomic::add is not inlined and it creates the stack frame. Once the variable is decremented, but before the return, the stack may be unmapped already. Althought chances are not high. So this can in theory crash, right?

src/hotspot/os/linux/crac_linux.cpp line 562:

> 560:   // We cannot release this in after_threads_restored(), have to wait
> 561:   // until the last thread restores
> 562:   int dec = Atomic::sub(&persist_waiters, 1);

`Atomic::sub(&persist_waiters, 1)` is in the HAS_RSEQ ifdef. Apparently needs

...
#endif // HAS_RSEQ

int dec = Atomic::sub(&persist_waiters, 1);

#ifdef HAS_RSEQ
...


Or move the rseq_config memory freeing to `crac::after_threads_restored`, as a mirror of `crac::before_threads_persisted` which already spins on the `persist_waiters`.

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

> 361:   VM_Version::crac_restore_finalize();
> 362: 
> 363:   memory_restore();

memory_restore() does not gurantee not calling glibc extensively, so it may call unsupported CPU instruction before `VM_Version::crac_restore_finalize()` will have a chance to terminate the VM cleanly.

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

> 370:   if (CRPersistMemory) {
> 371:     // Before reinit_memory the code must not change memory layout, e.g. mmapping
> 372:     // or even malloc'ing anything (malloc running out of space could run short and allocate

Does the comment regarding malloc is valid in the current implementation? Is that the C lib malloc, or JVM's one, which we control and indeed should not use?

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

PR Review: https://git.openjdk.org/crac/pull/95#pullrequestreview-1700082007
PR Review Comment: https://git.openjdk.org/crac/pull/95#discussion_r1373459286
PR Review Comment: https://git.openjdk.org/crac/pull/95#discussion_r1373453376
PR Review Comment: https://git.openjdk.org/crac/pull/95#discussion_r1373476222
PR Review Comment: https://git.openjdk.org/crac/pull/95#discussion_r1373472920