Yes, I agree. Please file a ticket. -- Yakov Zhdanov, Director R&D *GridGain Systems* www.gridgain.com
2015-08-31 21:29 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > Yes, this is what I am talking about. Actually, for "busy" lock semantics > we need only 3 methods in regular RW lock terms: tryReadLock(), > readUnlock() and writeLock(). No need for reentrancy, write unlocks, etc.. > > This makes potential implementation very simple: read lock/unlock() methods > are simply atomic increment/decrement, and writeLock is a CAS-loop. > > Then we stripe it using either random (as it is done in > Striped64/LongAdder) or using sequece numbers. The latter approach could be > better given that the vast majority of Ignite's internal logic happens in > our own thread pools. If we assign each thread from a pool different stipe > number, they will work without any contention at all. > > On Mon, Aug 31, 2015 at 9:17 PM, Yakov Zhdanov <yzhda...@gridgain.com> > wrote: > > > Vova, can you please share your benchmark? That was long time ago and I > > recall that I got better results in more complex benchmarks than jmh one. > > > > However, I am quiet excited with your idea to change implementation to > > striped RW locks: > > > > * busy state - we do tryLock() on random read lock > > * blocked state - since this is very rare, we can acquire write locks on > > each stripe. > > > > Is this what you've meant? > > > > -- > > Yakov Zhdanov, Director R&D > > *GridGain Systems* > > www.gridgain.com > > > > 2015-08-31 21:06 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > > > > > On Mon, Aug 31, 2015 at 11:03 AM, Vladimir Ozerov < > voze...@gridgain.com> > > > wrote: > > > > > > > Exactly, we use it primarily as "busy" lock, i.e. lots of concurrent > > > > readers with writer blocking everything on node stop. But it doesn't > > > > outperform regular ReentrantReadWriteLock actually. > > > > > > > > > > I don't think this use-case is about performance. Yakov, can you jump > in > > > and remind us why we use the spin locks on Ignite instance stop? > > > > > > > > > > > > > > On Mon, Aug 31, 2015 at 6:41 PM, Dmitriy Setrakyan < > > > dsetrak...@apache.org> > > > > wrote: > > > > > > > > > I don't recall exactly, but from what I remember, there were other > > > > benefits > > > > > to the spin-lock approach. Don't we use some characteristics of > this > > > lock > > > > > to properly shut down the system? > > > > > > > > > > D. > > > > > > > > > > On Mon, Aug 31, 2015 at 5:24 AM, Vladimir Ozerov < > > voze...@gridgain.com > > > > > > > > > wrote: > > > > > > > > > > > Igniters, > > > > > > > > > > > > We have two pretty strange constructs: GridSpinReadWriteLock and > > base > > > > on > > > > > it > > > > > > GridSpinBusyLock. > > > > > > As I understand it was an effort to create more performant RWLock > > > than > > > > > > ReentrantReadWriteLock > > > > > > for cases when wrtie locks are very unlikely. > > > > > > > > > > > > As busy lock concept is also used in some sensitive places of > > > > "platforms" > > > > > > module, I measured performance of read lock-unlock cycles for > both > > > > > > ReentrantReadWriteLock > > > > > > and GridSpinReadWriteLock using JMH. > > > > > > > > > > > > Our implementation doesn't offer any perform benefits comparing > to > > > > > > ReentrantReadWriteLock, their performance are almost equal. This > > > makes > > > > > > sense, because essentailly both locks just CASes on a shared > > variable > > > > to > > > > > > obtain the read lock. Looks like we can safely remove these > > > "spinners" > > > > > and > > > > > > use ReentrantReadWriteLock instead. > > > > > > > > > > > > More serious perfomance gain can be achieved if we stripe the > lock > > > > (e.g. > > > > > > like it is done in LongAdder) thus decreasing contention on > shared > > > > > > variables. Quick experiments shown 5x throughput increase for > read > > > > > > lock-unlock cycles when lock is striped. > > > > > > > > > > > > Thoughts? > > > > > > > > > > > > Vladimir. > > > > > > > > > > > > > > > > > > > > >
