[
https://issues.apache.org/jira/browse/CASSANDRA-6662?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13897989#comment-13897989
]
Benedict edited comment on CASSANDRA-6662 at 2/11/14 4:35 PM:
--------------------------------------------------------------
On the whole LGTM. A few suggestions you're free to ignore:
- internalAppend/AndReconcile should be colocated with internalAdd etc
- in maybeSort(), leftCopy could be optimised to assign a static EMPTY array
for the typical case of being called when the whole thing is unsorted
- I don't think there much to be gained by binary search in addColumn; I would
simply attempt a fast sorted append (no reconcile), and if that fails just
perform an unsorted append and let it clean up next time. It may be that we
sometimes overwrite the same values in a CF, but I'm not sure that it happens
frequently enough to warrant the complexity.
- May be worth extracting the actual sorting from maybeSort() into sort() and
calling it conditionally, as it is currently too long to be inlined. This way
the conditional should be inlined so we don't incur the method invoke cost
except when we have to. I wouldn't ordinarily worry about this, but we call
these methods extensively.
- I have a slight preference, when merging two arrays, to use the three-loop
method instead of the one loop with three branches of an if statement. I think
it is clearer what is happening. Just a suggestion though.
I've pushed a version with my suggestions to
[iss-6662|https://github.com/belliottsmith/cassandra/tree/iss-6662-2]
was (Author: benedict):
On the whole LGTM. A few suggestions you're free to ignore:
- internalAppend/AndReconcile should be colocated with internalAdd etc
- in maybeSort(), leftCopy could be optimised to assign a static EMPTY array
for the typical case of being called when the whole thing is unsorted
- I don't think there much to be gained by binary search in addColumn; I would
simply attempt a fast sorted append (no reconcile), and if that fails just
perform an unsorted append and let it clean up next time. It may be that we
sometimes overwrite the same values in a CF, but I'm not sure that it happens
frequently enough to warrant the complexity.
- addColumn can use internalAppend instead of internalAdd
- May be worth extracting the actual sorting from maybeSort() into sort() and
calling it conditionally, as it is currently too long to be inlined. This way
the conditional should be inlined so we don't incur the method invoke cost
except when we have to. I wouldn't ordinarily worry about this, but we call
these methods extensively.
- I have a slight preference, when merging two arrays, to use the three-loop
method instead of the one loop with three branches of an if statement. I think
it is clearer what is happening. Just a suggestion though.
I've pushed a version with my suggestions to
[iss-6662|https://github.com/belliottsmith/cassandra/tree/iss-6662-2]
> Sort/reconcile cells in ArrayBackedSortedColumns only when an accessor is
> called
> --------------------------------------------------------------------------------
>
> Key: CASSANDRA-6662
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6662
> Project: Cassandra
> Issue Type: Improvement
> Reporter: Aleksey Yeschenko
> Assignee: Aleksey Yeschenko
> Fix For: 2.1
>
>
> To avoid poor performance with huge numbers of cells added out of order
> (which should be rare, but *can* happen with certain batch scenarios) we
> should make ABSC only sort/reconcile its cells when an actual accessor is
> actually called, delaying sorting until the very end.
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
