Skip to content

Commit f8738db

Browse files
pravindrawesm
pravindra
authored and
wesm
committed
[Gandiva] Reduce bitmap updates for if-else
In case of nested if-else conditions, eg. if A else if B else if C else D The else parts of A & C will not update validity bitmaps. Only the if parts and the terminal else update bitmaps.
1 parent 8b0dfe0 commit f8738db

8 files changed

Lines changed: 372 additions & 21 deletions

File tree

‎cpp/src/gandiva/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,5 @@ add_gandiva_unit_test(llvm_types_test.cc llvm_types.cc)
9393
add_gandiva_unit_test(llvm_generator_test.cc llvm_generator.cc engine.cc llvm_types.cc expr_decomposer.cc function_registry.cc annotator.cc status.cc ${BC_FILE_PATH_CC})
9494
add_gandiva_unit_test(annotator_test.cc annotator.cc)
9595
add_gandiva_unit_test(tree_expr_test.cc tree_expr_builder.cc expr_decomposer.cc annotator.cc function_registry.cc)
96+
add_gandiva_unit_test(expr_decomposer_test.cc expr_decomposer.cc tree_expr_builder.cc annotator.cc function_registry.cc)
9697
add_gandiva_unit_test(status_test.cc status.cc)

‎cpp/src/gandiva/dex.h‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,12 +214,14 @@ class IfDex : public Dex {
214214
ValueValidityPairPtr then_vv,
215215
ValueValidityPairPtr else_vv,
216216
DataTypePtr result_type,
217-
int local_bitmap_idx)
217+
int local_bitmap_idx,
218+
bool is_terminal_else)
218219
: condition_vv_(condition_vv),
219220
then_vv_(then_vv),
220221
else_vv_(else_vv),
221222
result_type_(result_type),
222-
local_bitmap_idx_(local_bitmap_idx) {}
223+
local_bitmap_idx_(local_bitmap_idx),
224+
is_terminal_else_(is_terminal_else) {}
223225

224226
void Accept(DexVisitor &visitor) override {
225227
visitor.Visit(*this);
@@ -232,6 +234,9 @@ class IfDex : public Dex {
232234
// The validity of the result is saved in this bitmap.
233235
int local_bitmap_idx() const { return local_bitmap_idx_; }
234236

237+
// is this a terminal else ? i.e no nested if-else underneath.
238+
bool is_terminal_else() const { return is_terminal_else_; }
239+
235240
const DataTypePtr &result_type() const { return result_type_; }
236241

237242
private:
@@ -240,6 +245,7 @@ class IfDex : public Dex {
240245
ValueValidityPairPtr else_vv_;
241246
DataTypePtr result_type_;
242247
int local_bitmap_idx_;
248+
bool is_terminal_else_;
243249
};
244250

245251
} // namespace gandiva

‎cpp/src/gandiva/dex_llvm_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ TEST_F(TestDex, TestVisitor) {
115115
nullable_internal_func.Accept(visitor);
116116
EXPECT_EQ(desc, name_map_[&typeid(NullableInternalFuncDex)]);
117117

118-
IfDex if_dex(nullptr, nullptr, nullptr, arrow::int32(), 0);
118+
IfDex if_dex(nullptr, nullptr, nullptr, arrow::int32(), 0, false);
119119
if_dex.Accept(visitor);
120120
EXPECT_EQ(desc, name_map_[&typeid(IfDex)]);
121121
}

‎cpp/src/gandiva/expr_decomposer.cc‎

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "codegen/expr_decomposer.h"
1818

1919
#include <memory>
20+
#include <stack>
2021
#include <string>
2122
#include <vector>
2223
#include "codegen/dex.h"
@@ -89,23 +90,26 @@ void ExprDecomposer::Visit(const FunctionNode &node) {
8990
// Decompose an IfNode
9091
void ExprDecomposer::Visit(const IfNode &node) {
9192
// Add a local bitmap to track the output validity.
92-
int local_bitmap_idx = annotator_.AddLocalBitMap();
93-
auto validity_dex = std::make_shared<LocalBitMapValidityDex>(local_bitmap_idx);
94-
9593
node.condition()->Accept(*this);
9694
auto condition_vv = result();
9795

96+
int local_bitmap_idx = PushThenEntry(node);
9897
node.then_node()->Accept(*this);
9998
auto then_vv = result();
99+
PopThenEntry(node);
100100

101+
PushElseEntry(node, local_bitmap_idx);
101102
node.else_node()->Accept(*this);
102103
auto else_vv = result();
104+
bool is_terminal_else = PopElseEntry(node);
103105

106+
auto validity_dex = std::make_shared<LocalBitMapValidityDex>(local_bitmap_idx);
104107
auto value_dex = std::make_shared<IfDex>(condition_vv,
105108
then_vv,
106109
else_vv,
107110
node.return_type(),
108-
local_bitmap_idx);
111+
local_bitmap_idx,
112+
is_terminal_else);
109113

110114
result_ = std::make_shared<ValueValidityPair>(validity_dex, value_dex);
111115
}
@@ -115,4 +119,67 @@ void ExprDecomposer::Visit(const LiteralNode &node) {
115119
result_ = std::make_shared<ValueValidityPair>(value_dex);
116120
}
117121

122+
// The bolow functions use a stack to detect :
123+
// a. nested if-else expressions.
124+
// In such cases, the local bitmap can be re-used.
125+
// b. detect terminal else expressions
126+
// The non-terminal else expressions do not need to track validity (the if statement
127+
// that has a match will do it).
128+
// Both of the above optimisations save CPU cycles during expression evaluation.
129+
130+
int ExprDecomposer::PushThenEntry(const IfNode &node) {
131+
int local_bitmap_idx;
132+
133+
if (!if_entries_stack_.empty() && !if_entries_stack_.top()->is_then_) {
134+
auto top = if_entries_stack_.top().get();
135+
136+
// inside a nested else statement (i.e if-else-if). use the parent's bitmap.
137+
local_bitmap_idx = top->local_bitmap_idx_;
138+
139+
// clear the is_terminal bit in the current top entry (else).
140+
top->is_terminal_else_ = false;
141+
} else {
142+
// alloc a new bitmap.
143+
local_bitmap_idx = annotator_.AddLocalBitMap();
144+
}
145+
146+
// push new entry to the stack.
147+
std::unique_ptr<IfStackEntry> entry(new IfStackEntry(node,
148+
true /*is_then*/,
149+
false /*is_terminal_else*/,
150+
local_bitmap_idx));
151+
if_entries_stack_.push(std::move(entry));
152+
return local_bitmap_idx;
153+
}
154+
155+
void ExprDecomposer::PopThenEntry(const IfNode &node) {
156+
DCHECK_EQ(if_entries_stack_.empty(), false) << "PopThenEntry: found empty stack";
157+
158+
auto top = if_entries_stack_.top().get();
159+
DCHECK_EQ(top->is_then_, true) << "PopThenEntry: found else, expected then";
160+
DCHECK_EQ(&top->if_node_, &node) << "PopThenEntry: found mismatched node";
161+
162+
if_entries_stack_.pop();
163+
}
164+
165+
void ExprDecomposer::PushElseEntry(const IfNode &node, int local_bitmap_idx) {
166+
std::unique_ptr<IfStackEntry> entry(new IfStackEntry(node,
167+
false /*is_then*/,
168+
true /*is_terminal_else*/,
169+
local_bitmap_idx));
170+
if_entries_stack_.push(std::move(entry));
171+
}
172+
173+
bool ExprDecomposer::PopElseEntry(const IfNode &node) {
174+
DCHECK_EQ(if_entries_stack_.empty(), false) << "PopElseEntry: found empty stack";
175+
176+
auto top = if_entries_stack_.top().get();
177+
DCHECK_EQ(top->is_then_, false) << "PopElseEntry: found then, expected else";
178+
DCHECK_EQ(&top->if_node_, &node) << "PopThenEntry: found mismatched node";
179+
bool is_terminal_else = top->is_terminal_else_;
180+
181+
if_entries_stack_.pop();
182+
return is_terminal_else;
183+
}
184+
118185
} // namespace gandiva

‎cpp/src/gandiva/expr_decomposer.h‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616
#ifndef GANDIVA_EXPR_DECOMPOSER_H
1717
#define GANDIVA_EXPR_DECOMPOSER_H
1818

19+
#include <stack>
20+
#include <memory>
1921
#include <utility>
22+
#include <gtest/gtest_prod.h>
2023

2124
#include "gandiva/expression.h"
2225
#include "codegen/node_visitor.h"
@@ -42,15 +45,53 @@ class ExprDecomposer : public NodeVisitor {
4245
}
4346

4447
private:
48+
FRIEND_TEST(TestExprDecomposer, TestStackSimple);
49+
FRIEND_TEST(TestExprDecomposer, TestNested);
50+
FRIEND_TEST(TestExprDecomposer, TestInternalIf);
51+
FRIEND_TEST(TestExprDecomposer, TestParallelIf);
52+
4553
void Visit(const FieldNode &node) override;
4654
void Visit(const FunctionNode &node) override;
4755
void Visit(const IfNode &node) override;
4856
void Visit(const LiteralNode &node) override;
4957

58+
// stack of if nodes.
59+
class IfStackEntry {
60+
public:
61+
IfStackEntry(const IfNode &if_node,
62+
bool is_then,
63+
bool is_terminal_else,
64+
int local_bitmap_idx)
65+
: if_node_(if_node),
66+
is_then_(is_then),
67+
is_terminal_else_(is_terminal_else),
68+
local_bitmap_idx_(local_bitmap_idx) {}
69+
70+
const IfNode &if_node_;
71+
bool is_then_;
72+
bool is_terminal_else_;
73+
int local_bitmap_idx_;
74+
};
75+
76+
// push 'then entry' to stack. returns either a new local bitmap or the parent's
77+
// bitmap (in case of nested if-else).
78+
int PushThenEntry(const IfNode &node);
79+
80+
// pop 'then entry' from stack.
81+
void PopThenEntry(const IfNode &node);
82+
83+
// push 'else entry' into stack.
84+
void PushElseEntry(const IfNode &node, int local_bmap_idx);
85+
86+
// pop 'else entry' from stack. returns 'true' if this is a terminal else condition
87+
// i.e no nested if condition below this node.
88+
bool PopElseEntry(const IfNode &node);
89+
5090
ValueValidityPairPtr result() { return std::move(result_); }
5191

5292
const FunctionRegistry &registry_;
5393
Annotator &annotator_;
94+
std::stack<std::unique_ptr<IfStackEntry>> if_entries_stack_;
5495
ValueValidityPairPtr result_;
5596
};
5697

‎cpp/src/gandiva/expr_decomposer_test.cc‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/*
2+
* Copyright (C) 2017-2018 Dremio Corporation
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <gtest/gtest.h>
18+
#include "gandiva/gandiva_aliases.h"
19+
#include "gandiva/tree_expr_builder.h"
20+
#include "codegen/annotator.h"
21+
#include "codegen/dex.h"
22+
#include "codegen/expr_decomposer.h"
23+
#include "codegen/function_signature.h"
24+
#include "codegen/function_registry.h"
25+
#include "codegen/node.h"
26+
27+
namespace gandiva {
28+
29+
using arrow::int32;
30+
31+
class TestExprDecomposer : public ::testing::Test {
32+
protected:
33+
FunctionRegistry registry_;
34+
};
35+
36+
TEST_F(TestExprDecomposer, TestStackSimple) {
37+
Annotator annotator;
38+
ExprDecomposer decomposer(registry_, annotator);
39+
40+
// if (a) _
41+
// else _
42+
IfNode node_a(nullptr, nullptr, nullptr, int32());
43+
44+
int idx_a = decomposer.PushThenEntry(node_a);
45+
EXPECT_EQ(idx_a, 0);
46+
decomposer.PopThenEntry(node_a);
47+
48+
decomposer.PushElseEntry(node_a, idx_a);
49+
bool is_terminal_a = decomposer.PopElseEntry(node_a);
50+
EXPECT_EQ(is_terminal_a, true);
51+
EXPECT_EQ(decomposer.if_entries_stack_.empty(), true);
52+
}
53+
54+
TEST_F(TestExprDecomposer, TestNested) {
55+
Annotator annotator;
56+
ExprDecomposer decomposer(registry_, annotator);
57+
58+
// if (a) _
59+
// else _
60+
// if (b) _
61+
// else _
62+
IfNode node_a(nullptr, nullptr, nullptr, int32());
63+
IfNode node_b(nullptr, nullptr, nullptr, int32());
64+
65+
int idx_a = decomposer.PushThenEntry(node_a);
66+
EXPECT_EQ(idx_a, 0);
67+
decomposer.PopThenEntry(node_a);
68+
69+
decomposer.PushElseEntry(node_a, idx_a);
70+
71+
{ // start b
72+
int idx_b = decomposer.PushThenEntry(node_b);
73+
EXPECT_EQ(idx_b, 0); // must reuse bitmap.
74+
decomposer.PopThenEntry(node_b);
75+
76+
decomposer.PushElseEntry(node_b, idx_b);
77+
bool is_terminal_b = decomposer.PopElseEntry(node_b);
78+
EXPECT_EQ(is_terminal_b, true);
79+
} // end b
80+
81+
bool is_terminal_a = decomposer.PopElseEntry(node_a);
82+
EXPECT_EQ(is_terminal_a, false); // there was a nested if.
83+
84+
EXPECT_EQ(decomposer.if_entries_stack_.empty(), true);
85+
}
86+
87+
TEST_F(TestExprDecomposer, TestInternalIf) {
88+
Annotator annotator;
89+
ExprDecomposer decomposer(registry_, annotator);
90+
91+
// if (a) _
92+
// if (b) _
93+
// else _
94+
// else _
95+
IfNode node_a(nullptr, nullptr, nullptr, int32());
96+
IfNode node_b(nullptr, nullptr, nullptr, int32());
97+
98+
int idx_a = decomposer.PushThenEntry(node_a);
99+
EXPECT_EQ(idx_a, 0);
100+
101+
{ // start b
102+
int idx_b = decomposer.PushThenEntry(node_b);
103+
EXPECT_EQ(idx_b, 1); // must not reuse bitmap.
104+
decomposer.PopThenEntry(node_b);
105+
106+
decomposer.PushElseEntry(node_b, idx_b);
107+
bool is_terminal_b = decomposer.PopElseEntry(node_b);
108+
EXPECT_EQ(is_terminal_b, true);
109+
} // end b
110+
111+
decomposer.PopThenEntry(node_a);
112+
decomposer.PushElseEntry(node_a, idx_a);
113+
114+
bool is_terminal_a = decomposer.PopElseEntry(node_a);
115+
EXPECT_EQ(is_terminal_a, true); // there was no nested if.
116+
117+
EXPECT_EQ(decomposer.if_entries_stack_.empty(), true);
118+
}
119+
120+
TEST_F(TestExprDecomposer, TestParallelIf) {
121+
Annotator annotator;
122+
ExprDecomposer decomposer(registry_, annotator);
123+
124+
// if (a) _
125+
// else _
126+
// if (b) _
127+
// else _
128+
IfNode node_a(nullptr, nullptr, nullptr, int32());
129+
IfNode node_b(nullptr, nullptr, nullptr, int32());
130+
131+
int idx_a = decomposer.PushThenEntry(node_a);
132+
EXPECT_EQ(idx_a, 0);
133+
134+
decomposer.PopThenEntry(node_a);
135+
decomposer.PushElseEntry(node_a, idx_a);
136+
137+
bool is_terminal_a = decomposer.PopElseEntry(node_a);
138+
EXPECT_EQ(is_terminal_a, true); // there was no nested if.
139+
140+
// start b
141+
int idx_b = decomposer.PushThenEntry(node_b);
142+
EXPECT_EQ(idx_b, 1); // must not reuse bitmap.
143+
decomposer.PopThenEntry(node_b);
144+
145+
decomposer.PushElseEntry(node_b, idx_b);
146+
bool is_terminal_b = decomposer.PopElseEntry(node_b);
147+
EXPECT_EQ(is_terminal_b, true);
148+
149+
EXPECT_EQ(decomposer.if_entries_stack_.empty(), true);
150+
}
151+
152+
int main(int argc, char **argv) {
153+
::testing::InitGoogleTest(&argc, argv);
154+
return RUN_ALL_TESTS();
155+
}
156+
157+
} // namespace gandiva

0 commit comments

Comments
 (0)