[Mondrian] Some optimizations based on profiler results

Ajit Vasudeo Joglekar ajogleka at thoughtworks.com
Tue Oct 23 10:33:07 EDT 2007


I have attached a patch for the 1) getCalculatedMembers() and 3) 
setContext(Member member) changes. 

getCalculatedMembers does a role based caching of calculated members. 
Currently the cache only works on role instance equality. If a consumer 
creates exactly similar role (with same access rights) as that of a 
already cached one when the calculated members are fetched it won't use 
the cached instance. Adding a role name attribute could be useful if we 
can ensure that all roles with same role name will always have exactly 
same access rights. This will allow us to cache against role name. Any 
ideas how this can be improved? 

For suggested change 2) equals with assert, I feel that equals should 
never throw an exception. It should do a null check. If assertion is 
required it should be somewhere up in the hierarchy. Since there was 
feedback against this change, it is not there in the patch

Requesting further comments, suggestions

Thanks,
-Ajit





"Julian Hyde" <julianhyde at speakeasy.net> 
Sent by: mondrian-bounces at pentaho.org
10/18/2007 10:37 PM
Please respond to
Mondrian developer mailing list <mondrian at pentaho.org>


To
"'Mondrian developer mailing list'" <mondrian at pentaho.org>
cc

Subject
RE: [Mondrian] Some optimizations based on profiler results






See my comments inline.
 
Lastly: there is a developer's guide, which contains coding guidelines 
among other things. Enough said. 
http://mondrian.pentaho.org/documentation/developers_guide.php
 
Julian

From: mondrian-bounces at pentaho.org [mailto:mondrian-bounces at pentaho.org] 
On Behalf Of Ajit Vasudeo Joglekar
Sent: Thursday, October 18, 2007 5:36 AM
To: mondrian at pentaho.org
Subject: [Mondrian] Some optimizations based on profiler results


We did some profiling on mondrian code base. We have a large catalog with 
hundreds of dimensions and measures. Based on the hotspots identified by 
the profiler here are some optimizations that make considerable difference 
in performance in the evaluation of a single mdx. 

We would like to introduce these changes. Inviting comments, suggestions 

Thanks, 
-Ajit, Ashwin 

=================== 

1) In RolapCubeSchemaReader we have a getCalculatedMembers method that 
creates a new list of calculated members based on the role configured in 
the Mondrian schema. 

In the scenario where we have a huge number of calculated members and no 
Role configured this operation becomes quite expensive on multiple 
invocations. 

We can fix this by returning the original list of calculated members if 
the access is the default access 

 public List<Member> getCalculatedMembers() { 
            List<Member> list = new ArrayList<Member>(); 
                       for (Formula formula : calculatedMembers) { 
                                     Member member = 
formula.getMdxMember(); 
                                     if (getRole().canAccess(member)) { 
                                         list.add(member); 
                                     } 
                                 } 
            return list; 
        } 
 
        becomes 
 
         public List<Member> getCalculatedMembers() { 
                 if(Access.ALL == getRole().getAccess()){ 
                         return calculatedMembers; // indicative - need to 
convert [] to list. one option is We can maintain ArrayList and return [] 
where ever necessary 
                 } 
                  
                 List<Member> list = new ArrayList<Member>(); 
                     for (Formula formula : calculatedMembers) { 
                         Member member = formula.getMdxMember(); 
                         if (getRole().canAccess(member)) { 
                             list.add(member); 
                         } 
                     } 
            return list; 
        }  
 
I guess you have a lot of calculated members? Your code is low on 
specifics - in particular there's not actually a Role.getAccess() method - 
but I can see that computing the set of accessible calc members once per 
role would be useful.
 
2) In the RolapMember equals method : 

        private boolean equals(RolapMember that) { 
                assert that != null; 
 
                return this.getUniqueName().equals(that.getUniqueName()); 
    } 
 
    Now if the assert fails it will throw an exception which means equals 
will fail with a non boolean value which should not happen and assert is 
an expensive operation 
    when compared to a null check. 
 
    it can be changed to: 
 
        private boolean equals(RolapMember that) { 
                if(that == null){ 
                        return false; 
                } 
 
                return this.getUniqueName().equals(that.getUniqueName()); 
    } 
 
This is a terrible idea. If you don't like the overhead of asserts, turn 
them off: 'java -da -dsa'.
 
3) In RolapEvaluator : 
 public final Member setContext(Member member) { 
        final RolapMember m = (RolapMember) member; 
        final int ordinal = m.getDimension().getOrdinal(root.cube); 
        final Member previous = currentMembers[ordinal]; 
 
        if (previous.isCalculated()) { 
            removeCalcMember(previous); 
        } 
        currentMembers[ordinal] = m; 
        if (m.isCalculated()) { 
            addCalcMember(m); 
        } 
        return previous; 
    } 
 
    now we can optimize this by adding an equals to verify that m and 
previous are not the same. 
 
     public final Member setContext(Member member) { 
            final RolapMember m = (RolapMember) member; 
            final int ordinal = m.getDimension().getOrdinal(root.cube); 
            final Member previous = currentMembers[ordinal]; 
            if (m.equals(previous)) 
                return previous; 
            if (previous.isCalculated()) { 
                removeCalcMember(previous); 
            } 
            currentMembers[ordinal] = m; 
            if (m.isCalculated()) { 
                addCalcMember(m); 
            } 
            return previous; 
    } 
 
    this reduces some operations in case of massive number of invocations 
to this method.  
 
If the profiler says this ia major save, go ahead and add it.
 
Julian_______________________________________________
Mondrian mailing list
Mondrian at pentaho.org
http://lists.pentaho.org/mailman/listinfo/mondrian

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.pentaho.org/pipermail/mondrian/attachments/20071023/48dfb2df/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: changelist10051.tar.gz
Type: application/octet-stream
Size: 31705 bytes
Desc: not available
Url : http://lists.pentaho.org/pipermail/mondrian/attachments/20071023/48dfb2df/attachment.obj 


More information about the Mondrian mailing list