The most efficient way is not to lock at all :)
See my updated answer in Code Review: you don't need a ReadWriteLock
, hence you don't need to lock the reads.
When you read (in Thread 1 for example), you get a snapshot of what's in the cache at that moment. If you have concurrent modifications (in Thread 2), even if you use locks, the content of the cache might have changed before Thread 1 finishes its computation and the lock doesn't buy you anything:
Thread 1 Thread 2
| |
getAllPeers |
| addPeer
Do something with |
the Peers but not |
the added one |
| |
So your implementation of getAllPeers()
is fine as it is.
As a side note, if you need to lock all the stripes in a Striped<Lock>
, you could do it using getAt()
, but the naive way could get you into trouble if one of the lock()
calls throws an unchecked exception (which is permitted):
for (int i = 0, size = stripes.size(); i++; i < size) {
stripes.getAt(i).lock();
}
try {
// Do something
} finally {
for (int i = 0, size = stripes.size(); i++; i < size) {
stripes.getAt(i).unlock();
}
}
An alternative way would be to do it recursively, but it increases the length of the stack by the number of stripes, so you could get a StackOverflowException
if you had a large number of stripes:
public void doSomethingWithLocks() {
doSomethingWithLock(0);
}
private void doSomethingWithLock(int stripe) {
if (stripe < stripes.size()) {
Lock lock = stripes.getAt(stripe);
lock.lock();
try {
doSomethingWithLock(stripe + 1);
} finally {
lock.unlock();
}
} else {
doSomething();
}
}
private void doSomething() {
// Do something
}