Taras Bobrovytsky 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 Done PS13, Line 1091: int target_capacity = INIT_CAPACITY; : while (target_capacity < necessary_capacity) target_capacity *= 2; > BitUtil::RoundUpToPowerOfTwo Done 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 ceilin Done. IncreaseCapacity() handles the ceiling. PS13, Line 1163: may > may be Done 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 const I made some changes. I am not sure if it makes sense to pass max_capacity as a parameter, but the ceiling is calculated here as you suggested. Let me know if my change makes sense to you. -- 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
