impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-2020: Inline big number strings
Date Fri, 03 Mar 2017 04:33:46 GMT
Hello Dan Hecht,

I'd like you to reexamine a change.  Please visit

to look at the new patch set (#12).

Change subject: IMPALA-2020: Inline big number strings

IMPALA-2020: Inline big number strings

We can directly spcecify these in the header instead of keeping the
constants out in a .o.  This lets the compiler inline them in a lot
more places, potentially giving better behavior than a table lookup
by directly inserting the constant.  This in turn allows divides by
now known compile time constants to be converted into multiply.

This is likely to become even more imporant with big integer divide,
which requires additional computation for DECIMAL_V2.

Testing: Compare the binary output of before and
after; ran expr test suite.

Using the following query I observe a small but statistically present
win of ~3% over an unpatched build.

select sum(l_extendedprice / l_discount) from tpch10_parquet.lineitem;


| sum(l_extendedprice / l_discount) |
| 61076151920731.010714279183910    |
Fetched 1 row(s) in 9.05s


| sum(l_extendedprice / l_discount) |
| 61076151920731.010714279183910    |
Fetched 1 row(s) in 8.79s

Will do some additional benchmarking after the V2 stuff and rounding
changes land.

Change-Id: I5095a366d914cebb0b64bd434a08dbb55c90ed30
M be/src/benchmarks/
M be/src/common/
M be/src/runtime/decimal-value.inline.h
M be/src/util/
M be/src/util/decimal-util.h
5 files changed, 10 insertions(+), 26 deletions(-)

  git pull ssh:// refs/changes/02/5902/12
To view, visit
To unsubscribe, visit

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5095a366d914cebb0b64bd434a08dbb55c90ed30
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Zach Amsden <>

View raw message