diff --git a/velox/expression/Expr.cpp b/velox/expression/Expr.cpp index dca94bd5e317..5d144b603824 100644 --- a/velox/expression/Expr.cpp +++ b/velox/expression/Expr.cpp @@ -547,7 +547,6 @@ Expr::PeelEncodingsResult Expr::peelEncodings( const auto& rowsToDecode = context->isFinalSelection() ? rows : *context->finalSelection(); decoded->makeIndices(*firstWrapper, rowsToDecode, numLevels); - auto indices = decoded->indices(); newRows = translateToInnerRows(rows, *decoded, newRowsHolder); @@ -1314,28 +1313,42 @@ namespace { void printExprWithStats( const exec::Expr& expr, const std::string& indent, - std::stringstream& out) { + std::stringstream& out, + std::unordered_map& uniqueExprs) { + auto it = uniqueExprs.find(&expr); + if (it != uniqueExprs.end()) { + // Common sub-expression. Print the full expression, but skip the stats. Add + // ID of the expression it duplicates. + out << indent << expr.toString(true) << " -> " << expr.type()->toString(); + out << " [CSE #" << it->second << "]" << std::endl; + return; + } + + uint32_t id = uniqueExprs.size() + 1; + uniqueExprs.insert({&expr, id}); + const auto& stats = expr.stats(); out << indent << expr.toString(false) << " [cpu time: " << succinctNanos(stats.timing.cpuNanos) << ", rows: " << stats.numProcessedRows << "] -> " - << expr.type()->toString() << std::endl; + << expr.type()->toString() << " [#" << id << "]" << std::endl; auto newIndent = indent + " "; for (const auto& input : expr.inputs()) { - printExprWithStats(*input, newIndent, out); + printExprWithStats(*input, newIndent, out, uniqueExprs); } } } // namespace std::string printExprWithStats(const exec::ExprSet& exprSet) { const auto& exprs = exprSet.exprs(); + std::unordered_map uniqueExprs; std::stringstream out; for (auto i = 0; i < exprs.size(); ++i) { if (i > 0) { out << std::endl; } - printExprWithStats(*exprs[i], "", out); + printExprWithStats(*exprs[i], "", out, uniqueExprs); } return out.str(); } diff --git a/velox/expression/tests/ExprStatsTest.cpp b/velox/expression/tests/ExprStatsTest.cpp index 7920dbaf6b9a..1d2e81a180fb 100644 --- a/velox/expression/tests/ExprStatsTest.cpp +++ b/velox/expression/tests/ExprStatsTest.cpp @@ -88,22 +88,22 @@ TEST_F(ExprStatsTest, printWithStats) { // Check stats before evaluation. ASSERT_EQ( exec::printExprWithStats(*exprSet), - "multiply [cpu time: 0ns, rows: 0] -> BIGINT\n" - " plus [cpu time: 0ns, rows: 0] -> BIGINT\n" - " cast(c0 as BIGINT) [cpu time: 0ns, rows: 0] -> BIGINT\n" - " c0 [cpu time: 0ns, rows: 0] -> INTEGER\n" - " 3:BIGINT [cpu time: 0ns, rows: 0] -> BIGINT\n" - " cast(c1 as BIGINT) [cpu time: 0ns, rows: 0] -> BIGINT\n" - " c1 [cpu time: 0ns, rows: 0] -> INTEGER\n" + "multiply [cpu time: 0ns, rows: 0] -> BIGINT [#1]\n" + " plus [cpu time: 0ns, rows: 0] -> BIGINT [#2]\n" + " cast(c0 as BIGINT) [cpu time: 0ns, rows: 0] -> BIGINT [#3]\n" + " c0 [cpu time: 0ns, rows: 0] -> INTEGER [#4]\n" + " 3:BIGINT [cpu time: 0ns, rows: 0] -> BIGINT [#5]\n" + " cast(c1 as BIGINT) [cpu time: 0ns, rows: 0] -> BIGINT [#6]\n" + " c1 [cpu time: 0ns, rows: 0] -> INTEGER [#7]\n" "\n" - "eq [cpu time: 0ns, rows: 0] -> BOOLEAN\n" - " mod [cpu time: 0ns, rows: 0] -> BIGINT\n" - " cast(plus as BIGINT) [cpu time: 0ns, rows: 0] -> BIGINT\n" - " plus [cpu time: 0ns, rows: 0] -> INTEGER\n" - " c0 [cpu time: 0ns, rows: 0] -> INTEGER\n" - " c1 [cpu time: 0ns, rows: 0] -> INTEGER\n" - " 2:BIGINT [cpu time: 0ns, rows: 0] -> BIGINT\n" - " 0:BIGINT [cpu time: 0ns, rows: 0] -> BIGINT\n"); + "eq [cpu time: 0ns, rows: 0] -> BOOLEAN [#8]\n" + " mod [cpu time: 0ns, rows: 0] -> BIGINT [#9]\n" + " cast(plus as BIGINT) [cpu time: 0ns, rows: 0] -> BIGINT [#10]\n" + " plus [cpu time: 0ns, rows: 0] -> INTEGER [#11]\n" + " c0 -> INTEGER [CSE #4]\n" + " c1 -> INTEGER [CSE #7]\n" + " 2:BIGINT [cpu time: 0ns, rows: 0] -> BIGINT [#12]\n" + " 0:BIGINT [cpu time: 0ns, rows: 0] -> BIGINT [#13]\n"); evaluate(*exprSet, data); @@ -111,22 +111,42 @@ TEST_F(ExprStatsTest, printWithStats) { ASSERT_THAT( exec::printExprWithStats(*exprSet), ::testing::MatchesRegex( - "multiply .cpu time: .+, rows: 1024. -> BIGINT\n" - " plus .cpu time: .+, rows: 1024. -> BIGINT\n" - " cast.c0 as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT\n" - " c0 .cpu time: 0ns, rows: 0. -> INTEGER\n" - " 3:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT\n" - " cast.c1 as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT\n" - " c1 .cpu time: 0ns, rows: 0. -> INTEGER\n" + "multiply .cpu time: .+, rows: 1024. -> BIGINT .#1.\n" + " plus .cpu time: .+, rows: 1024. -> BIGINT .#2.\n" + " cast.c0 as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT .#3.\n" + " c0 .cpu time: 0ns, rows: 0. -> INTEGER .#4.\n" + " 3:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#5.\n" + " cast.c1 as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT .#6.\n" + " c1 .cpu time: 0ns, rows: 0. -> INTEGER .#7.\n" "\n" - "eq .cpu time: .+, rows: 1024. -> BOOLEAN\n" - " mod .cpu time: .+, rows: 1024. -> BIGINT\n" - " cast.plus as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT\n" - " plus .cpu time: .+, rows: 1024. -> INTEGER\n" - " c0 .cpu time: 0ns, rows: 0. -> INTEGER\n" - " c1 .cpu time: 0ns, rows: 0. -> INTEGER\n" - " 2:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT\n" - " 0:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT\n")); + "eq .cpu time: .+, rows: 1024. -> BOOLEAN .#8.\n" + " mod .cpu time: .+, rows: 1024. -> BIGINT .#9.\n" + " cast.plus as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT .#10.\n" + " plus .cpu time: .+, rows: 1024. -> INTEGER .#11.\n" + " c0 -> INTEGER .CSE #4.\n" + " c1 -> INTEGER .CSE #7.\n" + " 2:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#12.\n" + " 0:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#13.\n")); + } + + // Verify that common sub-expressions are identified properly. + { + auto exprSet = + compileExpressions({"(c0 + c1) % 5", "(c0 + c1) % 3"}, rowType); + evaluate(*exprSet, data); + ASSERT_THAT( + exec::printExprWithStats(*exprSet), + ::testing::MatchesRegex( + "mod .cpu time: .+, rows: 1024. -> BIGINT .#1.\n" + " cast.plus as BIGINT. .cpu time: .+, rows: 1024. -> BIGINT .#2.\n" + " plus .cpu time: .+, rows: 1024. -> INTEGER .#3.\n" + " c0 .cpu time: 0ns, rows: 0. -> INTEGER .#4.\n" + " c1 .cpu time: 0ns, rows: 0. -> INTEGER .#5.\n" + " 5:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#6.\n" + "\n" + "mod .cpu time: .+, rows: 1024. -> BIGINT .#7.\n" + " cast..plus.c0, c1.. as BIGINT. -> BIGINT .CSE #2.\n" + " 3:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#8.\n")); } // Use dictionary encoding to repeat each row 5 times. @@ -144,22 +164,22 @@ TEST_F(ExprStatsTest, printWithStats) { ASSERT_THAT( exec::printExprWithStats(*exprSet), ::testing::MatchesRegex( - "multiply .cpu time: .+, rows: 205. -> BIGINT\n" - " plus .cpu time: .+, rows: 205. -> BIGINT\n" - " cast.c0 as BIGINT. .cpu time: .+, rows: 205. -> BIGINT\n" - " c0 .cpu time: 0ns, rows: 0. -> INTEGER\n" - " 3:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT\n" - " cast.c1 as BIGINT. .cpu time: .+, rows: 205. -> BIGINT\n" - " c1 .cpu time: 0ns, rows: 0. -> INTEGER\n" + "multiply .cpu time: .+, rows: 205. -> BIGINT .#1.\n" + " plus .cpu time: .+, rows: 205. -> BIGINT .#2.\n" + " cast.c0 as BIGINT. .cpu time: .+, rows: 205. -> BIGINT .#3.\n" + " c0 .cpu time: 0ns, rows: 0. -> INTEGER .#4.\n" + " 3:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#5.\n" + " cast.c1 as BIGINT. .cpu time: .+, rows: 205. -> BIGINT .#6.\n" + " c1 .cpu time: 0ns, rows: 0. -> INTEGER .#7.\n" "\n" - "eq .cpu time: .+, rows: 205. -> BOOLEAN\n" - " mod .cpu time: .+, rows: 205. -> BIGINT\n" - " cast.plus as BIGINT. .cpu time: .+, rows: 205. -> BIGINT\n" - " plus .cpu time: .+, rows: 205. -> INTEGER\n" - " c0 .cpu time: 0ns, rows: 0. -> INTEGER\n" - " c1 .cpu time: 0ns, rows: 0. -> INTEGER\n" - " 2:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT\n" - " 0:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT\n")); + "eq .cpu time: .+, rows: 205. -> BOOLEAN .#8.\n" + " mod .cpu time: .+, rows: 205. -> BIGINT .#9.\n" + " cast.plus as BIGINT. .cpu time: .+, rows: 205. -> BIGINT .#10.\n" + " plus .cpu time: .+, rows: 205. -> INTEGER .#11.\n" + " c0 -> INTEGER .CSE #4.\n" + " c1 -> INTEGER .CSE #7.\n" + " 2:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#12.\n" + " 0:BIGINT .cpu time: 0ns, rows: 0. -> BIGINT .#13.\n")); } }