You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Benedict Elliott Smith (Jira)" <ji...@apache.org> on 2019/12/20 00:22:00 UTC
[jira] [Comment Edited] (CASSANDRA-15397) IntervalTree performance
comparison with Linear Walk and Binary Search based Elimination.
[ https://issues.apache.org/jira/browse/CASSANDRA-15397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17000505#comment-17000505 ]
Benedict Elliott Smith edited comment on CASSANDRA-15397 at 12/20/19 12:21 AM:
-------------------------------------------------------------------------------
The code looks pretty simple and clean, though I will need to look more carefully before we consider merging. We would want to rename the class, since it's no longer a tree, and we would probably want to avoid the extra work of going via streams (which looks like it would allocate O(\n) extra data).
As it stands, I would probably want to see more performance comparisons, particularly out to more outlandish numbers of sstables (at least one million), and also for excessively skewed distributions. Since the benefits shown in your graphs are modest in absolute terms, and the potential algorithmic harms of a linear scan could have significant downside risk. So we need to be sure we have properly established what that might be.
There are some potential simple improvements to this approach that would make it more desirable: instead of maintaining two lists of interval objects, we could maintain four {{long[]}}; two matches pairs of {{long[]}} each representing the two current sorted lists. One would be used for binary search, the other for the linear scan. This should dramatically improve the constant factors, only needing to support post-filtering for e.g. {{RandomPartitioner}}, as we would only be able to filter on a prefix for those tokens > 8 bytes.
Although this approach would require slightly more involved modifications, and a bit more verification, the win should be much more pronounced. Is this something you'd be willing to try?
was (Author: benedict):
The code looks pretty simple and clean, though I will need to look more carefully before we consider merging. We would want to rename the class, since it's no longer a tree, and we would probably want to avoid the extra work of going via streams (which looks like it would allocate O(n) extra data).
As it stands, I would probably want to see more performance comparisons, particularly out to more outlandish numbers of sstables (at least one million), and also for excessively skewed distributions. Since the benefits shown in your graphs are modest in absolute terms, and the potential algorithmic harms of a linear scan could have significant downside risk. So we need to be sure we have properly established what that might be.
There are some potential simple improvements to this approach that would make it more desirable: instead of maintaining two lists of interval objects, we could maintain four {{long[]}}; two matches pairs of {{long[]}} each representing the two current sorted lists. One would be used for binary search, the other for the linear scan. This should dramatically improve the constant factors, only needing to support post-filtering for e.g. {{RandomPartitioner}}, as we would only be able to filter on a prefix for those tokens > 8 bytes.
Although this approach would require slightly more involved modifications, and a bit more verification, the win should be much more pronounced. Is this something you'd be willing to try?
> IntervalTree performance comparison with Linear Walk and Binary Search based Elimination.
> ------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-15397
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15397
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/SSTable
> Reporter: Chandrasekhar Thumuluru
> Assignee: Chandrasekhar Thumuluru
> Priority: Low
> Labels: pull-request-available
> Attachments: 95p_10000_SSTable_with_5000_Searches.png, 95p_15000_SSTable_with_5000_Searches.png, 95p_20000_SSTable_with_5000_Searches.png, 95p_25000_SSTable_with_5000_Searches.png, 95p_30000_SSTable_with_5000_Searches.png, 95p_5000_SSTable_with_5000_Searches.png, 99p_10000_SSTable_with_5000_Searches.png, 99p_15000_SSTable_with_5000_Searches.png, 99p_20000_SSTable_with_5000_Searches.png, 99p_25000_SSTable_with_5000_Searches.png, 99p_30000_SSTable_with_5000_Searches.png, 99p_5000_SSTable_with_5000_Searches.png, IntervalList.java, IntervalListWithElimination.java, IntervalTreeSimplified.java, Mean_10000_SSTable_with_5000_Searches.png, Mean_15000_SSTable_with_5000_Searches.png, Mean_20000_SSTable_with_5000_Searches.png, Mean_25000_SSTable_with_5000_Searches.png, Mean_30000_SSTable_with_5000_Searches.png, Mean_5000_SSTable_with_5000_Searches.png, TESTS-TestSuites.xml.lz4, replace_intervaltree_with_intervallist.patch
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Cassandra uses IntervalTrees to identify the SSTables that overlap with search interval. In Cassandra, IntervalTrees are not mutated. They are recreated each time a mutation is required. This can be an issue during repairs. In fact we noticed such issues during repair.
> Since lists are cache friendly compared to linked lists and trees, I decided to compare the search performance with:
> * Linear Walk.
> * Elimination using Binary Search (idea is to eliminate intervals using start and end points of search interval).
> Based on the tests I ran, I noticed Binary Search based elimination almost always performs similar to IntervalTree or out performs IntervalTree based search. The cost of IntervalTree construction is also substantial and produces lot of garbage during repairs.
> I ran the tests using random intervals to build the tree/lists and another randomly generated search interval with 5000 iterations. I'm attaching all the relevant graphs. The x-axis in the graphs is the search interval coverage. 10p means the search interval covered 10% of the intervals. The y-axis is the time the search took in nanos.
> PS:
> # For the purpose of test, I simplified the IntervalTree by removing the data portion of the interval. Modified the template version (Java generics) to a specialized version.
> # I used the code from Cassandra version _3.11_.
> # Time in the graph is in nanos.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org