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

Add additional fields to GC:ProfStats #13734

Merged

Conversation

carlhoerberg
Copy link
Contributor

@@ -66,7 +66,9 @@ module GC
gc_no : UInt64,
markers_m1 : UInt64,
bytes_reclaimed_since_gc : UInt64,
reclaimed_bytes_before_gc : UInt64
reclaimed_bytes_before_gc : UInt64,
expl_freed_bytes_since_gc : UInt64,
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is is the same naming as Boehm but should this be more clear

maybe something like explicitly_freed_bytes_since_gc

Copy link
Member

Choose a reason for hiding this comment

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

We're going with the library names everywhere else, so it's fine to keep that here as well.

@oprypin
Copy link
Member

oprypin commented Aug 10, 2023

Something to consider: as I understand, anyone who has libgc < 8 will now experience a crash? Or was it already unsupported?

@carlhoerberg
Copy link
Contributor Author

carlhoerberg commented Aug 10, 2023 via email

@HertzDevil
Copy link
Contributor

HertzDevil commented Aug 10, 2023

The function is not supposed to break because it requires passing sizeof(LibGC::ProfStats) as an argument

@beta-ziliani beta-ziliani added this to the 1.10.0 milestone Aug 10, 2023
@straight-shoota straight-shoota changed the title Boehm GC ProfStats has new additional fields Add additional fields to GC:ProfStats Aug 14, 2023
@straight-shoota straight-shoota merged commit 14d8fb2 into crystal-lang:master Aug 14, 2023
53 checks passed
Blacksmoke16 pushed a commit to Blacksmoke16/crystal that referenced this pull request Dec 11, 2023
This pull request was closed.
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.

7 participants