[
https://issues.apache.org/jira/browse/CASSANDRA-18248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17686562#comment-17686562
]
Josh McKenzie commented on CASSANDRA-18248:
-------------------------------------------
{quote}compiled a list of candidates
{quote}
Looking at the [discussion in the
PR|https://github.com/apache/cassandra/pull/2110#discussion_r1100241694], it
looks a lot less like he compiled a list of candidates and a lot more like he
enumerated examples of us using streams in the Cql package as evidence that
it's idiomatic, not considered hot path enough to warrant replacing them all,
and likely premature optimization.
I'm inclined to agree with him. If you have a profile that shows that these
implementations are problematic from a performance perspective, sound reasoning
as to the relative resource consumption of these operations vs. the totality of
queries to warrant both the loss of idiomatic readability of the code, the
diffs we'll need to review and rebase across to make the changes, and the risk
of introducing simple defects as we hand-roll things currently handled by the
libraries, I'm all ears. Thus far I'm not seeing that on the PR or this ticket,
and for a change like this I think the burden's on the proposer to justify it
right?
Right now this reads like "Things might be slower and I also don't personally
find this paradigm more readable than simple loops", which doesn't personally
sway me from the "if the idiomatic approach ain't broke, don't fix it" default
position.
> Avoid streams usage in potential hot paths
> ------------------------------------------
>
> Key: CASSANDRA-18248
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18248
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Berenguer Blasi
> Priority: Normal
> Labels: low-hanging-fruit
>
> During the review of CASSANDRA-18068 it was discussed whether preparing
> queries might be considered a hot path or not. Seen in isolation it isn't.
> But given multi-tenancy setups i.e. and coming from the experience of seeing
> how prepare is used by some customers as per some bug reports this might not
> be 100% true
> Let's discuss here if removing streams would make any sense. Most rewrites I
> expect wouldn't impact readability. [~adelapena] took a first stab and
> compiled a first list of candidates:
> *
> [o.a.c.cql3/CQL3Type.java#L840|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/CQL3Type.java#L840]
> *
> [o.a.c.cql3/CQL3Type.java#L876|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/CQL3Type.java#L876]
> *
> [o.a.c.cql3/Lists.java#L133|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/Lists.java#L133]
> *
> [o.a.c.cql3/Sets.java#L123|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/Sets.java#L123]
> *
> [o.a.c.cql3/MultiColumnRelation.java#L225|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/MultiColumnRelation.java#L225]
> *
> [o.a.c.cql3/QueryProcessor.java#L481|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L481]
> *
> [o.a.c.cql3/TokenRelation.java#L138|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/TokenRelation.java#L138]
> *
> [o.a.c.cql3/conditions/ColumnConditions.java#L75|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/conditions/ColumnConditions.java#L75]
> *
> [o.a.c.cql3/functions/AbstractFunction.java#L69|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/functions/AbstractFunction.java#L69]
> *
> [o.a.c.cql3/functions/FunctionResolver.java#L203|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/functions/FunctionResolver.java#L203]
> *
> [o.a.c.cql3/functions/UDAggregate.java#L97|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java#L97]
> *
> [o.a.c.cql3/restrictions/StatementRestrictions.java#L380|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/restrictions/StatementRestrictions.java#L380]
> *
> [o.a.c.cql3/selection/AbstractFunctionSelector.java#L72|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/AbstractFunctionSelector.java#L72]
> *
> [o.a.c.cql3/selection/MapSelector.java#L113|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/MapSelector.java#L113]
> *
> [o.a.c.cql3/selection/Selectable.java#L708|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L708]
> *
> [o.a.c.cql3/selection/Selectable.java#L793|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L793]
> *
> [o.a.c.cql3/selection/Selectable.java#L886|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L886]
> *
> [o.a.c.cql3/selection/Selectable.java#L954|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L954]
> *
> [o.a.c.cql3/selection/Selectable.java#L1024|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/selection/Selectable.java#L1024]
> *
> [o.a.c.cql3/statements/SelectStatement.java#L1278|https://github.com/apache/cassandra/blob/cassandra-4.1.0/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java#L1278]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
