Skip to content

Refactor GC stats structs and names #50772

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

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented Aug 2, 2023

On top of #50682

@gbaraldi gbaraldi changed the title Add fallback if we have make a weird GC decision. Refactor GC stats structs and names Aug 2, 2023
@NHDaly NHDaly requested a review from d-netto August 3, 2023 01:00
@gbaraldi gbaraldi removed the request for review from NHDaly August 3, 2023 01:07
@gbaraldi gbaraldi force-pushed the accounting-refactor branch from 3e19377 to 859c5b4 Compare August 3, 2023 14:46
@brenhinkeller brenhinkeller added the GC Garbage collector label Aug 3, 2023
size_t localbytes = jl_atomic_load_relaxed(&ptls->gc_num.allocd) + gc_num.interval;
jl_atomic_store_relaxed(&ptls->gc_num.allocd, -(int64_t)gc_num.interval);
static_assert(sizeof(_Atomic(uint64_t)) == sizeof(gc_num.deferred_alloc), "");
jl_atomic_fetch_add((_Atomic(uint64_t)*)&gc_num.deferred_alloc, localbytes);
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we don't need anything like this anymore? When gc is disabled, we will start hitting a point where jl_gc_collect is called and jl_gc_disable_counter is atomically loaded on every allocation. Is that ok?

Copy link
Member Author

Choose a reason for hiding this comment

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

So this is one of the things I'm not super sure. One thing that we could do here is to set the next collection to a very high value so we don't try to enter it at all. And then forcibly run a gc when we reanable it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The other is to update it with a simple default algorithm so that it does trigger but not so often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GC Garbage collector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants