Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4787: Optimize APPX_MEDIAN() memory usage ......................................................................
Patch Set 8: (9 comments) http://gerrit.cloudera.org:8080/#/c/6025/8/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: Line 126: // TODO: this file should be cross compiled and then all of the builtin is this done? Line 961: struct ReservoirSampleState { i'd say this really turned into a class Line 967: // resize occurs, this needs to be updated from the outside. what does 'from the outside' mean? Line 977: ReservoirSampleState(int init_capacity) : use standard formatting Line 1016: // The array of ReservoirSamples starts right after ReservoirSampleState, so we use that's often done by putting an array of size 1 at the end of the header struct: ReservoirSample<T> samples[1]; and then you can do things like state.samples[5] = x; it's convenient and makes it explicit that you have a trailing var-len array. Line 1025: int64_t GetNext64(int64_t max) { while you're at it, this deserves a comment Line 1033: // Given a buffer that contains a ReservoirSampleState, resize the buffer so that it's its Line 1040: if (new_capacity * 2 >= MAX_CAPACITY) new_capacity = MAX_CAPACITY; if state->capacity is 10 and max_capacity is 40, this line sets new_capacity to 40. Line 1062: // If the array gets filled due to updates or merges, we reallocate a larger buffer to you should put this (= a brief description of what you're doing) somewhere at the beginning of the reservoir sample-related code. -- 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: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-HasComments: Yes
