Don't spawn a task per file to include, use a pool of tasks instead #225
Conversation
Drvi
force-pushed
the
td-include-tasks
branch
from
b1e24c7 to
9da139d
Compare
src/ReTestItems.jl
Outdated
| end | ||
| end | ||
|
|
||
| walkdir_channel = Channel{Tuple{String, FileNode}}(1024) |
There was a problem hiding this comment.
why 1024? (Please add a comment)
There was a problem hiding this comment.
No reason it has to be 1024 exactly. Usually one would use Inf (i.e. 9223372036854775807) as the limit, but realistically, you don't want this to be your limit, this number of elements would OOM us, surely. 1024 seemed like a more reasonable limit.
src/ReTestItems.jl
Outdated
| @spawn walkdir_task( | ||
| $walkdir_channel, $project_root, $root_node, $ti_filter, $paths, $projectfile, $report, $verbose_results | ||
| ) | ||
| for _ in 1:clamp(2*(nthreads()-(nthreads() == 1)), 1, 16) # 1 to 16 tasks, 1 if single-threaded |
There was a problem hiding this comment.
why this formula? why 2x nthreads? Since this is no longer a function of number of files, i guess we need a cap... but shouldn't the cap be a function of the number of threads (rather than fixed to 16)?
Also, what's the worst case scenario for this new formula? how would it compare to the old formula of a task per file?
we should document somewhere (as a comment here?) that we spawn N include_tasks for perf reasons, and then explain how the formula was determined
There was a problem hiding this comment.
Right, this is really tricky and this formula probably isn't optimal... my intuition is that I usually get better performance when I have more tasks than cores (hence the 2x), but in this case, unfortunately, each task is going make a lot of dynamic allocations (parsing allocates and eval-ing allocates a lot) and this means GC is likely to run and when GC runs, all threads are going to be waiting, so there is a break-even point where adding more tasks doesn't help performance because it makes GC pauses worse.
I didn't really experiment with the formula as it seemed "good enough"
2 participants
Uh oh!
There was an error while loading. Please reload this page.