Save thread context before yielding for GC #78
Conversation
src/gc-stock.c
Outdated
| @@ -2930,6 +2930,7 @@ size_t jl_maxrss(void); | |||
| // Only one thread should be running in this function | |||
| static int _jl_gc_collect(jl_ptls_t ptls, jl_gc_collection_t collection) | |||
| { | |||
| jl_gc_notify_thread_yield(ptls, NULL); | |||
There was a problem hiding this comment.
Do we need this call here, if the function is empty for Stock?
There was a problem hiding this comment.
Yes. It is empty. I just added this for clarification, as this is a place where a thread may yield. If the function impl is somehow not empty for stock in the future, this call will be useful. If you require this to be removed, or this is required to be removed when we upstream this, it can be removed.
I can put the above into a comment if you think that would be helpful.
There was a problem hiding this comment.
The comment might be useful indeed. Another question is, should we have that call in here instead for consistency? Or we would need to notify it twice for both times they call _jl_gc_collect (including that recollect phase)?
There was a problem hiding this comment.
Sounds good. I made the change.
This PR ports mmtk/mmtk-julia#159 to `dev`. The difference is that this PR adds a general call to the GC interface `jl_gc_notify_thread_yield`. In this case, each GC will do what they need in the call, and the context is saved in the GC specific TLS.
This PR ports mmtk/mmtk-julia#159 to
dev. The difference is that this PR adds a general call to the GC interfacejl_gc_notify_thread_yield. In this case, each GC will do what they need in the call, and the context is saved in the GC specific TLS.