search

FAForever / downlords-faf-client

Official client for Forged Alliance Forever
https://faforever.com
MIT License
196 stars 121 forks source link

Use ZGC for garbage collection #2951

Closed Sheikah45 closed 1 year ago

Sheikah45 commented 1 year ago

On my system noticed the OS reported memory usage was halved

codecov[bot] commented 1 year ago

Codecov Report

Merging #2951 (650f03c) into develop (b539935) will decrease coverage by 0.01%. The diff coverage is n/a.

:exclamation: Current head 650f03c differs from pull request most recent head cbcf770. Consider uploading reports for the commit cbcf770 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #2951 +/- ## ============================================= - Coverage 61.34% 61.33% -0.01% Complexity 4595 4595 ============================================= Files 557 557 Lines 20186 20186 Branches 1048 1048 ============================================= - Hits 12383 12381 -2 - Misses 7217 7219 +2 Partials 586 586 ``` [see 1 file with indirect coverage changes](https://app.codecov.io/gh/FAForever/downlords-faf-client/pull/2951/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FAForever) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/FAForever/downlords-faf-client/pull/2951?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FAForever). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FAForever) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/FAForever/downlords-faf-client/pull/2951?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FAForever). Last update [b539935...cbcf770](https://app.codecov.io/gh/FAForever/downlords-faf-client/pull/2951?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FAForever). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FAForever).
BlackYps commented 1 year ago

Halved? That's huge!

magge-faf commented 1 year ago

Yeah, that is amazing - have found no bugs so far, still testing it.

Sheikah45 commented 1 year ago

Yeah I ran through some tests with it. Still not entirely sure why. A lot of the memory used by the client is actually off heap, so it is rather hard to track down. So I was a little surprised when using the ZGC collector reported so much less memory usage. I am still investigating some as to why hence why this is not merged but figured I would send it out to be tested so others could confirm if it happened for them as well.

Sheikah45 commented 1 year ago

And previously this "unknown" memory segment was partly due to the Webviews. But using the ZGC the os reported memory actually correlates with the heap+nonheap memory. Where as before ~500MB were due to either the GC or some other native memory purpose. But I do not know why anything other than the memory of the GC would change just from the GC change.