smiklosovic commented on code in PR #4785:
URL: https://github.com/apache/cassandra/pull/4785#discussion_r3187271681
##########
src/java/org/apache/cassandra/cql3/statements/SelectStatement.java:
##########
@@ -815,11 +814,21 @@ private ReadQuery getSliceCommands(QueryOptions options,
ClientState state, Colu
if (filter == null || filter.isEmpty(table.comparator))
return ReadQuery.empty(table);
- List<DecoratedKey> decoratedKeys = new ArrayList<>(keys.size());
- for (ByteBuffer key : keys)
+ List<DecoratedKey> decoratedKeys;
+ if (keys.size() == 1) // reduce allocations in collections for keys
{
+ ByteBuffer key = keys.get(0);
QueryProcessor.validateKey(key);
-
decoratedKeys.add(table.partitioner.decorateKey(ByteBufferUtil.clone(key)));
+ decoratedKeys =
Collections.singletonList(table.partitioner.decorateKey(key));
+ }
+ else
+ {
+ decoratedKeys = new ArrayList<>(keys.size());
+ for (ByteBuffer key : keys)
+ {
+ QueryProcessor.validateKey(key);
Review Comment:
What if some key is invalid? Then we would allocate
`ArrayList<>(keys.size())` unnecessarily because validateKey() throws and this
whole method fails.
What about
for (ByteBuffer key : keys)
QueryProcessor.validateKey(key);
decoratedKeys = new ArrayList<>(keys.size());
for (ByteBuffer key : keys)
decoratedKeys.add(table.partitioner.decorateKey(key));
This would be 2 iterator allocations and 1 decoratedKeys allocation when all
keys are valid versus current 1 decorated keys validation every time and 1
iterator allocation even when some key is invalid.
I think that your approach is better if there is bigger probability that all
keys are valid which I think is true.
Maybe some dynamic resizing of that decoratedKeys list would be possible
too? Like we would optimistically create e.g. half of keys.size() at first and
then we would inflate it more ... dunno. I think it might be kept as you did it
now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
