Skip to content
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

Add parallel marking to GC #41760

Closed
wants to merge 2 commits into from
Closed

Conversation

jpsamaroo
Copy link
Member

This is a first pass at multithreading parts of GC marking. The intended approach is simple: Store a "recruitment location" (function address) into a global, and have non-leader threads (those not running _jl_gc_collect directly) check for this global to be set to non-NULL. Threads that encounter a valid recruitment location will set their GC state to the new JL_GC_STATE_PARALLEL (name subject to change), and will call into the recruitment location. The only available recruitment function, gc_mark_loop_recruited, will put the thread to work in gc_mark_loop, assuming that the GC leader thread setup some work for this thread to do.

I'm posting this PR early because I need some help figuring out what I'm doing wrong. So far, the recruitment mechanism seems to work well, but I haven't figured out what things I need to setup for each thread before letting them enter the mark loop.

I've also got some code commented out, which would (presumably, if it worked) allow threads to steal work from each other. That's probably not required for this PR, but I started on it in case it might be useful.

N.B. I am bad at atomics, thread safety, and GC semantics, so please point out what terrible things I have done here!

@vchuravy @vtjnash

@jpsamaroo
Copy link
Member Author

@yuyichao what's the logic behind the check

julia/src/gc.c

Lines 2706 to 2707 in 5118a1b

if (__unlikely(!jl_is_datatype(vt)))
gc_assert_datatype_fail(ptls, vt, sp);
? I'm hitting this a lot, and I'm not sure I understand the reason why it can fail.

@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #41760 (5118a1b) into master (2fbeef8) will increase coverage by 0.03%.
The diff coverage is 77.91%.

❗ Current head 5118a1b differs from pull request most recent head 7f4646d. Consider uploading reports for the commit 7f4646d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #41760      +/-   ##
==========================================
+ Coverage   80.02%   80.06%   +0.03%     
==========================================
  Files         350      350              
  Lines       73943    74011      +68     
==========================================
+ Hits        59170    59254      +84     
+ Misses      14773    14757      -16     
Impacted Files Coverage Δ
base/compiler/validation.jl 5.45% <0.00%> (-0.06%) ⬇️
base/libuv.jl 68.25% <ø> (ø)
base/set.jl 84.93% <ø> (ø)
stdlib/Distributed/src/managers.jl 44.77% <0.00%> (+10.44%) ⬆️
stdlib/InteractiveUtils/src/macros.jl 89.18% <ø> (ø)
stdlib/REPL/src/REPLCompletions.jl 25.58% <0.00%> (ø)
base/compiler/abstractinterpretation.jl 70.36% <48.14%> (-0.35%) ⬇️
base/compiler/typelattice.jl 81.33% <50.00%> (ø)
base/io.jl 88.18% <66.66%> (-0.71%) ⬇️
stdlib/Distributed/src/cluster.jl 71.94% <70.00%> (+6.20%) ⬆️
... and 69 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fbeef8...7f4646d. Read the comment docs.

@yuyichao
Copy link
Contributor

yuyichao commented Aug 5, 2021

It just means that you are reading garbage somehow.

@kshyatt kshyatt added the GC Garbage collector label Nov 16, 2021
@ViralBShah
Copy link
Member

Ok to close this?

@jpsamaroo
Copy link
Member Author

@ViralBShah we have someone actively working on getting this PR up to date, who may end up pushing to this PR or another one (in which case we'll supersede this). I'd like to keep this open until that happens, just so people have something to watch for progress updates.

@d-netto d-netto mentioned this pull request Mar 16, 2022
3 tasks
@jpsamaroo
Copy link
Member Author

Superseded by #44643

@jpsamaroo jpsamaroo closed this Mar 16, 2022
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.

4 participants