[CASSANDRA-21524][trunk] Row merging logic optimizations#4941
Open
netudima wants to merge 9 commits into
Open
Conversation
fix an import order
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
there is no need to add empty interators to columnDataIterators if a row is null, also 2 loops over rows can be combined into one
MergeIterator logic is generic and can be used in many different use cases, for example to merge partitions and merge rows. While it is useful from development point of view to avoid duplicated code - it is non-friendly for JIT, we have polluted profiles with more that 2 types per call site, so we pay for it with frequent metamorphic calls, it is especially costly when we iterate over cells. A possible way to avoid this issue is to copy the class during a build time to create a separate class with exactly the same code but a different name. It allows to avoid development overheads with duplicated code to edit and at the same time helps JIT to optimize.
When we created a merged row we need to calculated minDeletionTime. If none of the merged rows had any deletion or expiring data, neither does the result, so we can pass the already-known min local deletion time and avoid rescanning the whole btree to recompute it.
DeletionTime.deletes check can skip Cell.timestamp invocation (potentially megamorphic) if the DeletionTime is LIVE.
In most cases merged cells have identical types, so we can add a fast path for it to Row.Merger.ColumnDataReducer#useColumnMetadata
ColumnDataReducer.versions can be switched from List to array to reduce allocations and to avoid extra deferences and checks/conditions.
replaceAndSink is large but we can split it to hot and colder parts to allow it to inline hot part into reduce method