Yingyi Bu has posted comments on this change. Change subject: ASTERIXDB-1892: Sets a proper hash table cardinality during hash-group by ......................................................................
Patch Set 7: (4 comments) https://asterix-gerrit.ics.uci.edu/#/c/1702/5/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java: PS5, Line 268: length headers.length --> requireHeaderFrameCompaction.length() https://docs.oracle.com/javase/7/docs/api/java/util/BitSet.html#length() https://asterix-gerrit.ics.uci.edu/#/c/1702/7/hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java File hyracks-fullstack/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/structures/SerializableHashTable.java: PS7, Line 86: markHeaderFrameDirty markHeaderFrameDirty -> markHeaderFrameReclaimable It feels requireHeaderFrameCompaction can help very little -- a header is set even when there is only 2 bytes available but to reclaim it you need every byte available. I think there's better way to achieve this, e.g., maintain a used-slot-count in the header frame using the last four bytes. In this way, you don't need to iterative over all bytes in the header. Can we defer the GC of header to the next change and let this change only fix the OOM issue? PS7, Line 268: headers requireHeaderFrameCompaction.length() PS7, Line 269: if flip the if condition and reduce the nesting. -- To view, visit https://asterix-gerrit.ics.uci.edu/1702 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I651139b2b559ad4d2f6137a5c844814606516a90 Gerrit-PatchSet: 7 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Taewoo Kim <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-HasComments: Yes
