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 decimal-util.cc 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; Before: +-----------------------------------+ | sum(l_extendedprice / l_discount) | +-----------------------------------+ | 61076151920731.010714279183910 | +-----------------------------------+ Fetched 1 row(s) in 9.05s After: +-----------------------------------+ | 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 Reviewed-on: http://gerrit.cloudera.org:8080/5902 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/2644d6a6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/2644d6a6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/2644d6a6 Branch: refs/heads/master Commit: 2644d6a68824deda0db3d13481e64e713d60027b Parents: 3435321 Author: Zach Amsden <[email protected]> Authored: Sat Feb 4 01:18:22 2017 +0000 Committer: Impala Public Jenkins <[email protected]> Committed: Sat Mar 4 02:24:47 2017 +0000 ---------------------------------------------------------------------- be/src/benchmarks/overflow-benchmark.cc | 1 - be/src/common/init.cc | 1 - be/src/runtime/decimal-value.inline.h | 7 ++----- be/src/util/decimal-util.cc | 16 +++------------- be/src/util/decimal-util.h | 11 +++++------ 5 files changed, 10 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2644d6a6/be/src/benchmarks/overflow-benchmark.cc ---------------------------------------------------------------------- diff --git a/be/src/benchmarks/overflow-benchmark.cc b/be/src/benchmarks/overflow-benchmark.cc index 3ee020f..1d4caf6 100644 --- a/be/src/benchmarks/overflow-benchmark.cc +++ b/be/src/benchmarks/overflow-benchmark.cc @@ -431,7 +431,6 @@ void TestClzBranchFree(int batch_size, void* d) { int main(int argc, char** argv) { CpuInfo::Init(); - DecimalUtil::InitMaxUnscaledDecimal16(); cout << Benchmark::GetMachineInfo() << endl; TestData data; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2644d6a6/be/src/common/init.cc ---------------------------------------------------------------------- diff --git a/be/src/common/init.cc b/be/src/common/init.cc index a1e734b..1e90f53 100644 --- a/be/src/common/init.cc +++ b/be/src/common/init.cc @@ -171,7 +171,6 @@ void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm, DiskInfo::Init(); MemInfo::Init(); OsInfo::Init(); - DecimalUtil::InitMaxUnscaledDecimal16(); TestInfo::Init(test_mode); // Verify CPU meets the minimum requirements before calling InitGoogleLoggingSafe() http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2644d6a6/be/src/runtime/decimal-value.inline.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/decimal-value.inline.h b/be/src/runtime/decimal-value.inline.h index 5782bfe..f06ba76 100644 --- a/be/src/runtime/decimal-value.inline.h +++ b/be/src/runtime/decimal-value.inline.h @@ -38,11 +38,8 @@ inline DecimalValue<T> DecimalValue<T>::FromDouble(int precision, int scale, dou // Multiply the double by the scale. // Unfortunately, this conversion is not exact, and there is a loss of precision. - // Despite having enough bits in the mantissa to represent all the leading bits in - // powers of 10 up to around 10**38, the conversion done is still inexact. Writing - // literals directly as 1.0e23 produces exactly the same number. The error starts - // around 1.0e23 and can take either positive or negative values. This means the - // multiplication can cause an unwanted decimal overflow. + // The error starts around 1.0e23 and can take either positive or negative values. + // This means the multiplication can cause an unwanted decimal overflow. d *= DecimalUtil::GetScaleMultiplier<double>(scale); // Decimal V2 behavior http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2644d6a6/be/src/util/decimal-util.cc ---------------------------------------------------------------------- diff --git a/be/src/util/decimal-util.cc b/be/src/util/decimal-util.cc index a1c626e..b6196f2 100644 --- a/be/src/util/decimal-util.cc +++ b/be/src/util/decimal-util.cc @@ -20,18 +20,8 @@ namespace impala { -const int32_t DecimalUtil::MAX_UNSCALED_DECIMAL4 = 999999999; -const int64_t DecimalUtil::MAX_UNSCALED_DECIMAL8 = 999999999999999999; -int128_t DecimalUtil::MAX_UNSCALED_DECIMAL16; - -// We can't write 38 9's as a literal in C++, so generate it a different way. -void DecimalUtil::InitMaxUnscaledDecimal16() { - // TODO: is there a better way to do this? - MAX_UNSCALED_DECIMAL16 = 0; - for (int i = 0; i < ColumnType::MAX_PRECISION; ++i) { - MAX_UNSCALED_DECIMAL16 *= 10; - MAX_UNSCALED_DECIMAL16 += 9; - } -} +const int32_t DecimalUtil::MAX_UNSCALED_DECIMAL4; +const int64_t DecimalUtil::MAX_UNSCALED_DECIMAL8; +const int128_t DecimalUtil::MAX_UNSCALED_DECIMAL16; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/2644d6a6/be/src/util/decimal-util.h ---------------------------------------------------------------------- diff --git a/be/src/util/decimal-util.h b/be/src/util/decimal-util.h index d15de0a..9048793 100644 --- a/be/src/util/decimal-util.h +++ b/be/src/util/decimal-util.h @@ -32,16 +32,15 @@ namespace impala { class DecimalUtil { public: /// Maximum absolute value of a valid Decimal4Value. This is 9 digits of 9's. - static const int32_t MAX_UNSCALED_DECIMAL4; + static const int32_t MAX_UNSCALED_DECIMAL4 = 999999999; /// Maximum absolute value of a valid Decimal8Value. This is 18 digits of 9's. - static const int64_t MAX_UNSCALED_DECIMAL8; + static const int64_t MAX_UNSCALED_DECIMAL8 = 999999999999999999; /// Maximum absolute value a valid Decimal16Value. This is 38 digits of 9's. - static int128_t MAX_UNSCALED_DECIMAL16; - - /// Initializes MAX_UNSCALED_DECIMAL16. Must be called once before using it. - static void InitMaxUnscaledDecimal16(); + static const int128_t MAX_UNSCALED_DECIMAL16 = 99 + 100 * + (MAX_UNSCALED_DECIMAL8 + (1 + MAX_UNSCALED_DECIMAL8) * + static_cast<int128_t>(MAX_UNSCALED_DECIMAL8)); /// TODO: do we need to handle overflow here or at a higher abstraction. template<typename T>
