[ CASSANDRA-21088][trunk] Minor perf optimizations around memtable put logic #4538
Conversation
| return result; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
This method & transformLeaf is duplicating a lot of code. Do we have any metrics on performance improvements? otherwise the duplication could cause cause inconsistencies in code, maintenance issues and could create bugs.
There was a problem hiding this comment.
yes, I've attached test results to the ticket. Unfortunately the method reference trick which I mentioned in another comment does not work for logic in org.apache.cassandra.db.rows.BTreeRow#clone.
BTree functionality is stable and does not change frequently to introduce too much maintenance cost here. Also, the same pattern has been already used previously in this class for transformAndFilter logic.
jyothsnakonisa
left a comment
jyothsnakonisa
left a comment
There was a problem hiding this comment.
Thanks, looks good to me. However It would be nice if you can get it reviewed by someone who is more familiar with this code path.
| * If no modifications are made, the original is returned. | ||
| * NOTE: codewise *identical* to {@link #transform(Object[], Function)} | ||
| */ | ||
| public static <I, I2, O> Object[] transform(Object[] tree, BiFunction<? super I, ? super I2, ? extends O> function, I2 param) |
There was a problem hiding this comment.
does Cloner parameter need to be a generic ?
| public static <I, I2, O> Object[] transform(Object[] tree, BiFunction<? super I, ? super I2, ? extends O> function, I2 param) | |
| public static <I, O> Object[] transform(Object[] tree, BiFunction<? super I, Cloner, ? extends O> function, Cloner cloner) |
There was a problem hiding this comment.
I think it would be better to keep the original generic style, reasons:
- (coupling) BTree is a generic utility logic, it is better to not create an unnecessary coupling to specific logic related to memory allocation
- (generality/abstraction) the method logic is generic and does not depend on the argument type (it is correct for any type), it can be used in other use cases later if needed
- (consistency) the same style with generics is ready used in BTree, for example: transformAndFilter uses exactly the same style.
Avoid BTree iterators allocation in ColumnsCollector.update reduce amout of virtual calls for Cell.dataSize avoid capturing lambda allocation in BTreeRow.clone do not recalculate minLocalDeletionTime when BTreeRow is cloned avoid CAS write if EncodingStats is not changed after merge Patch by Dmitry Konstantinov; reviewed by Jyothsna Konisa, Michael Semb Wever for CASSANDRA-21088
netudima
force-pushed
the
CASSANDRA-21088-trunk
branch
from
9b24d7d to
2ca9d8f
Compare
3 participants
Avoid BTree iterators allocation in ColumnsCollector.update
reduce amount of virtual calls for Cell.dataSize
avoid capturing lambda allocation in BTreeRow.clone
do not recalculate minLocalDeletionTime when BTreeRow is cloned
Patch by Dmitry Konstantinov; reviewed by TBD for CASSANDRA-21088