-
Notifications
You must be signed in to change notification settings - Fork 9
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 concurrency safe iterator wrapper ChannelLike #121
Conversation
Very nice idea, I had wanted something like this before. |
e39a1d6
to
d4975b8
Compare
d4975b8
to
94a5e75
Compare
Ok, cleaned it up and added some docs. If you prefer to keep it internal we can do that, but it isn't really much code to maintain. On the other hand, perhaps this isn't a package of "generally useful things". Let me know what you think. Posting the benchmarks here for posterity since I removed the file with them from the commit: Benchmark: using OhMyThreads: tmapreduce, GreedyScheduler
using BenchmarkTools: @btime
function mc_parallel(N; ntasks, chunksize)
scheduler = GreedyScheduler(; ntasks, chunksize, split = :consecutive)
M = tmapreduce(+, 1:N; scheduler) do i
rand()^2 + rand()^2 < 1.0
end
pi = 4 * M / N
return pi
end
const N = 100_000_000
for chunksize in (10, 1_000, 100_000), ntasks in (1, 2, 4)
@btime mc_parallel(N; ntasks = $ntasks, chunksize = $chunksize)
end PR:
master:
|
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.
LGTM and I'm fine with documenting this as API. However, I think we need to modify the GreedyScheduler
docstring which currently mentions Channel
explicitly. Probably we shouldn't mention the specific implementation of the "storage" at all.
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #121 +/- ##
==========================================
- Coverage 90.24% 85.81% -4.44%
==========================================
Files 3 7 +4
Lines 82 578 +496
==========================================
+ Hits 74 496 +422
- Misses 8 82 +74 ☔ View full report in Codecov by Sentry. |
BTW, could perhapse make sense to mention this in the docs under https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#Another-safe-way-based-on-Channel. |
94a5e75
to
4bc8088
Compare
Added to changelog and updated the GreedyScheduler docstring. |
Unfortunately, we mention "channel" more times in the docstring in the keyword arguments section. Would be great if you could "fix" this. |
4bc8088
to
88611e5
Compare
Done.
Fixed, replaced with "shared workqueue". |
88611e5
to
f0d9902
Compare
`ChannelLike` wraps an indexable object such that it can be iterated by concurrent tasks in a safe manner similar to a `Channel`. This is instead of `Channel` in the chunked `GreedyScheduler`.
f0d9902
to
1776729
Compare
Just to show that the idea from JuliaFolds2/ChunkSplitters.jl#49 can be used to improve the (chunked) GreedyScheduler.
The advantages are that the iterator doesn't have to be copied into a Channel, and that the AtomicChunk-thingy is more efficient (but not as general, of course) than a Channel. See some benchmarks in the test.jl file.