Clear thread state to ensure threads local state don't keep references #1046
Conversation
We're surely running into JuliaLang/julia#40626. This is my attempt to clear thread's local storage after multithreaded parsing.
Codecov ReportBase: 89.75% // Head: 89.74% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1046 +/- ##
==========================================
- Coverage 89.75% 89.74% -0.02%
==========================================
Files 9 9
Lines 2246 2253 +7
==========================================
+ Hits 2016 2022 +6
- Misses 230 231 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| (obj::_Returns)(@nospecialize(args...); @nospecialize(kw...)) = obj.value | ||
|
|
||
| function clear_thread_states() |
There was a problem hiding this comment.
@quinnj - can you please explain why this is enough to clear thread states?
There was a problem hiding this comment.
It's detailed in the JuliaLang/julia#40626 issue. Each thread basically has a thread local storage where they will store the most recent Task that it ran, which includes a reference to the result of the task that ran. So to ensure each thread's local state is cleared, we run a dummy no-op task on each thread (via :static) to ensure no references remain to items that should be candidates for GC.
| # end up getting stuck in thread local storage | ||
| # running clear_thread_states clears out any thread local storage tasks | ||
| struct _Returns{V} <: Function |
There was a problem hiding this comment.
why do you define _Returns instead of using Base.Returns?
There was a problem hiding this comment.
I ran into compat issues with 1.6; maybe I just needed to do Base.Returns instead and it was exported starting in 1.7?
There was a problem hiding this comment.
Ah yes, docs states:
Returns requires at least Julia 1.7.
|
I believe this broke multithreaded parsing within You can replicate by doing any asyncmap(x -> CSV.read("hw_25000.csv", DataFrame; ntasks=3), 1:3; ntasks=3) |
|
Looks like that fixed it! Thanks! Btw I just pulled the CSV file from https://people.sc.fsu.edu/~jburkardt/data/csv/csv.html; my example above should be fully replicable. I originally demonstrated the issue using a bare Might be worth adding a test. Although I don't think it's likely this exact problem with reoccur, I don't see any tests that involve |
|
Ah that makes sense. Thanks for confirming. Yeah, I'll look into adding a test. |
We're surely running into JuliaLang/julia#40626.
This is my attempt to clear thread's local storage after multithreaded parsing.