On Wed, May 31, 2023 at 04:44:49PM +0200, Laszlo Ersek wrote:
On 5/31/23 13:13, Richard W.M. Jones wrote:
> 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,
Right, from your explanation above, this is what I've been expecting --
what works for OCaml 5 may not work for OCaml 4.
In fact I don't understand the OCaml runtime's developers -- this is a
compatibility breaking change! Every project that uses the Gc module
will have to add compat code (version checking) now.
Yes, it's not a good change.
About the patch, we could try to conditionalize it on
Sys.ocaml_release.Sys.major >= 5 (see /usr/lib64/ocaml/sys.mli)
Rich.
> so it
> should probably have some kind of conditional on the OCaml version.
> It also needs a much clearer explanation.
Thanks
Laszlo
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top