Jianfeng Jia has posted comments on this change. Change subject: Updated DeletableFrameTupleAppender to support reusing index slots. ......................................................................
Patch Set 1: (9 comments) Looks good in general. Just a few minor comments. https://asterix-gerrit.ics.uci.edu/#/c/1214/1/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/util/DeletableFrameTupleAppender.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/util/DeletableFrameTupleAppender.java: Line 34: * The offsets also store both the start and end values because tuples may be out of out of "range"? Line 41: * <li><code>tuple_append</code> holds an int indicating the offset used to append the next tuple.</li> maybe it helps to explain the difference between `tuple_append` and the `next_index`? Line 79: private IntegerPairPool ipp = new IntegerPairPool(); it's better to unify the initialization place, either all inside constructor or outside. Line 80: ipp should be final variable as well? Line 88: private int getIndexCount() { it's often more clear to move the private functions to the end. I just realized it recently ... Line 128: if (getTupleEndOffset(nextIndex) <= 0) { is it possible to merge this condition with the while loop? Line 160: setTupleAppend(0); If I understand correctly, this offset should add some paddings? Line 279: } do we need to set the member variable `nextIndex` as well? https://asterix-gerrit.ics.uci.edu/#/c/1214/1/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/util/IntegerPairPool.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/sort/util/IntegerPairPool.java: Line 26: list = new ArrayList<>(); since the behavior is just adding and removing to and from tail, it would be better to use LinkedList instead of ArrayList -- To view, visit https://asterix-gerrit.ics.uci.edu/1214 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05cda3360cc503bf847aeca5998be06f1f7d3c15 Gerrit-PatchSet: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Preston Carman <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
