[
https://issues.apache.org/jira/browse/CALCITE-5277?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ruben Q L updated CALCITE-5277:
-------------------------------
Description:
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).
was:
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
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).
> 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: 10m
> 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)
