Fix Gc.quick_stat documentation

[jmid]

While torturing the poor Gc module, a test spotted that the Gc.quick_stat documentation was a bit off.

• the promised live_words, live_blocks, free_words, and fragments fields are not zero
• the largest_free and stack_size fields returned by Gc.stat are also 0 (along with heap_chunks and free_blocks - as documented in the stat record type at the top) and hence better be documented in the stat function
• nit alert: while not being a native speaker, I believe singular "this metric" is preferable

I can see #12850 included in 5.2.0 made adjustments to the Gc.quick_stat functionality and documentation, but OCaml5 has returned non-zero fields for these entries since 5.0.0 AFAICS.

A top-level example to illustrate:

OCaml version 5.4.0+dev0-2024-08-25
Enter #help;; for help.

Gc.compact ();;
- : unit = ()
Gc.quick_stat ();;
- : Gc.stat =
{Gc.minor_words = 757748.; promoted_words = 204684.; major_words = 402624.;
 minor_collections = 18; major_collections = 9; heap_words = 435112;
 heap_chunks = 0; live_words = 349577; live_blocks = 80388;
 free_words = 82724; free_blocks = 0; largest_free = 0; fragments = 2811;
 compactions = 1; top_heap_words = 485635; stack_size = 0;
 forced_major_collections = 2}
Gc.stat ();;
- : Gc.stat =
{Gc.minor_words = 794089.; promoted_words = 205774.; major_words = 403714.;
 minor_collections = 24; major_collections = 12; heap_words = 435112;
 heap_chunks = 0; live_words = 349957; live_blocks = 80491;
 free_words = 82344; free_blocks = 0; largest_free = 0; fragments = 2811;
 compactions = 1; top_heap_words = 485635; stack_size = 0;
 forced_major_collections = 3}

[damiendoligez]

LGTM

[damiendoligez]

@jmid: do you want to add an entry to the Changes file? I don't think it's necessary here, but if you want to get credit for these changes, that's the place.

[jmid]

Thanks! I've added a Changes entry, thinking that users of these may potentially like to know.
I added the entry to the 5.3 section, as I'm expecting this documentation fix to be cherry-picked to 5.3.

[Octachron]

Indeed, documentation fixes should be cherry-picked to 5.3 (at least until the first rc).

[kayceesrk]

Are we sure this should be “since”? It does not read right to me. The number of minor GCs since the last minor GC will be 0, but what quick_stat returns is different. How about:

(** Returns a record with the current values of the memory management 
    counters like [stat]. Unlike [stat], [quick_stat] does not perform a full major 
    collection, and hence, is much faster. However, [quick_stat] reports the 
    counters sampled at the last minor collection or at the end of the last major 
    collection cycle (whichever is the latest). Hence, the memory stats returned 
    by [quick_stat] are not instantaneously accurate. *)

[kayceesrk]

Also, in the original comment, I am not sure which field the program's total memory usage is supposed to represent (if at all). The documentation attached to stats refers to the program's total memory stats, while being vague, is a bit more accurate. I did wonder whether usage was supposed to be stats.

[jmid]

Sounds good to me!
The reordering of sentences also improve readability IMO, mentioning the central difference first and only secondly going into more detail.

[kayceesrk]

@jmid Would you be able to squash the commits into a single commit?

[jmid]

Sure, squashed and force pushed 👍

[kayceesrk]

Thanks

[jmid]

Indeed, documentation fixes should be cherry-picked to 5.3 (at least until the first rc).

Gentle reminder to cherry-pick this documentation fix to 5.3

[kayceesrk]

Gentle reminder to cherry-pick this documentation fix to 5.3

FYI @Octachron. Is there a label that we use to indicate that certain PRs are marked for cherry-picking to the release branch?

[gasche]

Please feel free to cherry-pick yourself after merging, there is documentation on the specific git incantation recommended at https://github.com/ocaml/ocaml/blob/trunk/HACKING.adoc#keep-merge-commits-when-merging-and-cherry-picking-github-prs

[Octachron]

A label could be helpful when checking that no bug fixes was forgotten. Maybe a whole set 5.3-bugfix, 5.3-docfix and 5.3-configfix?

[kayceesrk]

A label could be helpful when checking that no bug fixes was forgotten. Maybe a whole set 5.3-bugfix, 5.3-docfix and 5.3-configfix?

Alternatively, adding this to the 5.3 milestone may be better to avoid polluting the labels namespace with many different labels per release? I've done so now.

[jmid]

The milestone has the downside of marking things as 'Closed' once a PR is merged, thus risking to forget to cherry-pick 🤷