[
https://issues.apache.org/jira/browse/CALCITE-5277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17602193#comment-17602193
]
Thomas Rebele edited comment on CALCITE-5277 at 9/9/22 10:07 AM:
-----------------------------------------------------------------
The map itself is not part of the cache key, but it influences the order of the
statements of the generated code. The generated code is then used as the cache
key.
The method EnumerableRelImplementor.stash(T, Class<? super T>) modifies the
map, so it cannot be an ImmutableMap.
was (Author: thomas.rebele):
The map itself is not part of the cache key, but it influences the order of the
statements of the generated code. The generated code is then used as cache key.
The method EnumerableRelImplementor.stash(T, Class<? super T>) modifies the
map, so it cannot be an ImmutableMap.
> Make EnumerableRelImplementor stashedParameters order deterministic to
> increase BINDABLE_CACHE hit rate
> -------------------------------------------------------------------------------------------------------
>
> Key: CALCITE-5277
> URL: https://issues.apache.org/jira/browse/CALCITE-5277
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Ruben Q L
> Assignee: Ruben Q L
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.32.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Currently, EnumerableRelImplementor stashedParameters is a map with no
> deterministic order when its values are traversed.
> Because of this, it is possible to have two EnumerableRel which are
> effectively equal, but which generate a different dynamic code, just because
> the statements in which stashedParameters are defined are the same, but in
> different order.
> {code}
> final Collection<Statement> stashed =
> Collections2.transform(stashedParameters.values(),
> input -> Expressions.declare(Modifier.FINAL, input,
> Expressions.convert_(
> Expressions.call(DataContext.ROOT,
> BuiltInMethod.DATA_CONTEXT_GET.method,
> Expressions.constant(input.name)),
> input.type)));
> {code}
> This has a disadvantage: in this situation, EnumerableInterpretable's
> BINDABLE_CACHE will not have a cache hit, so the Bindable will need to be
> unnecessarily recreated in EnumerableInterpretable#getBindable.
> If we guarantee a deterministic order in EnumerableRelImplementor
> stashedParameters (e.g. insertion order), we would have a cache hit in these
> situations (and we could skip the bindable re-creation).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
