[
https://issues.apache.org/jira/browse/CALCITE-3085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16850140#comment-16850140
]
Julian Hyde commented on CALCITE-3085:
--------------------------------------
Has this issue shown up empirically?
The stack is implemented as a {{ArrayDeque}}, so push and pop should be very
few machine instructions.
{{RelShuttleImpl}} is not the only possible implementation of {{RelShuttle}}.
It is just a good general-purpose implementation. If it doesn't meet your
needs, you can create your own.
No sub-classes in Calcite use the {{stack}} field, but it's there, and it is
potentially useful, if I am looking for say a {{Filter}} within a {{Project}}
within a {{Filter}}.
Consider {{AbstractList}} class in the JDK. It contains a {{modCount}} field is
useful for most implementations (prevents concurrent modifications) but is
unnecessary overhead for sub-classes that are immutable. To avoid that
overhead, I created {{AbstractImmutableList}}, but I wouldn't suggest removing
{{modCount}} from {{AbstractList}}.
> Unused stack field in RelShuttleImpl
> ------------------------------------
>
> Key: CALCITE-3085
> URL: https://issues.apache.org/jira/browse/CALCITE-3085
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Reporter: Laurent Goujon
> Assignee: Laurent Goujon
> Priority: Minor
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> {{RelShuttleImpl}} class has a protected {{stack}} field which is being
> populated when going over children, but those content is actually never used.
> In Calcite code base, no subclasses are actually using the content of the
> stack ({{CorelMapBuilder}} is populating the stack but does not read the
> content back either).
> Searching code on github didn't show any usage of it either (but this is not
> foolprof).
> As maintaining this stack has a non-negligible impact on memory/performance,
> I would suggest to remove the field.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
