[MONDRIAN-2426] Performance with subtotals/totals is significantly wo… #1106
Conversation
|
Please see this comment for background on this function: |
Build Completed🔥 This pull request has some issues. It would be preferable to fix them in order for it to be just perfect. See below for more details. Some links are also available below for further assistance in addressing those issues. Build Commandsmvn -B -f 'pom.xml' -pl 'mondrian' -P '!assemblies' -DrunITs=true -Dsurefire.runOrder=alphabetical -Dfailsafe.runOrder=alphabetical -Daudit -amd clean installCleanup Commandsmvn -B -f 'pom.xml' -pl 'mondrian' -P '!assemblies' -amd build-helper:remove-project-artifactChanged files mondrian/src/it/java/mondrian/olap/fun/CachedExistsTest.java
mondrian/src/main/java/mondrian/olap/fun/BuiltinFunTable.java
mondrian/src/main/java/mondrian/olap/fun/extra/CachedExistsFunDef.javaIntegration Test CoverageThese statistics help you identify how your changes have affected the coverage of the following files. If a file is not in this list, then its coverage was not affected by your changes. To get some help interpreting these metrics, please refer to Jacoco's documentation.
|
|
@mkambol @lucboudreau @kurtwalker Could someone help review this custom function I added for improving Analyzer performance? Thanks |
|
Here are the Analyzer changes that use this new function: |
|
I'm not sure the keys generated have enough information to be unique. A virtual cube using the same level name would require different subtotals but hit the same cache key, no? |
|
@lucboudreau I don't think whether the cube is virtual or not would matter. The generated keys (i.e. tuples of member unique names) are used to look up items from the input non-dynamic set. Whether this is used in a virtual cube context or not should not matter. There is no nonempty logic in the function. |
|
@bennychow I think it may cause issues if a report is ran on the base cube first, then on a virtual cube with the same D/H/L names afterwards. A test would prove me wrong. |
|
@lucboudreau This cache is query specific. That’s why I put it into the Query object. Two different MDX statements would not share the same cache. BTW, how long does the Query object live around for? It is freed for garbage collection right after the query is done? |
|
@bennychow We don't force garbage collection. It'll get picked up whenever the JVM needs more space. |
|
My only last question is: should this be in the core function def for Exists? Is there a reason we wouldn't want this as the default behavior? |
|
CachedExists assumes the first set is non-dynamic and requires the caller to name this set and pass that same name in on subsequent calls to benefit from the cache. There’s also overhead for building this cache which may be unnecessary if Exists is only called once. I think this CachedExists is very specific to Analyzer and hard to generalize to Exists. |
|
I can't remember if we have more documentation to update. I'm on my phone and can't check, and I'll take a look tomorrow, but there may be a MDX document to keep updated in the git project somewhere. And yeah. The first set turning static is a blocker for the general case Exists. |
|
Hey @bennychow sorry about the delay. The MDX functions need to be documented here too. https://github.com/pentaho/mondrian/blob/master/mondrian/src/site/resources/doc/mdx.html |
…rse than without. Add function to be used by Analyzer.
Build Completed🔥 This pull request has some issues. It would be preferable to fix them in order for it to be just perfect. See below for more details. Some links are also available below for further assistance in addressing those issues. Build Commandsmvn -B -f 'pom.xml' -pl 'mondrian' -P '!assemblies' -DrunITs=true -Dsurefire.runOrder=alphabetical -Dfailsafe.runOrder=alphabetical -Daudit -amd clean installCleanup Commandsmvn -B -f 'pom.xml' -pl 'mondrian' -P '!assemblies' -amd build-helper:remove-project-artifactChanged files mondrian/src/it/java/mondrian/olap/fun/CachedExistsTest.java
mondrian/src/main/java/mondrian/olap/fun/BuiltinFunTable.java
mondrian/src/main/java/mondrian/olap/fun/extra/CachedExistsFunDef.java
mondrian/src/site/resources/doc/mdx.htmlIntegration Test CoverageThese statistics help you identify how your changes have affected the coverage of the following files. If a file is not in this list, then its coverage was not affected by your changes. To get some help interpreting these metrics, please refer to Jacoco's documentation.
|
|
@lucboudreau Hi Luc, I added the description to the function list HTML. If this looks good, can you merge this PR? Thanks |
…rse than without. Add function to be used by Analyzer.