[Mondrian] Improving concurrency on Segment.getCellValue()

Eric McDermid mcdermid at stonecreek.com
Mon Dec 1 21:17:42 EST 2008

One of my clients uses Mondrian in a situation that requires a fairly  
high level of concurrency, and is seeing a slowdown that is at least  
partially caused by reader-v-reader contention on  
Segment.getCellValue().  That is, because the method itself is  
synchronized, multiple readers queue up to go through even though no  
writes are occuring.

That seems less than ideal, especially since there's no obvious reason  
multiple readers shouldn't be able to execute Segment.getCellValue()  
at the same time.  I've been doing some analysis in the hopes of  
finding a way to allow greater read concurrency with minimal  
disruption to existing access patterns, and could use some feedback.

== Overview ==

Segment's method-level synchronization is apparently there to guard  
the internal reference to the SegmentDataset ("data") and the  
associated segment state ("state", which is either "Loading", "Ready"  
or "Failed").  The data reference starts out as null, may be set by  
setData(), and should not change again once set.  Similarly, state  
starts out as Loading, and will eventually be changed to either Ready  
(by setData()) or Failed (by setFailIfStillLoading()).

For reference, the synchronized methods of Segment and their  
operations on data and state are:

	reads/writes data
	reads/writes state
	reads data
	reads/writes state
	reads data
	reads state

Additionally, there are several other unsynchronized methods of  
Segment that read either data or state:

	reads data
	reads data
	reads state
	reads state
	reads state

(I suspect all of these latter methods really should be guarded.   
Though I haven't dug deep enough to be sure their usages aren't  
somehow guarded elsewhere, they don't appear to be.   The last two in  
particular may not present much actual risk, though:  isLoading() is  
private and only called by waitUntilLoaded(), and isFailed() is only  
called from SegmentLoaderTest.)

  == Approaches ==

I've been mulling over a couple of ideas to solve the problem:

1.  Replace method synchronization in Segment with a CountdownLatch  
for data, and a ReentrantReadWriteLock for state.

In this approach, we'd create a CountdownLatch of count 1 when the  
segment is created.  Each reader would call await() on the latch  
before being allowed to access the data reference.  Both getData() or  
setFailIfStillLoading() would decrement the latch, allowing readers to  
proceed.  Even though setFailIfStillLoading() doesn't modify data, it  
still needs to release any waiting readers if it is to match current  

Since both setData() and setFailIfStillLoading() can change the value  
of state, ReentrantReadWriteLock seems to be the right fit here.  Each  
reader would be responsible for acquiring the read lock before reading  
the value of state, and setData() and setFailIfStillLoading() would  
need the write lock before writing.

This approach is fairly simple and encapsulates locking behavior  
within a single class, at the cost of requiring some lock overhead for  
every read.

2.  Delay access to the segment using a synchronized wrapper while it  
is loading, then swap out the reference for an unsynchronized  
immutable version

Since attempting to read the data reference while it is still loading  
is a fairly rare occurrence, another option may be able to split each  
Segment object into two objects that share the current Segment  
interface: a synchronized wrapper object (let's call its class  
"LoadingSegment") and an unsynchronized immutable object that is only  
constructed once the underlying SegmentDataset is ready (call its  
class "LoadedSegment").

Where we currently create the Segment and add it to Aggregation's  
SegmentRefs list, we'd create and add LoadingSegment.  Initially,  
LoadingSegment would block all calls that can't be completed without  
the data reference.  Once we are able to construct LoadedSegment, we  
pass a reference to it to LoadingSegment, which then allows pending  
calls to proceed and delegates their execution to LoadedSegment.   
Additionally, we replace the LoadingSegment reference in Aggregation's  
list with LoadedSegment, ensuring that any future calls to  
getCellValue() incur no synchronization cost.

In LoadedSegment, methods such as setData() and  
setFailIfStillLoading() would just raise assertions, while  
waitUntilLoaded() would return immediately.  Other methods not reliant  
on data or state such as wouldContain() would be moved to a common  
abstract base class.

== Assessment ==

I see definite advantages to approach #2, particularly in terms of  
helping to guarantee the "no modifications once set" behavior that is  
currently just implied, as well as in providing lock-free read access  
to the underlying data.

Practically speaking, though, I currently lean toward approach #1.   
It's simpler to implement (we'd have to use most if not all of  
approach #1 to manage locking in LoadingSegment anyway) and allows us  
to document/enforce all concurrency expectations and guarantees within  
a single class.  Initial experiments also suggest it makes a big  
difference in the application I'm currently working on.

While #1 may not be as efficient in some very high concurrency  
applications as #2, it's unclear just how big a performance difference  
there would be.  In such a case, I'm inclined to go for the simpler  
solution until we see that even this level of concurrency overhead is  
actually causing problems.

Does anyone have a distinct preference for one over the other, or  
perhaps an alternate approach I should consider?

More information about the Mondrian mailing list