[jira] [Comment Edited] (CASSANDRA-6662) Sort/reconcile cells in ArrayBackedSortedColumns only when an accessor is called

["Benedict (JIRA)" <ji...@apache.org>]

    [ 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)