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

Testing scavenger tenuring #588

Merged
merged 11 commits into from
May 3, 2023
Merged

Testing scavenger tenuring #588

merged 11 commits into from
May 3, 2023

Conversation

PalumboN
Copy link
Collaborator

With @jordanmontt we started to profile the GC activity, especially the Scavenger.

Looking at the code, we realised that the Tenuring algorithm was not tested.
Then we realise that the Scavenger had a bug setting the tenureThreshold, so the tenuring is not working as expected.

In this PR:

  • Tenuring algorithm is tested (a bit)
  • tenureThreshold computation was fixed (it was setting it in the wrong semi-space)
  • Renaming some methods for clarity (the uses of these methods are strange, we need to improve the API with the image)

Copy link
Member

@guillep guillep left a comment

Choose a reason for hiding this comment

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

Good! Thanks

smalltalksrc/VMMaker/SpurGenerationScavenger.class.st Outdated Show resolved Hide resolved
smalltalksrc/VMMaker/SpurGenerationScavenger.class.st Outdated Show resolved Hide resolved
smalltalksrc/VMMaker/SpurGenerationScavenger.class.st Outdated Show resolved Hide resolved
@guillep
Copy link
Member

guillep commented Apr 26, 2023

There are some perm space test failing, are they related?

@guillep
Copy link
Member

guillep commented Apr 26, 2023

There are some perm space test failing, are they related?

I've seen the same tests fail in another PR, I don't think they are related to this one

@PalumboN
Copy link
Collaborator Author

PalumboN commented May 2, 2023

Here is a chart with the before and after 🤔

image

@guillep
Copy link
Member

guillep commented May 3, 2023

the dots are the scavenge thresholds? this means that we are periodically wrongly tenuring 2MB of objects in this scenario, right?

@guillep
Copy link
Member

guillep commented May 3, 2023

also the second graph required 2/3 of the scavenges...

@PalumboN
Copy link
Collaborator Author

PalumboN commented May 3, 2023

Yes!

@PalumboN
Copy link
Collaborator Author

PalumboN commented May 3, 2023

The bench of the second graph was shorter (it was in interactive mode), so I don't know the impact on the amount of scavenges.

@guillep
Copy link
Member

guillep commented May 3, 2023

Thanks!

@guillep guillep merged commit ac59054 into pharo-10 May 3, 2023
@guillep guillep deleted the testing-scavenger-tenuring branch May 4, 2023 13:24
@tesonep tesonep mentioned this pull request May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants