Re: [Libguestfs] [PATCH libguestfs 1/2] ocaml/implicit_close test: collect all currently unreachable blocks

On Wed, May 31, 2023 at 12:32:49PM +0200, Laszlo Ersek wrote: > On 5/31/23 11:12, Richard W.M. Jones wrote: >> On Sat, May 27, 2023 at 03:32:36PM +0200, Jürgen Hötzel wrote: >>> Fixes failing implice_close test on OCaml 5. >>> --- >>> ocaml/t/guestfs_065_implicit_close.ml | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/ocaml/t/guestfs_065_implicit_close.ml b/ocaml/t/guestfs_065_implicit_close.ml >>> index 567e550b4..5e00c21ac 100644 >>> --- a/ocaml/t/guestfs_065_implicit_close.ml >>> +++ b/ocaml/t/guestfs_065_implicit_close.ml >>> @@ -30,8 +30,8 @@ let () = >>> *) >>> >>> (* This should cause the GC to close the handle. *) >>> -let () = Gc.compact () >>> +let () = Gc.full_major () >>> >>> let () = assert (!close_invoked = 1) >>> >>> -let () = Gc.compact () >>> +let () = Gc.full_major () >> >> I don't understand this patch at all. If there a test failing we need >> to diagnose why it is failing, not paper over the symptoms. >> >> What is the exact failure? > > Well my assumption is that (a) we need to force a garbage collection for > the (unreachable) handle to be closed actually (from earlier, nothing > new regarding that), but (b) with OCaml 5, "compact" is not strong > enough for that, while "full_major" is. Whether that means OCaml 5 > changed the semantics of these functions, or that even with OCaml 4 > we've only (consistently) lucky with "compact", I can't tell. So it turns out there is a difference between OCaml 4.14 and 5 here. In 4.14: Gc.compact finishes the current major cycle and then compacts the heap: https://github.com/ocaml/ocaml/blob/74fe398bbe2e53db21a998356b042c232d42a... Gc.full_major finishes the major cycle and then sometimes compacts the heap based on a threshold of how much heap is being used: https://github.com/ocaml/ocaml/blob/74fe398bbe2e53db21a998356b042c232d42a... So Gc.full_major is (kind of) a subset of Gc.compact, which (mostly) matches the documentation. In 5.0: Gc.compact finishes the major cycle, and as far as I can tell doesn't compact the heap (bug?!): https://github.com/ocaml/ocaml/blob/ffb2022797986324213891a59c02af46269b5... But the big difference is Gc.full_major: https://github.com/ocaml/ocaml/blob/ffb2022797986324213891a59c02af46269b5... As you can see from a comment in the code, "[the new garbage collector] can require up to 3 GC cycles for a currently-unreachable object to be collected", and Gc.full_major does this. (I don't believe this is true for the old GC in OCaml <= 4.) So in 5.0 full_major is quite a different operation from compact. It runs multiple major cycles to make sure everything is collected, and doesn't compact the heap, but then neither does Gc.compact which is no longer a superset of Gc.full_major. The patch there is correct, for OCaml 5, but breaks OCaml 4,

so it should probably have some kind of conditional on the OCaml version. It also needs a much clearer explanation.