Only run GC 3 times instead of 4 to scrub#350
Merged
Conversation
Per slack, 'Diogo changed it so it only needs 3 to promote everything to old generation now'.
Collaborator
|
Diogo Netto liked the comment on Slack so I'll take that as a green flag. Have asked him for a review nonetheless to avoid screwing up |
|
Olá boa tarde eu não entendi direito o sentido da notificação?
Em qua., 3 de jan. de 2024 14:08, Guillaume Dalle ***@***.***>
escreveu:
… @Diogo-netto <https://github.com/Diogo-Netto> liked the comment on Slack
so I'll take that as a green flag
—
Reply to this email directly, view it on GitHub
<#350 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BABOSY45WBJVJPTICGV4VHTYMWGCBAVCNFSM6AAAAABBIFJTIWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZVGY4TKMJTGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Collaborator
|
Sorry I tagged the wrong Diogo Netto 🤣 |
|
Sem problemas 😃
Em qua., 3 de jan. de 2024 14:50, Guillaume Dalle ***@***.***>
escreveu:
… Sorry I tagged the wrong Diogo Netto 🤣
—
Reply to this email directly, view it on GitHub
<#350 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BABOSY4HW5YWSGBKXGDYYLLYMWLAHAVCNFSM6AAAAABBIFJTIWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZVG42DMOJVGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
d-netto
reviewed
Jan 4, 2024
There was a problem hiding this comment.
If the goal of gcscrub is to ensure all objects reach the oldest generation, then this PR conforms with the changes we introduced in the GC in v1.10.
Want to note that since you are relying on internal GC behavior here (which we're free to change in any version), then it might be a good idea to guard these changes under a version check: i.e. put a @static if VERSION=1.10 or alike.
Collaborator
|
@Zentrik wanna add that to make sure that on <= 1.9 we do 4 sweeps and on >= 1.10 we do 3? |
gdalle
approved these changes
Jan 4, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Per slack, 'Diogo changed it so it only needs 3 to promote everything to old generation now'.