[Mondrian] RE: Eigenbase perforce change 13490 for review

Julian Hyde jhyde at pentaho.com
Fri Mar 19 17:19:59 EDT 2010


Kurtis,

I ran some performance numbers of the code immediately before and after your
original change (13367) and this change to back out that change (13490). I
ran the test four times for each revision:

Revision Run 1   Run 2   Run 3   Run 4   Mean    Delta
======== ======= ======= ======= ======= ======= =====
13366    592.129 588.708 581.555 571.316 583.4
13367    643.464 619.785 631.077 671.164 641.4   +9.9%
13489    565.039 560.823 578.871 595.692 575.1   -10.3%
13490    606.785 610.929 581.400 605.406 601.1   +4%

Your original change slowed down the code by 9.9%. Between 13367 and 13489 I
did some tuning that improved performance by 10.3%. I said that I wanted to
regain the 9.9% lost by your original change, which would have taken the
numbers to ~517 seconds. But 13490 actually made the code 4% slower.

So I am going to ask you again to fully back out change 13366.

I know that you are solving a legitimate problem. Schemas with many
dimensions & levels exist in the real world and we have not spent a lot of
effort tuning for them.

I would welcome some performance tests for large schemas so that I can see
how bad the problem is, and maybe help tune for them. A version of the
Foodmart Sales cube that has N copies of each dimension (e.g. Gender,
Gender1, Gender2, ..., Marital Status, Marital Status1, ...) would be
useful, because we could run all of the standard queries against it and see
the % slowdown.

I will accept a fix to the problem if you can find a way to do it that does
not negatively affect mondrian's performance for schemas with small
dimensions. I'm pretty sure that a solution exists.

Julian

> -----Original Message-----
> From: Matt Campbell [mailto:Matthew.Campbell at thomson.com] 
> Sent: Wednesday, March 17, 2010 8:36 AM
> To: Aaron Phillips; Ezequiel Cuellar; Julian Hyde; John V. 
> Sichi; Will Gorman
> Subject: Eigenbase perforce change 13490 for review
> 
> http://p4web.eigenbase.org/@md=d&c=6PU@//13490?ac=10
> 
> Change 13490 by mkambol at guest_AA-5501 on 2010/03/17 08:35:51
> 
> 	MONDRIAN: taking out nonAllMembers Map.  Moving all the 
> logic into the getNonAllMembers method
> 
> Affected files ...
> 
> ... 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
> ator.java#6 edit
> ... 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
atorRoot.java#3 edit
> 
> Differences ...
> 
> ==== 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
> ator.java#6 (ktext) ====
> 
> 2c2
> < // $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
> ator.java#5 $
> ---
> > // $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
> ator.java#6 $
> 45c45
> <  * @version $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
> ator.java#5 $
> ---
> >  * @version $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
> ator.java#6 $
> 56d55
> <     private TreeMap<Integer, RolapMember> nonAllMemberMap;
> 79a79
> >     private Member[] nonAllMembers;
> 114,115d113
> <         nonAllMemberMap =
> <             (TreeMap<Integer, RolapMember>) 
> parent.nonAllMemberMap.clone();
> 144,145d141
> <         nonAllMemberMap =
> <             (TreeMap<Integer, RolapMember>) 
> root.nonAllDefaultMembers.clone();
> 244,245c240,249
> <         final Collection<RolapMember> members = 
> nonAllMemberMap.values();
> <         return members.toArray(new Member[members.size()]);
> ---
> >         if (nonAllMembers == null) {
> >             final List<RolapMember> members = new 
> ArrayList<RolapMember>();
> >             for (RolapMember rolapMember : currentMembers) {
> >                 if(!rolapMember.isAll()){
> >                     members.add(rolapMember);
> >                 }
> >             }
> >             nonAllMembers = members.toArray(new 
> Member[members.size()]);
> >         }
> >         return nonAllMembers;
> 408,412d411
> <         if (m.isAll()) {
> <             nonAllMemberMap.remove(ordinal);
> <         } else {
> <             nonAllMemberMap.put(ordinal, m);
> <         }
> 415a415
> >         nonAllMembers = null;
> 630c630
> <         for (RolapMember member : nonAllMemberMap.values()) {
> ---
> >         for (Member member : getNonAllMembers()) {
> 
> ==== 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
atorRoot.java#3 (ktext) ====
> 
> 2c2
> < // $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
atorRoot.java#2 $
> ---
> > // $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
atorRoot.java#3 $
> 28c28
> <  * @version $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
atorRoot.java#2 $
> ---
> >  * @version $Id: 
> //open/mondrian-release/3.2/src/main/mondrian/rolap/RolapEvalu
atorRoot.java#3 $
> 48d47
> <     final TreeMap<Integer, RolapMember> nonAllDefaultMembers;
> 68d66
> <         nonAllDefaultMembers = new TreeMap<Integer, RolapMember>();
> 94,97d91
> <             if (!defaultMember.isAll()) {
> <                 nonAllDefaultMembers.put(
> <                     hierarchy.getOrdinalInCube(), defaultMember);
> <             }
> 




More information about the Mondrian mailing list