-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Promote objects more eagerly #49644
Promote objects more eagerly #49644
Conversation
Did you measure the impact of this change? |
I believe this was not done before since this will cause too many objects to be promoted to old. |
On the
For reference:
|
At least from those benchmarks it does seem to be a pretty clean win. |
So I would like to understand the trade-off better (especially in light of #49545 (comment)) IIUC we are trading length of short pauses for increased memory usage (and maybe more frequent pauses)? |
The issue there is that it seems we should do a full collection every once in a while but don't. |
428d249
to
2a6653d
Compare
One of the relevant metrics for our GC is our many times an object is scanned before reaching This PR reduces the number of mark passes until 3 passes seems overly conservative, and I think that we could keep our young generation smaller (e.g. shorter pauses in the incremental mark) by promoting objects more eagerly. Ideally, we should be more quantitative about this by doing a demographic analysis of our heap, but I'm not sure how feasible this is. |
The reasoning seems to make sense to me. I didn't realize we had essentially 2 age bits, because of the GC_OLD bit state operating that way as well. I think a broader question here still yet to be investigated is whether we can do even better if we keep this code, but split the older generation in 2, so that there is a young generation that promotes quickly (as this PR does), then a middle aged generation (using the age bits, or perhaps still without them), and finally an old generation (and then also system image / yoda generation too). With appropriate remset and other tracking metadata as relevant for each. The way that this transitions now alternatively makes me think we should take a hard look at whether the sweep phase is needed anymore, or could be lazy, since all it seems to be doing is swapping GC_MARKED for GC_OLD, which it seems that we could alternatively do by inverting the color for them. With 00 meaning young (or remset), 01 and 10 being marked or old (depending on that page's current color), and 11 being marked and old. |
Shall we merge this? |
I would say yes |
Will merge tomorrow if there are no objections. |
The function `jl_gc_internal_obj_base_ptr` takes a pointer and tries to determine if it is a valid object pointer. As such it has to carefully validate all data it reads, and abort whenever there are obvious inconsistencies. This patch adds a check which aborts when `meta->osize` is zero, just before we perform a division-with-remainder by this value, thus avoiding a potential division-by-zero exception. This fixes a crash we are seeing in our code. The crash did not happen before PR JuliaLang#49644 was merged because back then there was a check for `meta->ages` not being zero, which apparently was enough to detect invalid values for `meta` (e.g. when `meta` points into a null page).
The function `jl_gc_internal_obj_base_ptr` takes a pointer and tries to determine if it is a valid object pointer. As such it has to carefully validate all data it reads, and abort whenever there are obvious inconsistencies. This patch adds a check which aborts when `meta->osize` is zero, just before we perform a division-with-remainder by this value, thus avoiding a potential division-by-zero exception. This fixes a crash we are seeing in our code. The crash did not happen before PR #49644 was merged because back then there was a check for `meta->ages` not being zero, which apparently was enough to detect invalid values for `meta` (e.g. when `meta` points into a null page).
…uliaLang#54946) We don't store anything in the lowest two bits of `sz` after JuliaLang#49644.
…uliaLang#54946) We don't store anything in the lowest two bits of `sz` after JuliaLang#49644.
Before:
After: