Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mention in Gc documentation that compaction is currently not implemented. #11816

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open

Mention in Gc documentation that compaction is currently not implemented. #11816

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
039ee40
stdlib: annotate compaction related functions and fields to mention t…
abbysmal Dec 13, 2022
cf28163
runtime: caml_gc_compaction should do a full major cycle
abbysmal Jan 5, 2023
b8d5879
runtime: max_overhead should always be zero
abbysmal Jan 5, 2023
48fa0da
stdlib: modify entries related to compaction in the documentation to …
abbysmal Jan 5, 2023
498e9ce
testsuite: promote lib-runtime-events test
abbysmal Jan 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions stdlib/gc.mli
View file
Open in desktop
Expand Up @@ -79,7 +79,9 @@ type stat =
are not available for allocation. *)

compactions : int;
(** Number of heap compactions since the program was started. *)
(** 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. *)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove the last sentence - i.e. we just state that it's always zero because compaction was removed.


top_heap_words : int;
(** Maximum size reached by the major heap, in words. *)
Expand Down Expand Up @@ -143,7 +145,9 @@ type control =
Default: 0. *)

max_overhead : int;
(** Heap compaction is triggered when the estimated amount
(** 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

of "wasted" memory is more than [max_overhead] percent of the
amount of live data. If [max_overhead] is set to 0, heap
compaction is triggered at the end of each major GC cycle
Expand Down Expand Up @@ -300,7 +304,9 @@ external full_major : unit -> unit = "caml_gc_full_major"
unreachable blocks. *)

external compact : unit -> unit = "caml_gc_compaction"
(** Perform a full major collection and compact the heap. Note that heap
(** 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
Comment on lines +299 to +301
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

compaction is a lengthy operation. *)

val print_stat : out_channel -> unit
Expand Down