This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit c5b7c6395b1f161bbc55596bb3694fbcf3e658c6 Author: stiga-huang <[email protected]> AuthorDate: Fri Feb 24 15:59:59 2023 +0800 IMPALA-11943: Mark utf8 string functions with IR_ALWAYS_INLINE String functions that have both UTF-8 and the traditional ASCII behaviors have checks for the UTF8_MODE query option. The check is intended to be replaced with constants during codegen in LlvmCodeGen::InlineConstFnAttrs(). However, as mentioned in the method comment, InlineConstFnAttrs() only replaces call instructions inside the current function. To replace the call to FunctionContextImpl::GetConstFnAttr() inside the callee functions, we have to inline the callee functions (by annotating them with IR_ALWAYS_INLINE). This patch annotates UTF-8 related string functions with IR_ALWAYS_INLINE to make sure the checks on UTF8_MODE are all replaced in codegen. Note that builtin functions don't need the annotation if they are not invoked by other builtin functions, because InlineConstFnAttrs() will be invoked recursively in the expression tree. See ScalarFnCall::GetCodegendComputeFnImpl(). Perf tests: Ran PERF_STRING queries in targeted-perf (scale factor 100) on parquet format. Saw significant improvements: +-----------------+--------------+--------+-------------+------------+-----------+----------------+-------+----------------+---------+---------+ | Query | File Format | Avg(s) | Base Avg(s) | Delta(Avg) | StdDev(%) | Base StdDev(%) | Iters | Median Diff(%) | MW Zval | Tval | +-----------------+--------------+--------+-------------+------------+-----------+----------------+-------+----------------+---------+---------+ | PERF_STRING-Q6 | parquet/none | 11.98 | 12.53 | -4.39% | 0.44% | 0.51% | 30 | -4.59% | -6.54 | -36.46 | | PERF_STRING-Q9 | parquet/none | 11.76 | 12.35 | -4.77% | 0.38% | 0.41% | 30 | -5.04% | -6.54 | -47.82 | | PERF_STRING-Q7 | parquet/none | 9.88 | 10.44 | I -5.34% | 0.83% | 1.10% | 30 | I -5.64% | -6.54 | -21.64 | | PERF_STRING-Q13 | parquet/none | 9.52 | 10.08 | I -5.56% | 0.55% | 0.59% | 30 | I -5.89% | -6.54 | -38.72 | | PERF_STRING-Q11 | parquet/none | 10.97 | 11.64 | I -5.72% | 0.44% | 0.61% | 30 | I -6.00% | -6.54 | -42.72 | | PERF_STRING-Q4 | parquet/none | 5.30 | 5.66 | I -6.33% | 1.06% | 1.44% | 30 | I -6.49% | -6.54 | -19.84 | | PERF_STRING-Q3 | parquet/none | 5.27 | 5.70 | I -7.43% | 1.03% | 0.94% | 30 | I -8.12% | -6.54 | -30.40 | | PERF_STRING-Q5 | parquet/none | 5.87 | 6.36 | I -7.69% | 1.18% | 0.88% | 30 | I -8.47% | -6.54 | -30.09 | | PERF_STRING-Q12 | parquet/none | 6.56 | 7.15 | I -8.33% | 0.67% | 0.73% | 30 | I -9.03% | -6.54 | -47.94 | | PERF_STRING-Q10 | parquet/none | 6.62 | 7.24 | I -8.54% | 0.84% | 0.60% | 30 | I -9.34% | -6.54 | -48.21 | | PERF_STRING-Q2 | parquet/none | 4.77 | 5.25 | I -9.20% | 0.89% | 1.05% | 30 | I -10.17% | -6.54 | -37.90 | | PERF_STRING-Q1 | parquet/none | 4.16 | 4.62 | I -9.80% | 2.10% | 1.10% | 30 | I -11.27% | -6.54 | -24.50 | | PERF_STRING-Q8 | parquet/none | 5.10 | 6.57 | I -22.40% | 0.87% | 0.73% | 30 | I -28.74% | -6.54 | -123.34 | +-----------------+--------------+--------+-------------+------------+-----------+----------------+-------+----------------+---------+---------+ Note that Q8-Q13 are new queries added by this patch to reveal the performance difference. The perf test command is bin/single_node_perf_run.py --iterations 30 --scale 100 \ --table_formats parquet/none --num_impalads 3 \ --query_names 'PERF_STRING.*' f4a321cf8 c8d2b6a90 Running on this branch: https://github.com/stiga-huang/impala/commits/inline-utf8-func-perf-test Change-Id: I19e8fba332ae329da8b1d37dba3bbc64f59e6f3a Reviewed-on: http://gerrit.cloudera.org:8080/19535 Reviewed-by: Daniel Becker <[email protected]> Reviewed-by: Csaba Ringhofer <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- be/src/exprs/mask-functions-ir.cc | 75 +++++++++++++--------- be/src/exprs/string-functions-ir.cc | 17 +++-- .../workloads/targeted-perf/queries/string.test | 60 +++++++++++++++++ 3 files changed, 117 insertions(+), 35 deletions(-) diff --git a/be/src/exprs/mask-functions-ir.cc b/be/src/exprs/mask-functions-ir.cc index b6725ea5c..689da79d0 100644 --- a/be/src/exprs/mask-functions-ir.cc +++ b/be/src/exprs/mask-functions-ir.cc @@ -23,6 +23,7 @@ #include <openssl/err.h> #include <openssl/sha.h> +#include "codegen/impala-ir.h" #include "exprs/anyval-util.h" #include "exprs/math-functions.h" #include "exprs/string-functions.h" @@ -213,9 +214,9 @@ static int GetUtf8CodePointCount(FunctionContext* ctx, const StringVal& val) { /// Mask the given string except the first 'un_mask_char_count' chars. Ported from /// org.apache.hadoop.hive.ql.udf.generic.GenericUDFMaskShowFirstN. -static inline StringVal MaskShowFirstNImpl(FunctionContext* ctx, const StringVal& val, - int un_mask_char_count, int masked_upper_char, int masked_lower_char, - int masked_digit_char, int masked_other_char) { +IR_ALWAYS_INLINE static inline StringVal MaskShowFirstNImpl(FunctionContext* ctx, + const StringVal& val, int un_mask_char_count, int masked_upper_char, + int masked_lower_char, int masked_digit_char, int masked_other_char) { // To be consistent with Hive, negative char_count is treated as 0. if (un_mask_char_count < 0) un_mask_char_count = 0; if (val.is_null || val.len == 0 || un_mask_char_count >= val.len) return val; @@ -229,9 +230,9 @@ static inline StringVal MaskShowFirstNImpl(FunctionContext* ctx, const StringVal /// Mask the given string except the last 'un_mask_char_count' chars. Ported from /// org.apache.hadoop.hive.ql.udf.generic.GenericUDFMaskShowLastN. -static inline StringVal MaskShowLastNImpl(FunctionContext* ctx, const StringVal& val, - int un_mask_char_count, int masked_upper_char, int masked_lower_char, - int masked_digit_char, int masked_other_char) { +IR_ALWAYS_INLINE static inline StringVal MaskShowLastNImpl(FunctionContext* ctx, + const StringVal& val, int un_mask_char_count, int masked_upper_char, + int masked_lower_char, int masked_digit_char, int masked_other_char) { // To be consistent with Hive, negative char_count is treated as 0. if (un_mask_char_count < 0) un_mask_char_count = 0; if (val.is_null || val.len == 0 || un_mask_char_count >= val.len) return val; @@ -247,9 +248,9 @@ static inline StringVal MaskShowLastNImpl(FunctionContext* ctx, const StringVal& /// Mask the first 'mask_char_count' chars of the given string. Ported from /// org.apache.hadoop.hive.ql.udf.generic.GenericUDFMaskFirstN. -static inline StringVal MaskFirstNImpl(FunctionContext* ctx, const StringVal& val, - int mask_char_count, int masked_upper_char, int masked_lower_char, - int masked_digit_char, int masked_other_char) { +IR_ALWAYS_INLINE static inline StringVal MaskFirstNImpl(FunctionContext* ctx, + const StringVal& val, int mask_char_count, int masked_upper_char, + int masked_lower_char, int masked_digit_char, int masked_other_char) { if (mask_char_count <= 0 || val.is_null || val.len == 0) return val; if (mask_char_count > val.len) mask_char_count = val.len; if (!ctx->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE)) { @@ -262,9 +263,9 @@ static inline StringVal MaskFirstNImpl(FunctionContext* ctx, const StringVal& va /// Mask the last 'mask_char_count' chars of the given string. Ported from /// org.apache.hadoop.hive.ql.udf.generic.GenericUDFMaskLastN. -static inline StringVal MaskLastNImpl(FunctionContext* ctx, const StringVal& val, - int mask_char_count, int masked_upper_char, int masked_lower_char, - int masked_digit_char, int masked_other_char) { +IR_ALWAYS_INLINE static inline StringVal MaskLastNImpl(FunctionContext* ctx, + const StringVal& val, int mask_char_count, int masked_upper_char, + int masked_lower_char, int masked_digit_char, int masked_other_char) { if (mask_char_count <= 0 || val.is_null || val.len == 0) return val; if (mask_char_count > val.len) mask_char_count = val.len; if (!ctx->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE)) { @@ -279,9 +280,9 @@ static inline StringVal MaskLastNImpl(FunctionContext* ctx, const StringVal& val /// Mask the whole given string. Ported from /// org.apache.hadoop.hive.ql.udf.generic.GenericUDFMask. -static inline StringVal MaskImpl(FunctionContext* ctx, const StringVal& val, - int masked_upper_char, int masked_lower_char, int masked_digit_char, - int masked_other_char) { +IR_ALWAYS_INLINE static inline StringVal MaskImpl(FunctionContext* ctx, + const StringVal& val, int masked_upper_char, int masked_lower_char, + int masked_digit_char, int masked_other_char) { if (val.is_null || val.len == 0) return val; if (!ctx->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE)) { return MaskSubStr(ctx, val, 0, val.len, masked_upper_char, @@ -410,8 +411,8 @@ static DateVal MaskImpl(FunctionContext* ctx, const DateVal& val, int day_value, /// Gets the first character of 'str'. Returns 'default_value' if 'str' is empty. /// In UTF-8 mode, the first code point is returned. /// Otherwise, the first char is returned. -static inline uint32_t GetFirstChar(FunctionContext* ctx, const StringVal& str, - uint32_t default_value) { +IR_ALWAYS_INLINE static inline uint32_t GetFirstChar(FunctionContext* ctx, + const StringVal& str, uint32_t default_value) { // To be consistent with Hive, empty string is converted to default value. String with // length > 1 will only use its first char. if (str.len == 0) return default_value; @@ -453,9 +454,12 @@ StringVal MaskFunctions::MaskShowFirstN(FunctionContext* ctx, const StringVal& v return MaskShowFirstNImpl(ctx, val, un_mask_char_count, MASKED_UPPERCASE, MASKED_LOWERCASE, MASKED_DIGIT, MASKED_OTHER_CHAR); } -StringVal MaskFunctions::MaskShowFirstN(FunctionContext* ctx, const StringVal& val, - const IntVal& char_count, const StringVal& upper_char, const StringVal& lower_char, - const StringVal& digit_char, const StringVal& other_char) { +// Marked as IR_ALWAYS_INLINE since this is called in another overload. +// This way the overload can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal MaskFunctions::MaskShowFirstN(FunctionContext* ctx, + const StringVal& val, const IntVal& char_count, const StringVal& upper_char, + const StringVal& lower_char, const StringVal& digit_char, + const StringVal& other_char) { return MaskShowFirstNImpl(ctx, val, char_count.val, GetFirstChar(ctx, upper_char, MASKED_UPPERCASE), GetFirstChar(ctx, lower_char, MASKED_LOWERCASE), @@ -540,9 +544,12 @@ StringVal MaskFunctions::MaskShowLastN(FunctionContext* ctx, const StringVal& va return MaskShowLastNImpl(ctx, val, un_mask_char_count, MASKED_UPPERCASE, MASKED_LOWERCASE, MASKED_DIGIT, MASKED_OTHER_CHAR); } -StringVal MaskFunctions::MaskShowLastN(FunctionContext* ctx, const StringVal& val, - const IntVal& char_count, const StringVal& upper_char, const StringVal& lower_char, - const StringVal& digit_char, const StringVal& other_char) { +// Marked as IR_ALWAYS_INLINE since this is called in another overload. +// This way the overload can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal MaskFunctions::MaskShowLastN(FunctionContext* ctx, + const StringVal& val, const IntVal& char_count, const StringVal& upper_char, + const StringVal& lower_char, const StringVal& digit_char, + const StringVal& other_char) { return MaskShowLastNImpl(ctx, val, char_count.val, GetFirstChar(ctx, upper_char, MASKED_UPPERCASE), GetFirstChar(ctx, lower_char, MASKED_LOWERCASE), @@ -617,9 +624,12 @@ StringVal MaskFunctions::MaskFirstN(FunctionContext* ctx, const StringVal& val, return MaskFirstNImpl(ctx, val, char_count.val, MASKED_UPPERCASE, MASKED_LOWERCASE, MASKED_DIGIT, MASKED_OTHER_CHAR); } -StringVal MaskFunctions::MaskFirstN(FunctionContext* ctx, const StringVal& val, - const IntVal& char_count, const StringVal& upper_char, const StringVal& lower_char, - const StringVal& digit_char, const StringVal& other_char) { +// Marked as IR_ALWAYS_INLINE since this is called in another overload. +// This way the overload can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal MaskFunctions::MaskFirstN(FunctionContext* ctx, + const StringVal& val, const IntVal& char_count, const StringVal& upper_char, + const StringVal& lower_char, const StringVal& digit_char, + const StringVal& other_char) { return MaskFirstNImpl(ctx, val, char_count.val, GetFirstChar(ctx, upper_char, MASKED_UPPERCASE), GetFirstChar(ctx, lower_char, MASKED_LOWERCASE), @@ -694,9 +704,12 @@ StringVal MaskFunctions::MaskLastN(FunctionContext* ctx, const StringVal& val, return MaskLastNImpl(ctx, val, char_count.val, MASKED_UPPERCASE, MASKED_LOWERCASE, MASKED_DIGIT, MASKED_OTHER_CHAR); } -StringVal MaskFunctions::MaskLastN(FunctionContext* ctx, const StringVal& val, - const IntVal& char_count, const StringVal& upper_char, const StringVal& lower_char, - const StringVal& digit_char, const StringVal& other_char) { +// Marked as IR_ALWAYS_INLINE since this is called in another overload. +// This way the overload can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal MaskFunctions::MaskLastN(FunctionContext* ctx, + const StringVal& val, const IntVal& char_count, const StringVal& upper_char, + const StringVal& lower_char, const StringVal& digit_char, + const StringVal& other_char) { return MaskLastNImpl(ctx, val, char_count.val, GetFirstChar(ctx, upper_char, MASKED_UPPERCASE), GetFirstChar(ctx, lower_char, MASKED_LOWERCASE), @@ -766,7 +779,9 @@ StringVal MaskFunctions::Mask(FunctionContext* ctx, const StringVal& val) { return MaskImpl(ctx, val, MASKED_UPPERCASE, MASKED_LOWERCASE, MASKED_DIGIT, MASKED_OTHER_CHAR); } -StringVal MaskFunctions::Mask(FunctionContext* ctx, const StringVal& val, +// Marked as IR_ALWAYS_INLINE since this is called in other overloads. +// This way the overloads can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal MaskFunctions::Mask(FunctionContext* ctx, const StringVal& val, const StringVal& upper_char, const StringVal& lower_char, const StringVal& digit_char, const StringVal& other_char) { return MaskImpl(ctx, val, diff --git a/be/src/exprs/string-functions-ir.cc b/be/src/exprs/string-functions-ir.cc index dc9468328..0a2319483 100644 --- a/be/src/exprs/string-functions-ir.cc +++ b/be/src/exprs/string-functions-ir.cc @@ -57,7 +57,9 @@ uint64_t StringFunctions::re2_mem_limit_ = 8 << 20; // - 1-indexed positions // - supported negative positions (count from the end of the string) // - [optional] len. No len indicates longest substr possible -StringVal StringFunctions::Substring(FunctionContext* context, +// Marks as IR_ALWAYS_INLINE since this is called in other overloads. +// So the overloads can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal StringFunctions::Substring(FunctionContext* context, const StringVal& str, const BigIntVal& pos, const BigIntVal& len) { if (str.is_null || pos.is_null || len.is_null) return StringVal::null(); if (context->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE)) { @@ -257,7 +259,7 @@ StringVal StringFunctions::Rpad(FunctionContext* context, const StringVal& str, IntVal StringFunctions::Length(FunctionContext* context, const StringVal& str) { if (str.is_null) return IntVal::null(); - if (context->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE, 0)) { + if (context->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE)) { return Utf8Length(context, str); } return IntVal(str.len); @@ -288,7 +290,10 @@ IntVal StringFunctions::Utf8Length(FunctionContext* context, const StringVal& st return IntVal(CountUtf8Chars(str.ptr, str.len)); } -StringVal StringFunctions::Lower(FunctionContext* context, const StringVal& str) { +// Marks as IR_ALWAYS_INLINE since this is called in MaskFunctions::MaskHash(). +// So the caller can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE StringVal StringFunctions::Lower(FunctionContext* context, + const StringVal& str) { if (str.is_null) return StringVal::null(); if (context->impl()->GetConstFnAttr(FunctionContextImpl::UTF8_MODE)) { return LowerUtf8(context, str); @@ -879,8 +884,10 @@ IntVal StringFunctions::Ascii(FunctionContext* context, const StringVal& str) { return IntVal((str.len == 0) ? 0 : static_cast<int32_t>(str.ptr[0])); } -IntVal StringFunctions::Instr(FunctionContext* context, const StringVal& str, - const StringVal& substr, const BigIntVal& start_position, +// Marks as IR_ALWAYS_INLINE since this is called in other overloads. +// So the overloads can also replace GetConstFnAttr() calls in codegen. +IR_ALWAYS_INLINE IntVal StringFunctions::Instr(FunctionContext* context, + const StringVal& str, const StringVal& substr, const BigIntVal& start_position, const BigIntVal& occurrence) { if (str.is_null || substr.is_null || start_position.is_null || occurrence.is_null) { return IntVal::null(); diff --git a/testdata/workloads/targeted-perf/queries/string.test b/testdata/workloads/targeted-perf/queries/string.test index c42c31aca..f6f0faf03 100644 --- a/testdata/workloads/targeted-perf/queries/string.test +++ b/testdata/workloads/targeted-perf/queries/string.test @@ -56,3 +56,63 @@ WHERE lower(l_comment) = 'egular courts above the' ---- TYPES BIGINT ==== +---- QUERY: PERF_STRING-Q8 +-- Make sure utf8 string functions are inlined in jitted codes so the checks for +-- utf8 mode are replaced with constants +SELECT count(*) FROM lineitem +WHERE instr(l_comment, 'egular courts above the') > 0 +---- RESULTS +28 +---- TYPES +BIGINT +==== +---- QUERY: PERF_STRING-Q9 +-- Make sure utf8 string functions are inlined in jitted codes so the checks for +-- utf8 mode are replaced with constants +SELECT count(*) FROM lineitem +WHERE mask(l_comment) is null +---- RESULTS +0 +---- TYPES +BIGINT +==== +---- QUERY: PERF_STRING-Q10 +-- Make sure utf8 string functions are inlined in jitted codes so the checks for +-- utf8 mode are replaced with constants +SELECT count(*) FROM lineitem +WHERE mask_first_n(l_comment) is null +---- RESULTS +0 +---- TYPES +BIGINT +==== +---- QUERY: PERF_STRING-Q11 +-- Make sure utf8 string functions are inlined in jitted codes so the checks for +-- utf8 mode are replaced with constants +SELECT count(*) FROM lineitem +WHERE mask_show_first_n(l_comment) is null +---- RESULTS +0 +---- TYPES +BIGINT +==== +---- QUERY: PERF_STRING-Q12 +-- Make sure utf8 string functions are inlined in jitted codes so the checks for +-- utf8 mode are replaced with constants +SELECT count(*) FROM lineitem +WHERE mask_last_n(l_comment) is null +---- RESULTS +0 +---- TYPES +BIGINT +==== +---- QUERY: PERF_STRING-Q13 +-- Make sure utf8 string functions are inlined in jitted codes so the checks for +-- utf8 mode are replaced with constants +SELECT count(*) FROM lineitem +WHERE mask_show_last_n(l_comment) is null +---- RESULTS +0 +---- TYPES +BIGINT +====
