<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<HTML><HEAD>
<META http-equiv=Content-Type content="text/html; charset=us-ascii">
<META content="MSHTML 6.00.6000.16441" name=GENERATOR></HEAD>
<BODY>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>Thiyagu,</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>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></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>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></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>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></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>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></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>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></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>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></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>Lastly, please rename 
ConcurrentMDXTest to ConcurrentMdxTest; our coding standards call for acronyms 
to be InitCapped not CAPITALIZED.</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>Julian</FONT></SPAN></DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2></FONT></SPAN>&nbsp;</DIV>
<DIV dir=ltr align=left><SPAN class=486114010-01062007><FONT face=Verdana 
color=#000080 size=2>PS Sorry I didn't reply earlier. This reply somehow ended 
up in my Drafts folder and I forgot to send it.</FONT></SPAN></DIV><BR>
<BLOCKQUOTE dir=ltr 
style="PADDING-LEFT: 5px; MARGIN-LEFT: 5px; BORDER-LEFT: #000080 2px solid; MARGIN-RIGHT: 0px">
  <DIV class=OutlookMessageHeader lang=en-us dir=ltr align=left>
  <HR tabIndex=-1>
  <FONT face=Tahoma size=2><B>From:</B> mondrian-bounces@pentaho.org 
  [mailto:mondrian-bounces@pentaho.org] <B>On Behalf Of </B>Thiyagu 
  Palanisamy<BR><B>Sent:</B> Thursday, May 31, 2007 5:42 AM<BR><B>To:</B> 
  mondrian@pentaho.org<BR><B>Subject:</B> [Mondrian] Re: Optimizing Sync Blocks 
  in Aggregation class<BR></FONT><BR></DIV>
  <DIV></DIV><BR><FONT face=Verdana size=2>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><BR><BR><FONT face=Verdana size=2><B>New 
  Changes:</B></FONT> <BR><FONT face=Verdana size=2><B>1</B>. <B>Synchronized 
  block removed from AggregationManager.getCellFromCache()</B></FONT> <BR><FONT 
  face=Verdana size=2>The block was covering call to Aggregation.getCellValue() 
  and synchronization is handled with in Aggregation.getCellValue() method 
  itself.</FONT> <BR><FONT face=Verdana size=2><B>2. Synchronized block removed 
  from Aggregation.getCellValue()</B></FONT> <BR><FONT face=Verdana size=2>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 &lt;quote java doc&gt;</FONT><FONT face=Verdana 
  size=2><I> </I></FONT><FONT face=Verdana size=2>it is<I> more</I> efficient 
  than alternatives when traversal operations vastly outnumber 
  mutations</FONT><FONT face=Verdana size=2>&lt;/</FONT><FONT face=Verdana 
  size=2>quote java doc&gt;, 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> <BR><FONT face=Verdana size=2><B>3. Made 
  Segment.getCellValue() Synchronized</B></FONT> <BR><FONT face=Verdana 
  size=2><B>&nbsp; &nbsp; &nbsp; &nbsp; </B>As we have removed synchronization 
  from Aggregation we are synchronizing at Segment level.</FONT> <BR>
  <P><FONT face=Verdana size=2>We have uploaded the latest change to </FONT><A 
  href="http://forums.pentaho.org/showthread.php?t=54274"><FONT face=Verdana 
  color=blue 
  size=2><U>http://forums.pentaho.org/showthread.php?t=54274</U></FONT></A> 
  <P><FONT face=Verdana size=2>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> 
  <P>
  <P><FONT face=Verdana size=2>Thanks,</FONT> 
  <P><FONT face=Verdana size=2>Thiyagu &amp; Tushar</FONT> <BR><BR><BR><BR>
  <TABLE width="100%">
    <TBODY>
    <TR vAlign=top>
      <TD width="40%"><FONT face=sans-serif size=1><B>Thiyagu 
        Palanisamy/India/ThoughtWorks</B> </FONT>
        <P><FONT face=sans-serif size=1>30/05/2007 20:29</FONT> </P>
      <TD width="59%">
        <TABLE width="100%">
          <TBODY>
          <TR vAlign=top>
            <TD>
              <DIV align=right><FONT face=sans-serif size=1>To</FONT></DIV>
            <TD><FONT face=sans-serif size=1>Mondrian</FONT> 
          <TR vAlign=top>
            <TD>
              <DIV align=right><FONT face=sans-serif size=1>cc</FONT></DIV>
            <TD>
          <TR vAlign=top>
            <TD>
              <DIV align=right><FONT face=sans-serif size=1>Subject</FONT></DIV>
            <TD><FONT face=sans-serif size=1>Optimizing Sync Blocks in 
              Aggregation class</FONT></TD></TR></TBODY></TABLE><BR>
        <TABLE>
          <TBODY>
          <TR vAlign=top>
            <TD>
            <TD></TD></TR></TBODY></TABLE><BR></TD></TR></TBODY></TABLE><BR><BR><FONT 
  face=Verdana size=2>Hi,</FONT> <BR><FONT face=Verdana size=2>&nbsp;</FONT> 
  <BR><FONT face=Verdana size=2>We are proposing optimization of synchronization 
  blocks involved in loading of data from DB.</FONT> <BR><FONT face=Verdana 
  size=2>&nbsp;</FONT> <BR><FONT face=Verdana size=2>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> 
  <BR><FONT face=Verdana size=2>&nbsp;</FONT> <BR><FONT face=Verdana 
  size=2><B>Issues with the current implementation</B>:</FONT> <BR><FONT 
  face=Verdana size=2>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. 
  </FONT><BR><FONT face=Verdana size=2>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> 
  <BR><FONT face=Verdana size=2>&nbsp;</FONT> <BR><FONT face=Verdana size=2>We 
  got 2 different working solutions to address this issue</FONT> <BR><FONT 
  face=Verdana size=2>&nbsp;</FONT> <BR><FONT face=Verdana size=2><B>Approach 1: 
  Keep locking on Aggregation itself, but optimization the duration lock (CR 
  9325)</B></FONT> <BR><FONT face=Verdana size=2>Changes made for this 
  solution:</FONT> <BR><FONT face=Verdana size=2><B>1. Syncronized block removed 
  from AggregationManager.load()</B></FONT> <BR><FONT face=Verdana size=2>&nbsp; 
  &nbsp; Because this blocks covers call to Aggregation.optimizePredicates(), 
  Aggregation.load() but both these methods are already Synchronized. 
  </FONT><BR><FONT face=Verdana size=2><B>2. Made 
  Aggregation.optimizePredicates() </B></FONT><FONT face=Verdana color=#c20000 
  size=2><B>non</B></FONT><FONT face=Verdana size=2><B> 
  synchronized</B><BR>&nbsp; &nbsp;Uses <B>maxConstraints,star fields</B> of 
  aggregation instance but they are read only and set <B>only </B>in 
  constructor. <BR>&nbsp; &nbsp;Parameters to this method</FONT> <BR><FONT 
  face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;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> <BR><FONT face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; 
  &nbsp;predicates : created at FBCR which is unique to a single request, it is 
  also read only and has no association to Aggregation object. </FONT><BR><FONT 
  face=Verdana size=2><B>3. Made Aggregation.load() </B></FONT><FONT 
  face=Verdana color=#c20000 size=2><B>non</B></FONT><FONT face=Verdana 
  size=2><B> synchronized</B><BR>&nbsp; &nbsp;Uses 
  constrainedColumnsBitKey,segmentRefs fields of aggregation instance.</FONT> 
  <BR><FONT face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 
  constrainedColumnsBitKey : This is read only and set in constructor</FONT> 
  <BR><FONT face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; segmentRefs : 
  We are still synchronizing on aggregation when accessing this</FONT> <BR><FONT 
  face=Verdana size=2>&nbsp; &nbsp; &nbsp;Parameters to this method</FONT> 
  <BR><FONT face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 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. </FONT><BR><FONT face=Verdana size=2>&nbsp; &nbsp; &nbsp; 
  &nbsp; &nbsp; predicates : has no association to Aggregation object</FONT> 
  <BR><FONT face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; 
  pinnedSegments : has no association to Aggregation object</FONT> <BR><FONT 
  face=Verdana size=2>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; Measures: has no 
  association to Aggregation object<B><BR>4. Changes in Aggregation.load()</B> 
  &nbsp;<BR>&nbsp; &nbsp;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 face=Verdana size=2><B>Advantages with this approach</B><BR>&nbsp; 
  &nbsp;1. Aggregation object is locked only during the time of adding loaded 
  Segments to segmentRefs.<BR>&nbsp; &nbsp;2. This approach requires only minor 
  modifications to the existing locking strategy</FONT> 
  <P><FONT face=Verdana size=2><B>Disadvantage with this approach</B><BR>&nbsp; 
  &nbsp;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> 
  <P><FONT face=Verdana size=2><B>Approach 2: Move the lock from Aggregation to 
  Segment wherever it is possible (CR 9366) <BR>&nbsp; &nbsp;1. Synchronized 
  block removed from AggregationManager.load() </B>: reason same as approach 
  1<BR><B>&nbsp; &nbsp;2. Made Aggregation.optimizePredicates() </B></FONT><FONT 
  face=Verdana color=#c20000 size=2><B>non</B></FONT><FONT face=Verdana 
  size=2><B> synchronized </B>: reason same as Approach 1<B><BR>&nbsp; &nbsp;3. 
  Made Aggregation.load() </B></FONT><FONT face=Verdana color=#c20000 
  size=2><B>non</B></FONT><FONT face=Verdana size=2><B> synchronized </B>: 
  reason same as Approach 1<B><BR>&nbsp; &nbsp;4. Changes in Aggregation.load() 
  &nbsp;<BR>&nbsp; &nbsp; &nbsp; &nbsp;</B>Aggregation object is altered only 
  when New Segments are added to segmentRefs, so we have Sync block covering add 
  to segmentRefs &nbsp; &nbsp; <B><BR>&nbsp; &nbsp;5. Made 
  Aggregation.getCellValue() </B></FONT><FONT face=Verdana color=#c20000 
  size=2><B>non</B></FONT><FONT face=Verdana size=2><B> synchronized &nbsp; 
  <BR>&nbsp; &nbsp; &nbsp; &nbsp;</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 face=Verdana size=2><B>Advantage with this approach</B><BR>&nbsp; 
  &nbsp;1.Aggregation object is locked only during the time of adding Segments 
  to segmentRefs. <BR>&nbsp; &nbsp;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 face=Verdana size=2><B>Disadvantage with this approach</B><BR>&nbsp; 
  &nbsp;1.Locking strategy has been changed to part on Aggregation and part on 
  Segment. This also means that we need more testing.<BR>&nbsp; &nbsp;2.We are 
  using segment state for synchronization, this piece of code already exists but 
  not used currently.</FONT> 
  <P><FONT face=Verdana size=2>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> 
  <P><FONT face=Verdana size=2>Thanks,<BR>Thiyagu, Tushar</FONT> 
  <P><FONT face=Verdana color=#000080 
size=2></FONT></P></BLOCKQUOTE></BODY></HTML>