thread safe set implementation #666
Conversation
|
This is thread safe set implementation along the lines of thread safe Array |
|
@pitr-ch please review this as this is my first open source contribution. |
pitr-ch
added
the
enhancement
|
Thank you for your contribution! At first glance it looks good, however I'll have to return to it and give it more thought. Maybe @thedarkone could help. @thedarkone would you have time to help with the review? |
spec/concurrent/set_spec.rb
Outdated
| it 'concurrency' do | ||
| (1..THREADS).map do |i| | ||
| Thread.new do | ||
| set << i |
There was a problem hiding this comment.
Change this whole spec to look like Hash spec:
1000.times do |j| loop, just like for Hash).
|
@thedarkone, @pitr-ch can u please review the changes.. |
lib/concurrent/set.rb
Outdated
| # `Concurrent::Set`. It reads Set `a`, then it creates new `Concurrent::Set` | ||
| # which is concatenation of `a` and `b`, then it writes the concatenation to `a`. | ||
| # The read and write are independent operations they do not form a single atomic | ||
| # operation therefore when two `+=` operations are executed concurrently updates |
There was a problem hiding this comment.
Unfinished sentence: updates ... may be lost. Use #merge instead.
From
concurrent-ruby/lib/concurrent/array.rb
Line 21 in df482db
spec/concurrent/set_spec.rb
Outdated
| Thread.new do | ||
| 1000.times do |j| | ||
| set << i | ||
| set.empty? |
There was a problem hiding this comment.
This spec does not make set any expectations. Can you clarify what you are testing and make expectations about the behavior in the spec?
There was a problem hiding this comment.
@ianks actually this is my first ever contribution so i dont have much knowledge about minitests so can u plese help me out in this?
There was a problem hiding this comment.
Actually the tests follow what we have for Array and Hash. I'll improve all of them at least a bit.
|
@thedarkone @ianks @eregon Thanks you for helping with the review! |
ghost
assigned 
|
I've improved the tests a bit, updated documentation, squashed and rebased. After CI passes it can be merged. |
No description provided.