impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4787: Optimize APPX MEDIAN() memory usage
Date Mon, 06 Mar 2017 23:39:32 GMT
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 <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message