I don't like the function name. You should change it, probably to finalise_last or something like that. Does anyone have a good idea?

As for the documentation, I find it confusing because it speaks about "unreachable for the first time" and "unreachable for the last time", notions that are non-obvious and not used anywhere else. Moreover, you absolutely need to mention ephemerons here, because that's the whole point.

I see that these finalisers still cause their associated values to be marked during minor collections. Instead couldn't we only mark the values associated with "traditional" finalisers, and move the finalisers whose values are not promoted directly into the finalising set?

Does anyone have a good idea?

I don't have a specific good idea, but it might be good to just call these by some other name than "finaliser" ("destructor", "cleaner", ...).

Moreover, you absolutely need to mention ephemerons here, because that's the whole point.

I agree. The documentation should include a guarantee that these new finalisers run after any weak pointers to the object have been cleared.

@lpw25 Indeed it is a good idea. I can modify caml_final_do_young_roots to not mark these values and move the finalisers into the todo set. Moreover your remark shows that there is a bug in the current patch caml_final_do_strong_roots should not mark values that have the FINAL_CALLED_WITHOUT_VALUE flag. I will fix that.

The documentation should include a guarantee that these new finalisers run after any weak pointers to the object have been cleared.

Just to be clear, when the finalisers run, the weak pointers could have not yet been cleared, but the accessors behave as if it is already done (during the clean phase accessors verify if the weak pointer is going to be cleared in this phase).

@damiendoligez

As for the documentation, I find it confusing because it speaks about "unreachable for the first time" and "unreachable for the last time", notions that are non-obvious and not used anywhere else.

In the current documentation of GC.finalize the notion of "unreachable for the first time" is used:

  [f] will be called with [v] as
   argument at some point between the first time [v] becomes unreachable
   (including through weak pointers) and the time [v] is collected by
   the GC.

In any case I'm going to add something about weak pointers and ephemerons.

@damiendoligez Could you please check the comments I added in cfa8a1d before I try to implement @lpw25 idea?

When the branch will be stable, I will stash the commits, but for now I think it can help the review.

@damiendoligez I don't understand why there is a finalisable set and a recent set, they both appear at the same time in 0a2021e. Why can't we just check if the value is young or not? Thank you for your help.

The comment in cfa8a1d looks OK.

The recent set is used when scanning roots for minor collection. We don't want to go through all the finalized values at each minor GC, so we isolate a subset that is guaranteed to contain all young finalized values.

@lpw25 idea have been implemented: don't mark value with finalisation_last` during minor collection. It was trickier than what I though.

The implementation have been simplified, by keeping the two datastructures separate.

The added tests doesn't contain any oracle but with the debug runtime it found an error. It is hard to have deterministic (ocamlc/ocamlopt) oracle for this problems.

@damiendoligez Something strange: for executing all the finaliser, sometime two Gc.full_major () are needed. I though that after a Gc.full_major () another one was always useless. So perhaps a problem remains. An example can be seen by replacing in testsuite/test/misc/finalise.ml let debug = false by true. The output is finaliser.log.txt.

@damiendoligez Something strange: for executing all the finaliser, sometime two Gc.full_major () are needed. I though that after a Gc.full_major () another one was always useless.

It was just because I attached a Gc.finaliser(_first) and a Gc.finaliser_last on the same object. In fact if there is some Gc.finaliser(_first), the documentation of Gc.full_major is misleading:

external full_major : unit -> unit = "caml_gc_full_major"
(** Do a minor collection, finish the current major collection cycle,
   and perform a complete new cycle.  This will collect all currently
   unreachable blocks. *)

Edit: substitute first and last

Maybe it should say "this will collect and finalize all currently unreachable blocks".

@bobot What is the status of this? I'm rather hoping to use these in 4.04.

On my side it is finished (the modification of the documentation of full_major can wait). But it needs a review, there were a lot of change since the last review for integrating your idea.

Reviewed, rebased, merged.

@damiendoligez Thank you!