-
Notifications
You must be signed in to change notification settings - Fork 9
feat(worker): add requestgc api #88
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
base: main
Are you sure you want to change the base?
Conversation
Initiate the worker auto gc every `ENV["MALT_AUTO_GC_SECONDS"]` or 900 seconds. | ||
""" | ||
function autogc(w::Worker) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can call this once, to turn on the auto gc loop, right? And subsequent calls are ignored? I think that's nice API!
We should document this, and maybe a different name would make this more clear. E.g. start_background_auto_gc
(I love long names but feel free to get creative)
""" | ||
Malt.requestgc(w::Worker) | ||
Request a garbage collection on the worker `w`. This is a non-blocking call (on the worker). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add in these docs: "For a blocking GC, you can use Malt.remote_call_wait(GC.gc, w)
."
I think the sentence This is a non-blocking call (on the worker).
could be expanded a bit more, and be more technical about timing and throttling
|
||
const _channel_cache = Dict{UInt64,AbstractChannel}() | ||
const _gc_event = Base.Event() | ||
const _gc_task = Threads.@spawn begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider Threads.@spawn :interactive
? I don't know which is better here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really nice!
We should check whether this works and is useful (before spending time on the finishing steps). So either in Pluto, PlutoSliderServer or Quarto, let's apply this and check that it is beneficial.
I think this still needs a test (at least a sanity test that just calls the functions). Maybe you could check those @debug
logs to see if throttling works?
We also need to update the Documenter.jl page.
No description provided.