On 9 Jul 2009, at 11:37, Galder Zamarreno wrote:
On 07/09/2009 11:55 AM, Manik Surtani wrote:
>
> On 8 Jul 2009, at 19:53, Jason T. Greene wrote:
>
>> Manik Surtani wrote:
>>> * Make the concurrent map volatile
>>> * When iterating, first create a new ConcurrentMap and replace the
>>> old one with the new one so all concurrent threads write to the
>>> new Map
>>> * Iterate over the old map
>>
>> That would lead to race conditions since a concurrent writing thread
>> could write to the "old" map, either by getting a recent yet
>> incorrect
>> read off the volatile, or reading it right before it changes.
>
> True, since referencing the map and writing to it isn't atomic.
>
> We could guard access to the map with a read/write lock. Safe, if a
> little heavy-handed... map writers would acquire a RL (since we want
> concurrent access here) but the async flushing thread would need to
> wait
> for a WL to swap the map reference, releasing the lock after the map
> reference has been swapped.
Yeah, I don't think it could be volatile because it only applies to
the variable itself, not the contents pointed at. The R/W lock
approach looks like a valid one and better than using synchronized
blocks. Another thing I had in my mind is whether we need the copy
to be a ConcurrentMap, we could probably just use a
Immutables.immutableMapCopy().
Sync blocks would suck since it is effectively an exclusive lock. We
want non-exclusive (read) locks for threads writing to this map - we
want multiple application threads to do this concurrently otherwise
this becomes a bottleneck. And hence the need for a ConcurrentMap.
And we don't make a copy at all - all the async flushing thread does
is that it creates a new, empty ConcurrentMap and swaps the 'async
queue' Map so that application threads can continue logging their
changes somewhere while old changes can be iterated through and
flushed. And for this, the field still needs to be volatile (not
strictly so, since we can rely on the fencing that will happen in the
lock as a side-effect)
I looked at the two implementations (ConcurrentHashMap(Map)
constructor vs Immutables.immutableMapCopy() which would
Object.clone() on the map) but I'm not sure which one would be
faster. Since clone() would rely on a native clone() impl, I'd
imagine that to be faster.
Like I said, we don't need a copy. We need a new, empty map for
threads to log changes to while the old one's being flushed.
> --
> Manik Surtani
> manik(a)jboss.org
> Lead, Infinispan
> Lead, JBoss Cache
>
http://www.infinispan.org
>
http://www.jbosscache.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache
--
Manik Surtani
manik(a)jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org