<br><font size=2 face="sans-serif">Julian,</font>
<br>
<br><font size=2 face="sans-serif">Thanks for the review.</font>
<br>
<br><font size=2 face="sans-serif">I have checked in the Approach 2.</font>
<br>
<br><font size=2 face="sans-serif">>></font><font size=2 color=#000080 face="Verdana">Re.
our previous discussions on JDK 1.4 vs. 1.5. I understand that CopyOnWriteArrayList
is only available under 1.5. Can we use a factory method to control the
creation of this list, so that it can be made available in JDK 1.4 using
retroweaver?</font>
<br>
<br><font size=2 face="sans-serif">Retroweaver supports CopyOnWriteArrayList,
we have test it. (Our previous discussions was about </font><tt><font size=2>ReentrantReadWriteLock</font></tt><font size=2 face="sans-serif">)</font>
<br>
<br><font size=2 face="sans-serif">>></font><font size=2 color=#000080 face="Verdana">I
don't really understand the profiler results. They seem to be improved
in general, but there is a large number of blocks for HashMap in approach
1. What caused that?</font>
<br>
<br><font size=2 face="sans-serif">I didn't spend time on optimizing/analysing
the approach 1 because we were inclined towards approach 2. </font>
<br>
<br><font size=2 face="sans-serif">>></font><font size=2 color=#000080 face="Verdana">What
is the high-level locking strategy? In particular, if multiple objects
need to be locked (say Aggregation and Segment), is there a well-defined
locking order.</font>
<br><font size=2 face="sans-serif">We have removed all locking on Aggregation(Segment
list is synchronized by itself), we are locking only on Segment and
all the locking is done by method level synchronization, so we don't need
to bother about explicit locking before any call. </font>
<br>
<br><font size=2 face="sans-serif">>></font><font size=2 color=#000080 face="Verdana">
please document the locking strategy in the code</font>
<br><font size=2 face="sans-serif">Updated details about fields and methods
which are newly synchronized.</font>
<br>
<br><font size=2 face="sans-serif">- Thiyagu</font>
<br><font size=2 face="sans-serif"> </font>
<br>
<br>
<br>
<table width=100%>
<tr valign=top>
<td width=40%><font size=1 face="sans-serif"><b>"Julian Hyde"
<julianhyde@speakeasy.net></b> </font>
<br><font size=1 face="sans-serif">Sent by: mondrian-bounces@pentaho.org</font>
<p><font size=1 face="sans-serif">11/06/2007 13:15</font>
<table border>
<tr valign=top>
<td bgcolor=white>
<div align=center><font size=1 face="sans-serif">Please respond to<br>
Mondrian developer mailing list <mondrian@pentaho.org></font></div></table>
<br>
<td width=59%>
<table width=100%>
<tr valign=top>
<td>
<div align=right><font size=1 face="sans-serif">To</font></div>
<td><font size=1 face="sans-serif">"'Mondrian developer mailing list'"
<mondrian@pentaho.org></font>
<tr valign=top>
<td>
<div align=right><font size=1 face="sans-serif">cc</font></div>
<td>
<tr valign=top>
<td>
<div align=right><font size=1 face="sans-serif">Subject</font></div>
<td><font size=1 face="sans-serif">RE: [Mondrian] Re: Optimizing Sync Blocks
in Aggregation class</font></table>
<br>
<table>
<tr valign=top>
<td>
<td></table>
<br></table>
<br>
<br>
<br><font size=2 color=#000080 face="Verdana">Thiyagu,</font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">Both approaches seem to be
an improvement. Approach 2 seems to be better, especially if the only mutable
state in Aggregation is the list of Segments.</font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">Re. our previous discussions
on JDK 1.4 vs. 1.5. I understand that CopyOnWriteArrayList is only available
under 1.5. Can we use a factory method to control the creation of this
list, so that it can be made available in JDK 1.4 using retroweaver? </font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">I don't really understand
the profiler results. They seem to be improved in general, but there is
a large number of blocks for HashMap in approach 1. What caused that?</font>
<br><font size=2 color=#000080 face="Verdana"> </font>
<br><font size=2 color=#000080 face="Verdana">What is the high-level locking
strategy? In particular, if multiple objects need to be locked (say Aggregation
and Segment), is there a well-defined locking order.</font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">When you implement this,
please document the locking strategy in the code. Maybe describe which
fields are mutable and when, which classes to lock and in what order. Less
is more - people are more likely to read, obey and maintain documentation
which is terse and to the point.</font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">I want to rationalize thread-locals
out of the design and into real objects with a stated purpose and strategy.
If you can start to do that with the design, I would appreciate it.</font>
<br><font size=2 color=#000080 face="Verdana"> </font>
<br><font size=2 color=#000080 face="Verdana">Lastly, please rename ConcurrentMDXTest
to ConcurrentMdxTest; our coding standards call for acronyms to be InitCapped
not CAPITALIZED.</font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">Julian</font>
<br><font size=3> </font>
<br><font size=2 color=#000080 face="Verdana">PS Sorry I didn't reply earlier.
This reply somehow ended up in my Drafts folder and I forgot to send it.</font>
<br>
<br>
<hr><font size=2 face="Tahoma"><b>From:</b> mondrian-bounces@pentaho.org
[mailto:mondrian-bounces@pentaho.org] <b>On Behalf Of </b>Thiyagu Palanisamy<b><br>
Sent:</b> Thursday, May 31, 2007 5:42 AM<b><br>
To:</b> mondrian@pentaho.org<b><br>
Subject:</b> [Mondrian] Re: Optimizing Sync Blocks in Aggregation class</font><font size=3><br>
</font>
<br><font size=2 face="Verdana"><br>
We have done some more improvements to our Approach 2 : <b>Move the lock
from Aggregation to Segment wherever it is possible (CR 9366) </b></font><font size=3><br>
</font><font size=2 face="Verdana"><b><br>
New Changes:</b></font><font size=3> </font><font size=2 face="Verdana"><b><br>
1</b>. <b>Synchronized block removed from AggregationManager.getCellFromCache()</b></font><font size=3>
</font><font size=2 face="Verdana"><br>
The block was covering call to Aggregation.getCellValue() and synchronization
is handled with in Aggregation.getCellValue() method itself.</font><font size=3>
</font><font size=2 face="Verdana"><b><br>
2. Synchronized block removed from Aggregation.getCellValue()</b></font><font size=3>
</font><font size=2 face="Verdana"><br>
This method was earlier synchronized to ensure that access to <b><i>segmentRefs</i></b>is
thread safe. Now we have made segmentRefs as CopyOnWriteArrayList instance
instead of ArrayList. This frees us up from handling the Synchronized access
to this list. We have selected CopyOnWriteArrayList because <quote java
doc><i> </i>it is<i> more</i> efficient than alternatives when traversal
operations vastly outnumber mutations</quote java doc>, which is
the case in our scenario, there will be lot of get cellvalue calls but
only few times we will be modifying segments in the list.</font><font size=3>
</font><font size=2 face="Verdana"><b><br>
3. Made Segment.getCellValue() Synchronized</b></font><font size=3> </font><font size=2 face="Verdana"><b><br>
</b>As we have removed synchronization from
Aggregation we are synchronizing at Segment level.</font><font size=3>
</font>
<p><font size=2 face="Verdana">We have uploaded the latest change to </font><a href="http://forums.pentaho.org/showthread.php?t=54274"><font size=2 color=blue face="Verdana"><u>http://forums.pentaho.org/showthread.php?t=54274</u></font></a><font size=3>
</font>
<p><font size=2 face="Verdana">We also ran Jprofiler against all the 3
versions(Current implementation, Approach1, Approach2) and the reports
are attached to the same post. We could see that lock contention is drastically
reduced for data load path (Aggregation object).</font><font size=3> </font>
<p><font size=2 face="Verdana">Thanks,</font><font size=3> </font>
<p><font size=2 face="Verdana">Thiyagu & Tushar</font><font size=3>
<br>
<br>
<br>
</font>
<table width=100%>
<tr valign=top>
<td width=48%><font size=1 face="sans-serif"><b>Thiyagu Palanisamy/India/ThoughtWorks</b>
</font>
<p><font size=1 face="sans-serif">30/05/2007 20:29</font><font size=3>
</font>
<td width=51%>
<br>
<table width=100%>
<tr valign=top>
<td width=15%>
<div align=right><font size=1 face="sans-serif">To</font></div>
<td width=84%><font size=1 face="sans-serif">Mondrian</font><font size=3>
</font>
<tr valign=top>
<td>
<div align=right><font size=1 face="sans-serif">cc</font></div>
<td>
<tr valign=top>
<td>
<div align=right><font size=1 face="sans-serif">Subject</font></div>
<td><font size=1 face="sans-serif">Optimizing Sync Blocks in Aggregation
class</font></table>
<br>
<br>
<table width=100%>
<tr valign=top>
<td width=50%>
<td width=50%></table>
<br></table>
<br><font size=3><br>
</font><font size=2 face="Verdana"><br>
Hi,</font><font size=3> </font><font size=2 face="Verdana"><br>
</font><font size=3> </font><font size=2 face="Verdana"><br>
We are proposing optimization of synchronization blocks involved in loading
of data from DB.</font><font size=3> </font><font size=2 face="Verdana"><br>
</font><font size=3> </font><font size=2 face="Verdana"><br>
Current system is synchronized on Aggregation object while their segments
are loaded from DB. We did analysis using JProfiler to identify the maximum
lock contentions.</font><font size=3> </font><font size=2 face="Verdana"><br>
</font><font size=3> </font><font size=2 face="Verdana"><b><br>
Issues with the current implementation</b>:</font><font size=3> </font><font size=2 face="Verdana"><br>
1. Because of the lock at Aggregation, requests are forced to wait even
though they are not needed. Eg., If any previous request is loading a new
segment to Aggregation all the requests that need the same aggregation
will be blocked regardless of segment data availability. <br>
2. Aggregation is locked for entire period of creating segment, firing
sql, loading result set back on to segments. So aggregation is locked when
DB is processing data.</font><font size=3> </font><font size=2 face="Verdana"><br>
</font><font size=3> </font><font size=2 face="Verdana"><br>
We got 2 different working solutions to address this issue</font><font size=3>
</font><font size=2 face="Verdana"><br>
</font><font size=3> </font><font size=2 face="Verdana"><b><br>
Approach 1: Keep locking on Aggregation itself, but optimization the duration
lock (CR 9325)</b></font><font size=3> </font><font size=2 face="Verdana"><br>
Changes made for this solution:</font><font size=3> </font><font size=2 face="Verdana"><b><br>
1. Syncronized block removed from AggregationManager.load()</b></font><font size=3>
</font><font size=2 face="Verdana"><br>
Because this blocks covers call to Aggregation.optimizePredicates(),
Aggregation.load() but both these methods are already Synchronized. <b><br>
2. Made Aggregation.optimizePredicates() </b></font><font size=2 color=#c20000 face="Verdana"><b>non</b></font><font size=2 face="Verdana"><b>
synchronized</b><br>
Uses <b>maxConstraints,star fields</b> of aggregation instance
but they are read only and set <b>only </b>in constructor. <br>
Parameters to this method</font><font size=3> </font><font size=2 face="Verdana"><br>
columns : created at CellRequest, it is read
only access in the method. This is related Aggregation object but columns
list will remain same for the life time of the Aggregation class.</font><font size=3>
</font><font size=2 face="Verdana"><br>
predicates : created at FBCR which is unique
to a single request, it is also read only and has no association to Aggregation
object. <b><br>
3. Made Aggregation.load() </b></font><font size=2 color=#c20000 face="Verdana"><b>non</b></font><font size=2 face="Verdana"><b>
synchronized</b><br>
Uses constrainedColumnsBitKey,segmentRefs fields of aggregation
instance.</font><font size=3> </font><font size=2 face="Verdana"><br>
constrainedColumnsBitKey : This is read
only and set in constructor</font><font size=3> </font><font size=2 face="Verdana"><br>
segmentRefs : We are still synchronizing
on aggregation when accessing this</font><font size=3> </font><font size=2 face="Verdana"><br>
Parameters to this method</font><font size=3> </font><font size=2 face="Verdana"><br>
columns : created at CellRequest, this
is set to columns instance variable. columns list will remain same for
the life time of the Aggregation class so we have made change to set only
at the first time load is called on aggregation. This will happen only
during creation of Aggregation instance and it will be threadlocal at that
point. <br>
predicates : has no association to Aggregation
object</font><font size=3> </font><font size=2 face="Verdana"><br>
pinnedSegments : has no association
to Aggregation object</font><font size=3> </font><font size=2 face="Verdana"><br>
Measures: has no association to Aggregation
object<b><br>
4. Changes in Aggregation.load()</b> <br>
New Segments are added to segmentRefs only after they are loaded
with the data. This will ensure that none of the Segments are exposed to
other threads when they are loading. </font>
<p><font size=2 face="Verdana"><b>Advantages with this approach</b><br>
1. Aggregation object is locked only during the time of adding
loaded Segments to segmentRefs.<br>
2. This approach requires only minor modifications to the existing
locking strategy</font><font size=3> </font>
<p><font size=2 face="Verdana"><b>Disadvantage with this approach</b><br>
If more than one request comes at the same time for a same new
segment of an existing aggregation, both the requests will fire query and
load the segment data. This issue is already exists in the current version
too, but window in which this can happen is small, this will happen only
until one of the request reaches the AggregationManager.load(). With the
new change this will happen until segment is set to AggregationManager.segmentRefs.</font><font size=3>
</font>
<p><font size=2 face="Verdana"><b>Approach 2: Move the lock from Aggregation
to Segment wherever it is possible (CR 9366) <br>
1. Synchronized block removed from AggregationManager.load() </b>:
reason same as approach 1<b><br>
2. Made Aggregation.optimizePredicates() </b></font><font size=2 color=#c20000 face="Verdana"><b>non</b></font><font size=2 face="Verdana"><b>
synchronized </b>: reason same as Approach 1<b><br>
3. Made Aggregation.load() </b></font><font size=2 color=#c20000 face="Verdana"><b>non</b></font><font size=2 face="Verdana"><b>
synchronized </b>: reason same as Approach 1<b><br>
4. Changes in Aggregation.load() <br>
</b>Aggregation object is altered only when New Segments
are added to segmentRefs, so we have Sync block covering add to segmentRefs
<b><br>
5. Made Aggregation.getCellValue() </b></font><font size=2 color=#c20000 face="Verdana"><b>non</b></font><font size=2 face="Verdana"><b>
synchronized <br>
</b>Only access to segmentRefs needs to be Synchronized,
introduced sync block to cover access to segmentRefs and when segment is
not ready and segment would contain the requested value(which means another
thread is loading the segment) current request will be blocked until segment
is loaded. And return value once segment is ready. If segment fails to
load it will return null similar to when segment is not found. </font>
<p><font size=2 face="Verdana"><b>Advantage with this approach</b><br>
1.Aggregation object is locked only during the time of adding Segments
to segmentRefs. <br>
2. Concurrent requests will be blocked only if both the requests
are for same segment which is loading. This is a significant improvement
from Approach 1, because this reduces number of blocking requests. </font>
<p><font size=2 face="Verdana"><b>Disadvantage with this approach</b><br>
1.Locking strategy has been changed to part on Aggregation and
part on Segment. This also means that we need more testing.<br>
2.We are using segment state for synchronization, this piece of
code already exists but not used currently.</font><font size=3> </font>
<p><font size=2 face="Verdana">We have attached both our implementations
to the forum post: http://forums.pentaho.org/showthread.php?t=54274<br>
ConcurrentMDXTest class is to detected lock issues, this test is skewed
towards simulating concurrent access of same aggregation, this is not valid
for performance comparison of different implementations. <br>
<br>
Please give your inputs on which approach we should move forward and issues
or improvements we can make on to that approach. Also it would be of great
help if you can give us some more scenarios for concurrent testing.</font><font size=3>
</font>
<p><font size=2 face="Verdana">Thanks,<br>
Thiyagu, Tushar</font><font size=3> </font><tt><font size=2>_______________________________________________<br>
Mondrian mailing list<br>
Mondrian@pentaho.org<br>
http://lists.pentaho.org/mailman/listinfo/mondrian<br>
</font></tt>
<p>