Skip to content

Commit a9e8f25

Browse files
mbasmanovafacebook-github-bot
authored andcommitted
refactor: Reduce usage of dynamic_cast in ExprCompiler (facebookincubator#14145)
Summary: Pull Request resolved: facebookincubator#14145 ExprCompiler uses dynamic_cast to switch on expr type. This is inefficient and shows up in the profile. Introduce ExprKind enum and ITypedExpr::kind() getter to allow for more efficient switching. Also, introduce SpecialFormKind enum. Also, remove unnecessary includes. Reviewed By: xiaoxmeng Differential Revision: D78553108 fbshipit-source-id: 3469811bfc3c3f700ac5f5fd9bb71fdac5d336d3
1 parent 3da1940 commit a9e8f25

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+441
-209
lines changed

velox/benchmarks/basic/SimpleCastExpr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include <gflags/gflags.h>
2121

22+
#include "velox/core/Expressions.h"
2223
#include "velox/functions/lib/benchmarks/FunctionBenchmarkBase.h"
2324
#include "velox/vector/fuzzer/VectorFuzzer.h"
2425

velox/core/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ endif()
1818
velox_add_library(
1919
velox_core
2020
Expressions.cpp
21+
ITypedExpr.cpp
2122
FilterToExpression.cpp
2223
PlanFragment.cpp
2324
PlanNode.cpp

velox/core/Expressions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ TypePtr toRowType(
640640
ConcatTypedExpr::ConcatTypedExpr(
641641
const std::vector<std::string>& names,
642642
const std::vector<TypedExprPtr>& inputs)
643-
: ITypedExpr{toRowType(names, inputs), inputs} {}
643+
: ITypedExpr{ExprKind::kConcat, toRowType(names, inputs), inputs} {}
644644

645645
std::string ConcatTypedExpr::toString() const {
646646
std::string str{};

velox/core/Expressions.h

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ namespace facebook::velox::core {
2323

2424
class InputTypedExpr : public ITypedExpr {
2525
public:
26-
explicit InputTypedExpr(TypePtr type) : ITypedExpr{std::move(type)} {}
26+
explicit InputTypedExpr(TypePtr type)
27+
: ITypedExpr{ExprKind::kInput, std::move(type)} {}
2728

2829
bool operator==(const ITypedExpr& other) const final {
2930
const auto* casted = dynamic_cast<const InputTypedExpr*>(&other);
@@ -59,12 +60,13 @@ class ConstantTypedExpr : public ITypedExpr {
5960
// Creates constant expression. For complex types, only
6061
// Variant::null() value is supported.
6162
ConstantTypedExpr(TypePtr type, Variant value)
62-
: ITypedExpr{std::move(type)}, value_{std::move(value)} {}
63+
: ITypedExpr{ExprKind::kConstant, std::move(type)},
64+
value_{std::move(value)} {}
6365

6466
// Creates constant expression of scalar or complex type. The value comes from
6567
// index zero.
6668
explicit ConstantTypedExpr(const VectorPtr& value)
67-
: ITypedExpr{value->type()},
69+
: ITypedExpr{ExprKind::kConstant, value->type()},
6870
valueVector_{
6971
value->isConstantEncoding()
7072
? value
@@ -184,7 +186,7 @@ class CallTypedExpr : public ITypedExpr {
184186
TypePtr type,
185187
std::vector<TypedExprPtr> inputs,
186188
std::string name)
187-
: ITypedExpr{std::move(type), std::move(inputs)},
189+
: ITypedExpr{ExprKind::kCall, std::move(type), std::move(inputs)},
188190
name_(std::move(name)) {}
189191

190192
virtual const std::string& name() const {
@@ -247,14 +249,14 @@ class FieldAccessTypedExpr : public ITypedExpr {
247249
public:
248250
/// Used as a leaf in an expression tree specifying input column by name.
249251
FieldAccessTypedExpr(TypePtr type, std::string name)
250-
: ITypedExpr{std::move(type)},
252+
: ITypedExpr{ExprKind::kFieldAccess, std::move(type)},
251253
name_(std::move(name)),
252254
isInputColumn_(true) {}
253255

254256
/// Used as a dereference expression which selects a subfield in a struct by
255257
/// name.
256258
FieldAccessTypedExpr(TypePtr type, TypedExprPtr input, std::string name)
257-
: ITypedExpr{std::move(type), {std::move(input)}},
259+
: ITypedExpr{ExprKind::kFieldAccess, std::move(type), {std::move(input)}},
258260
name_(std::move(name)),
259261
isInputColumn_(dynamic_cast<const InputTypedExpr*>(inputs()[0].get())) {
260262
}
@@ -323,7 +325,8 @@ using FieldAccessTypedExprPtr = std::shared_ptr<const FieldAccessTypedExpr>;
323325
class DereferenceTypedExpr : public ITypedExpr {
324326
public:
325327
DereferenceTypedExpr(TypePtr type, TypedExprPtr input, uint32_t index)
326-
: ITypedExpr{std::move(type), {std::move(input)}}, index_(index) {
328+
: ITypedExpr{ExprKind::kDereference, std::move(type), {std::move(input)}},
329+
index_(index) {
327330
// Make sure this isn't being used to access a top level column.
328331
VELOX_USER_CHECK_NULL(
329332
std::dynamic_pointer_cast<const InputTypedExpr>(inputs()[0]));
@@ -445,9 +448,11 @@ using ConcatTypedExprPtr = std::shared_ptr<const ConcatTypedExpr>;
445448
class LambdaTypedExpr : public ITypedExpr {
446449
public:
447450
LambdaTypedExpr(RowTypePtr signature, TypedExprPtr body)
448-
: ITypedExpr(std::make_shared<FunctionType>(
449-
std::vector<TypePtr>(signature->children()),
450-
body->type())),
451+
: ITypedExpr(
452+
ExprKind::kLambda,
453+
std::make_shared<FunctionType>(
454+
std::vector<TypePtr>(signature->children()),
455+
body->type())),
451456
signature_(std::move(signature)),
452457
body_(std::move(body)) {}
453458

@@ -512,13 +517,13 @@ class CastTypedExpr : public ITypedExpr {
512517
/// and expected to be different from to-type.
513518
/// @param isTryCast Whether this expression is used for `try_cast`.
514519
CastTypedExpr(const TypePtr& type, const TypedExprPtr& input, bool isTryCast)
515-
: ITypedExpr{type, {input}}, isTryCast_(isTryCast) {}
520+
: ITypedExpr{ExprKind::kCast, type, {input}}, isTryCast_(isTryCast) {}
516521

517522
CastTypedExpr(
518523
const TypePtr& type,
519524
const std::vector<TypedExprPtr>& inputs,
520525
bool isTryCast)
521-
: ITypedExpr{type, inputs}, isTryCast_(isTryCast) {
526+
: ITypedExpr{ExprKind::kCast, type, inputs}, isTryCast_(isTryCast) {
522527
VELOX_USER_CHECK_EQ(
523528
1, inputs.size(), "Cast expression requires exactly one input");
524529
}

velox/core/ITypedExpr.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (c) Facebook, Inc. and its affiliates.
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 "velox/core/ITypedExpr.h"
18+
19+
namespace facebook::velox::core {
20+
21+
namespace {
22+
folly::F14FastMap<ExprKind, std::string> exprKindNames() {
23+
static const folly::F14FastMap<ExprKind, std::string> kNames = {
24+
{ExprKind::kInput, "INPUT"},
25+
{ExprKind::kFieldAccess, "FIELD"},
26+
{ExprKind::kDereference, "DEREFERENCE"},
27+
{ExprKind::kCall, "CALL"},
28+
{ExprKind::kCast, "CAST"},
29+
{ExprKind::kConstant, "CONSTANT"},
30+
{ExprKind::kConcat, "CONCAT"},
31+
{ExprKind::kLambda, "LAMBDA"},
32+
};
33+
34+
return kNames;
35+
}
36+
} // namespace
37+
38+
VELOX_DEFINE_ENUM_NAME(ExprKind, exprKindNames);
39+
} // namespace facebook::velox::core

velox/core/ITypedExpr.h

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,19 @@
1919

2020
namespace facebook::velox::core {
2121

22+
enum class ExprKind : int32_t {
23+
kInput = 0,
24+
kFieldAccess = 1,
25+
kDereference = 2,
26+
kCall = 3,
27+
kCast = 5,
28+
kConstant = 6,
29+
kConcat = 7,
30+
kLambda = 8,
31+
};
32+
33+
VELOX_DECLARE_ENUM_NAME(ExprKind);
34+
2235
class ITypedExpr;
2336
class ITypedExprVisitor;
2437
class ITypedExprVisitorContext;
@@ -28,21 +41,63 @@ using TypedExprPtr = std::shared_ptr<const ITypedExpr>;
2841
/// Strongly-typed expression, e.g. literal, function call, etc.
2942
class ITypedExpr : public ISerializable {
3043
public:
31-
explicit ITypedExpr(TypePtr type) : type_{std::move(type)}, inputs_{} {}
44+
ITypedExpr(ExprKind kind, TypePtr type)
45+
: kind_{kind}, type_{std::move(type)}, inputs_{} {}
46+
47+
ITypedExpr(ExprKind kind, TypePtr type, std::vector<TypedExprPtr> inputs)
48+
: kind_{kind}, type_{std::move(type)}, inputs_{std::move(inputs)} {}
3249

33-
ITypedExpr(TypePtr type, std::vector<TypedExprPtr> inputs)
34-
: type_{std::move(type)}, inputs_{std::move(inputs)} {}
50+
virtual ~ITypedExpr() = default;
51+
52+
ExprKind kind() const {
53+
return kind_;
54+
}
3555

3656
const TypePtr& type() const {
3757
return type_;
3858
}
3959

40-
virtual ~ITypedExpr() = default;
41-
4260
const std::vector<TypedExprPtr>& inputs() const {
4361
return inputs_;
4462
}
4563

64+
bool isInputKind() const {
65+
return kind_ == ExprKind::kInput;
66+
}
67+
68+
bool isFieldAccessKind() const {
69+
return kind_ == ExprKind::kFieldAccess;
70+
}
71+
72+
bool isDereferenceKind() const {
73+
return kind_ == ExprKind::kDereference;
74+
}
75+
76+
bool isCallKind() const {
77+
return kind_ == ExprKind::kCall;
78+
}
79+
80+
bool isCastKind() const {
81+
return kind_ == ExprKind::kCast;
82+
}
83+
84+
bool isConstantKind() const {
85+
return kind_ == ExprKind::kConstant;
86+
}
87+
88+
bool isConcatKind() const {
89+
return kind_ == ExprKind::kConcat;
90+
}
91+
92+
bool isLambdaKind() const {
93+
return kind_ == ExprKind::kLambda;
94+
}
95+
96+
template <typename T>
97+
const T* asUnchecked() const {
98+
return dynamic_cast<const T*>(this);
99+
}
100+
46101
/// Returns a copy of this expression with input fields replaced according
47102
/// to specified 'mapping'. Fields specified in the 'mapping' are replaced
48103
/// by the corresponding expression in 'mapping'.
@@ -88,8 +143,9 @@ class ITypedExpr : public ISerializable {
88143
}
89144

90145
private:
91-
TypePtr type_;
92-
std::vector<TypedExprPtr> inputs_;
146+
const ExprKind kind_;
147+
const TypePtr type_;
148+
const std::vector<TypedExprPtr> inputs_;
93149
};
94150

95151
} // namespace facebook::velox::core

velox/examples/ExpressionEval.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include "velox/common/memory/Memory.h"
18+
#include "velox/core/Expressions.h"
1819
#include "velox/functions/Udf.h"
1920
#include "velox/type/Type.h"
2021
#include "velox/vector/BaseVector.h"

velox/examples/OpaqueType.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include "velox/common/memory/Memory.h"
18+
#include "velox/core/Expressions.h"
1819
#include "velox/expression/VectorFunction.h"
1920
#include "velox/functions/Udf.h"
2021
#include "velox/type/Type.h"

velox/expression/CastExpr.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class CastExpr : public SpecialForm {
9090
bool isTryCast,
9191
std::shared_ptr<CastHooks> hooks)
9292
: SpecialForm(
93+
SpecialFormKind::kCast,
9394
type,
9495
std::vector<ExprPtr>({expr}),
9596
isTryCast ? kTryCast.data() : kCast.data(),

velox/expression/CoalesceExpr.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ CoalesceExpr::CoalesceExpr(
2222
std::vector<ExprPtr>&& inputs,
2323
bool inputsSupportFlatNoNullsFastPath)
2424
: SpecialForm(
25+
SpecialFormKind::kCoalesce,
2526
std::move(type),
2627
std::move(inputs),
2728
kCoalesce,
@@ -39,7 +40,7 @@ CoalesceExpr::CoalesceExpr(
3940
auto expectedType = resolveType(inputTypes);
4041
VELOX_CHECK(
4142
*expectedType == *this->type(),
42-
"Coalesce expression type different than its inputs. Expected {} but got Actual {}.",
43+
"Coalesce expression type different than its inputs. Expected {}, but got {}.",
4344
expectedType->toString(),
4445
this->type()->toString());
4546
}

0 commit comments

Comments
 (0)