Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9078 )
Change subject: IMPALA-6422: Use ldexp() instead of powf() in HLL. ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/9078/1/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/9078/1/be/src/exprs/aggregate-functions-ir.cc@1489 PS1, Line 1489: // TODO: Consider improving this loop (e.g. replacing 'if' with arithmetic op). > Remove TODO while we're here? Seems more misleading than helpful since it's Done http://gerrit.cloudera.org:8080/#/c/9078/1/be/src/exprs/aggregate-functions-ir.cc@1512 PS1, Line 1512: uint64_t estimate = HllFinalEstimate(src.ptr, src.len); > I believe src.len is always HLL_LEN here. Might be worth passing the consta I just removed the num_buckets arg from HllFinalEstimate() due to the DCHECK_EQ(num_buckets, HLL_LEN); in L1474. Seems more honest to not pass num_buckets since we can only deal with HLL_LEN. Please take another quick look. -- To view, visit http://gerrit.cloudera.org:8080/9078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I517614d3f9cf1cf56b15a173c3cfe76e0f2e0382 Gerrit-Change-Number: 9078 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: anujphadke <[email protected]> Gerrit-Comment-Date: Fri, 19 Jan 2018 18:41:41 +0000 Gerrit-HasComments: Yes
