Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4787: Optimize APPX_MEDIAN() memory usage ......................................................................
Patch Set 13: (5 comments) http://gerrit.cloudera.org:8080/#/c/6025/13/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: PS13, Line 1075: capacity_ = num_samples_; : sample_array_inline_ = true; as we discussed in person, let's move this to serialize which is when this happens PS13, Line 1091: int target_capacity = INIT_CAPACITY; : while (target_capacity < necessary_capacity) target_capacity *= 2; BitUtil::RoundUpToPowerOfTwo PS13, Line 1093: if (target_capacity > MAX_CAPACITY) target_capacity = MAX_CAPACITY; : bool result = IncreaseCapacity(ctx, target_capacity); this is simplified if IncreaseCapacity takes the max and handles the ceiling PS13, Line 1163: may may be PS13, Line 1168: // if it hasn't been allocated yet. Returns false if the operation fails. : bool IncreaseCapacity(FunctionContext* ctx, int new_capacity) { If you change this to take max_capacity (which always should pass the constant MAX_CAPACITY but makes the interface to this fn reasonable) and do the ceiling in here, then you don't really need DoubleCapacity, and you can simplify some logic in Merge as well. -- To view, visit http://gerrit.cloudera.org:8080/6025 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99adaad574d4fb0a3cf38c6cbad8b2a23df12968 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
