[crac] RFR: Persist memory in-JVM

[Anton Kozlov]

On Mon, 4 Sep 2023 06:24:05 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1PageBasedVirtualSpace.cpp line 298:
>> 
>>> 296:     flip = !flip;
>>> 297:     index = next;
>>> 298:   }
>> 
>> There is a pattern of computing the size of memory to be persisted, and re-computing the same value to be loaded. This function in particular looks very similar to G1PageBasedVirtualSpace::persist_for_checkpoint. Only later I found out, MemoryLoader verifies that the load request corresponds to a previous store request.
>> 
>> I propose to change this and similar functions to do verification solely, and make MemoryLoader to drive process of loading, since it anyway possesses all the data required for that. In this function, for verification it will be enough to check `_committed` did not change, and it can be done more efficiently, just by making a snapshot of the data and comparing current state and the snapshot. The verification can also be performed in debug builds only then.
>
>> just by making a snapshot of the data and comparing current state and the snapshot
> 
> Not sure if I follow in here; the verification could check if there's correct index entry in the loader, is that what you mean? Regrettably AFAIK it is not possible to check if given memory range is mapped with certain access modes.

Regarding the quote, I mean it could be:

// _snapshot_committed - copy of _committed only to be verified during checkpoint 

void G1PageBasedVirtualSpace::persist_for_checkpoint() {
  memcpy(_snapshot_committed, _committed, , sizeof(_snapshot_committed));
  ... // the rest of the current impl
}
 
void G1PageBasedVirtualSpace::load_on_restore() {
   guarantee(0 == memcmp(_snapshot_committed, _committed, sizeof(_snapshot_committed),
       "unexpected commit during checkpoint");
 }
 ```
 
And that's it -- G1PageBasedVirtualSpace just checks own invariants, an relies on MemoryPersister/Loader to load all necessary pages. This looks like as safe as the current implementation, and it could be a bit faster (no need to decide what do load here, and search that  in the index, just let Loader load everything), but the aim is to make code simpler and avoid repetitions.

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

PR Review Comment: https://git.openjdk.org/crac/pull/95#discussion_r1315135690