[CALCITE-5665] Reducing ineffective matches for MaterializedViewRules #3169
Conversation
| @@ -231,21 +260,11 @@ protected void perform(RelOptRuleCall call, @Nullable Project topProject, RelNod | |||
| continue; | |||
| } | |||
|
|
|||
| // 3.1. View checks before proceeding | |||
There was a problem hiding this comment.
Code comments are no longer complete:
3.1 was deleted, but 3.2 was reserved.
There was a problem hiding this comment.
Code comments are no longer complete: 3.1 was deleted, but 3.2 was reserved.
fixed
| */ | ||
| private final LoadingCache<RelMetadataQuery, MaterializationMetadata> | ||
| materializationMetadataCache = | ||
| CacheBuilder.newBuilder() |
There was a problem hiding this comment.
Should we set some parameters?
For example: initialCapacity, maximumSize, refreshAfterWrite, ...
There was a problem hiding this comment.
Should we set some parameters?
For example: initialCapacity, maximumSize, refreshAfterWrite, ...
Through Gc Roots analysis, I found that weakKeys cannot take effect. Even if I delete MaterializationMetadata#RelMetadataQuery, there is still a strong reference pointing to RelMetadataQuery in MaterializationMetadata#RelOptMaterialization#tableRel, so we should really add two parameters, maximumSize and expireAfterAccess, to materializationMetadataCache. Thanks for your suggestion and review!
asdfgh19
changed the title
| CacheBuilder.newBuilder() | ||
| .expireAfterAccess(5, TimeUnit.MINUTES) | ||
| .maximumSize(1_024) | ||
| .build(CacheLoader.from(MaterializationMetadata::new)); |
| if (notValidMaterializations.contains(materialization)) { | ||
| return false; | ||
| } else if (viewTableRefsMap.containsKey(materialization)) { | ||
| return true; |
There was a problem hiding this comment.
Should we use viewTableRefsMap.containsKey(materialization) && viewPredicateListMap.containsKey(materialization) to be more accurate?
There was a problem hiding this comment.
Should we use
viewTableRefsMap.containsKey(materialization) && viewPredicateListMap.containsKey(materialization)to be more accurate?
Indeed we should.Thanks for your suggestion and review!
|
Kudos, SonarCloud Quality Gate passed! |
julianhyde
force-pushed
the
main
branch
2 times, most recently
from
8a5cf83 to
cf7f71b
Compare
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions. |
jira: CALCITE-5665
For example, in the test MaterializedViewRelOptRulesTest#testJoinAggregateMaterializationNoAggregateFuncs9, MaterializedViewRule#perform was triggered 489 times in the original implementation, but in this pr, it was only triggered 23 times.
I did a simple test on my personal computer and added a timer to the MaterializedViewRelOptRulesTest#TESTER#optimize method. Here are the milliseconds this method executes before and after optimization.
before: 237 237 229 250 241 227 258 239 253 267 256 244 228 228 241 245 245 238 273 243
after: 199 202 191 190 243 206 207 204 213 239 239 205 192 200 215 196 195 200 193 217
before avg: 243.95 ms
after avg: 207.3 ms
before: 225 239 244 236 240 225 281 239 240 236 238 236 228 239 241 238 237 265 256 233
after: 204 201 187 196 192 213 197 186 200 194 189 200 207 199 210 213 188 214 201 206
before avg: 240.8 ms
after avg: 199.85 ms