Mention in Gc documentation that compaction is currently not implemented. #11816
Conversation
…hat it is missing from 5.0
abbysmal
changed the title
stdlib/gc.mli
Outdated
| (** Note: As of OCaml 5.0, compaction is not available and this field's | ||
| value will always be zero. | ||
| Number of heap compactions since the program was started. *) |
There was a problem hiding this comment.
I would remove the last sentence - i.e. we just state that it's always zero because compaction was removed.
stdlib/gc.mli
Outdated
| (** Note: As of OCaml 5.0, compaction is not implemented, and this field | ||
| will have no effect. | ||
| Heap compaction is triggered when the estimated amount |
There was a problem hiding this comment.
For the control words, I think this ought to be stronger - i.e. "must be zero" and Gc.set should raise Invalid_argument if it's not zero.
There was a problem hiding this comment.
It seems to me that raising Invalid_argument is a bit strong?
Isn't there a middle ground where we could notify users this is not taken into account without breaking their programs?
There was a problem hiding this comment.
My rationale is that these functions should be being called correctly - i.e. that it's a programming error to call Gc.set with an invalid set of parameters for a given version of OCaml (i.e. the user's program is broken, whether or not we raise an exception!). It doesn't seem right to me to be silently dropping the incorrect configuration request, at which point the only option is an exception and Invalid_argument seems more appropriate here than Failure?
| (** Note: As of OCaml 5.0, compaction is not implemented, and calling this | ||
| function will only trigger a major collection. | ||
| Perform a full major collection and compact the heap. Note that heap |
There was a problem hiding this comment.
I was going to suggest that this should presumably be equivalent to Gc.full_major ... but in fact it looks like there's a bug in the implementation, as the C code appears only to do a major GC slice!
…make it explicit that compaction is not implemented
|
@dra27 Thanks for your feedback. I made some adjustment following your comments, let me know if it looks better. |
|
This is relevant: #12193 |
I decided to go with #11812 step-by-step, and this PR is the first one.
This simply addresses the first point: annotate documentation for compaction related functions and stating that compaction is currently not available since release 5.0.