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.

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

fix a top_heap_words bug in GC statistics #11671

Merged
merged 1 commit into from Oct 28, 2022

Conversation

gasche
Copy link
Member

@gasche gasche commented Oct 27, 2022

This small change exactly follows @lthls's analysis in #11663, and fixes the repro case in the issue report.

For reference, Vincent's analysis of the bug:

If you look at the code of caml_compute_gc_stats, pool_max_words is very coarsely approximated by taking the sum of the maximums of all the currently domains... but not the orphaned stats.
So you have two domains, one of them terminates and orphans its shared heap, and the program terminates without any other domain adopting the pools. The result is that the reported heap words represent the words from both the main domain and the orphaned pools, while the reported max heap words only takes into account the words from the main domain..
I suspect this can be fixed by adding the orphaned heap stats' max words to the accumulators before looping on the domains.

fixes ocaml#11663

Suggested-by: Vincent Laviron <vincent.laviron@gmail.com>
Copy link
Contributor

@abbysmal abbysmal left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Thank you very much for the fix, should I also cherry pick this to 5.0?

@abbysmal abbysmal merged commit cc3d9cb into ocaml:trunk Oct 28, 2022
@gasche
Copy link
Member Author

gasche commented Oct 28, 2022

should I also cherry pick this to 5.0?

Yes, I think it would be nice.

@kayceesrk kayceesrk added this to the 5.0 milestone Oct 28, 2022
abbysmal added a commit that referenced this pull request Oct 28, 2022
fix a top_heap_words bug in GC statistics

(cherry picked from commit cc3d9cb)
@abbysmal
Copy link
Contributor

Cherry-picked cc3d9cb to 5.0 at 2f31f0f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants