[crac] RFR: Wait until G1 GC has finished before creating a snapshot. [v6]

[Jan Kratochvil]

On Fri, 4 Aug 2023 07:52:32 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:

>>> You suggest during CRaC snapshot the whole VM is already frozen so we do not need any synchronization? OK, if it is so I can just call `uncommit_regions`.
>> 
>> Please check if that is safe. I propose to call g1h->uncommit_regions() from the VM operation. But I'm not a G1/GC expert. AFAICS G1ServiceThread (G1ServiceThread::run_service) calls Task's execute() without sync, so we need at most synchornization implied by G1UncommitRegionTask::execute [1]. I missed this in the first reading: what does SuspendibleThreadSetJoiner do? That can be unnecessary if the uncommit is performed from the VM op. I don't see any synchronization beyond that (excluding some inside of uncommit_regions and its callees). 
>> 
>> It may be worth to discuss this on hotspot-gc-dev mail list [2].
>> 
>> [1] https://github.com/openjdk/crac/blob/crac/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp#L105
>> [2] https://mail.openjdk.org/pipermail/hotspot-gc-dev/
>
> I do not know much how all the threads and VM threads and GC pauses work but I disagree with this patch. The patch before was waiting until the asynchronous `G1UncommitRegionTask` has done its job. Currently we do the job ourselves but that means `G1UncommitRegionTask` can be executed asynchronously while we do the same job in our thread. And I do not see any `synchronized` keywords in the `uncommit_regions` code this patch calls now. So two threads doing the same job will clash.

I read now I should contact https://mail.openjdk.org/pipermail/hotspot-gc-dev/ , thanks for the advice.

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

PR Review Comment: https://git.openjdk.org/crac/pull/93#discussion_r1285214504