Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tidy: enable performance checks #2505

Merged
merged 1 commit into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ Checks:

modernize-use-override,

performance-*,
-performance-no-int-to-ptr,

HeaderFilterRegex:
src/include|tools/benchmark/include|tools/nodejs_api/src_cpp/include|tools/python_api/src_cpp/include|tools/rust_api/include|tools/shell/include
WarningsAsErrors: "*"
8 changes: 6 additions & 2 deletions src/binder/bind/bind_ddl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace kuzu {
namespace binder {

std::vector<std::unique_ptr<Property>> Binder::bindProperties(
std::vector<std::pair<std::string, std::string>> propertyNameDataTypes) {
const std::vector<std::pair<std::string, std::string>>& propertyNameDataTypes) {
std::vector<std::unique_ptr<Property>> boundPropertyNameDataTypes;
std::unordered_set<std::string> boundPropertyNames;
boundPropertyNames.reserve(propertyNameDataTypes.size());
Expand Down Expand Up @@ -135,7 +135,11 @@ std::unique_ptr<BoundCreateTableInfo> Binder::bindCreateRelTableGroupInfo(
auto relMultiplicity = extraInfo->relMultiplicity;
std::vector<std::unique_ptr<BoundCreateTableInfo>> boundCreateRelTableInfos;
for (auto& [srcTableName, dstTableName] : extraInfo->srcDstTablePairs) {
auto relTableName = relGroupName + "_" + srcTableName + "_" + dstTableName;
auto relTableName = std::string(relGroupName)
.append("_")
.append(srcTableName)
.append("_")
.append(dstTableName);
auto relExtraInfo =
std::make_unique<ExtraCreateRelTableInfo>(relMultiplicity, srcTableName, dstTableName);
auto relCreateInfo = std::make_unique<CreateTableInfo>(
Expand Down
4 changes: 2 additions & 2 deletions src/binder/bind/bind_reading_clause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ std::unique_ptr<BoundReadingClause> Binder::bindInQueryCall(const ReadingClause&
}
// TODO: this is dangerous because we could match to a scan function.
auto tableFunction = reinterpret_cast<function::TableFunction*>(
catalog.getBuiltInFunctions()->matchScalarFunction(std::move(funcName), inputTypes));
catalog.getBuiltInFunctions()->matchScalarFunction(funcName, inputTypes));
auto bindInput = std::make_unique<function::TableFuncBindInput>();
bindInput->inputs = std::move(inputValues);
auto bindData =
Expand All @@ -136,7 +136,7 @@ std::unique_ptr<BoundReadingClause> Binder::bindInQueryCall(const ReadingClause&
auto offset = expressionBinder.createVariableExpression(
*LogicalType::INT64(), std::string(InternalKeyword::ROW_OFFSET));
auto boundInQueryCall = std::make_unique<BoundInQueryCall>(
std::move(tableFunction), std::move(bindData), std::move(columns), offset);
tableFunction, std::move(bindData), std::move(columns), offset);
if (call.hasWherePredicate()) {
auto wherePredicate = expressionBinder.bindExpression(*call.getWherePredicate());
boundInQueryCall->setWherePredicate(std::move(wherePredicate));
Expand Down
1 change: 1 addition & 0 deletions src/binder/bind/copy/bind_copy_rdf_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ std::unique_ptr<BoundStatement> Binder::bindCopyRdfRelFrom(const Statement& /*st
columnNames.emplace_back(rdf::PID);
columnNames.emplace_back(InternalKeyword::DST_OFFSET);
std::vector<std::unique_ptr<common::LogicalType>> columnTypes;
columnTypes.reserve(3);
for (auto i = 0u; i < 3; ++i) {
columnTypes.push_back(LogicalType::INT64());
}
Expand Down
2 changes: 1 addition & 1 deletion src/binder/bind_expression/bind_comparison_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ std::shared_ptr<Expression> ExpressionBinder::bindComparisonExpression(
auto child = bindExpression(*parsedExpression.getChild(i));
children.push_back(std::move(child));
}
return bindComparisonExpression(parsedExpression.getExpressionType(), std::move(children));
return bindComparisonExpression(parsedExpression.getExpressionType(), children);
}

std::shared_ptr<Expression> ExpressionBinder::bindComparisonExpression(
Expand Down
2 changes: 1 addition & 1 deletion src/binder/bind_expression/bind_function_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ std::unique_ptr<Expression> ExpressionBinder::createInternalNodeIDExpression(
}

std::shared_ptr<Expression> ExpressionBinder::bindInternalIDExpression(
std::shared_ptr<Expression> expression) {
const std::shared_ptr<Expression>& expression) {
if (ExpressionUtil::isNodePattern(*expression)) {
auto& node = (NodeExpression&)*expression;
return node.getInternalID();
Expand Down
2 changes: 1 addition & 1 deletion src/binder/bind_expression/bind_property_expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ expression_vector ExpressionBinder::bindNodeOrRelPropertyStarExpression(const Ex
}

expression_vector ExpressionBinder::bindStructPropertyStarExpression(
std::shared_ptr<Expression> child) {
const std::shared_ptr<Expression>& child) {
expression_vector result;
auto childType = child->getDataType();
for (auto field : StructType::getFields(&childType)) {
Expand Down
9 changes: 3 additions & 6 deletions src/binder/binder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,17 +220,14 @@ function::TableFunction* Binder::getScanFunction(common::FileType fileType, bool
catalog.getBuiltInFunctions()->matchScalarFunction(READ_PARQUET_FUNC_NAME, inputTypes);
} break;
case common::FileType::NPY: {
func = catalog.getBuiltInFunctions()->matchScalarFunction(
READ_NPY_FUNC_NAME, std::move(inputTypes));
func = catalog.getBuiltInFunctions()->matchScalarFunction(READ_NPY_FUNC_NAME, inputTypes);
} break;
case common::FileType::CSV: {
func = catalog.getBuiltInFunctions()->matchScalarFunction(
isParallel ? READ_CSV_PARALLEL_FUNC_NAME : READ_CSV_SERIAL_FUNC_NAME,
std::move(inputTypes));
isParallel ? READ_CSV_PARALLEL_FUNC_NAME : READ_CSV_SERIAL_FUNC_NAME, inputTypes);
} break;
case common::FileType::TURTLE: {
func = catalog.getBuiltInFunctions()->matchScalarFunction(
READ_RDF_FUNC_NAME, std::move(inputTypes));
func = catalog.getBuiltInFunctions()->matchScalarFunction(READ_RDF_FUNC_NAME, inputTypes);
} break;
default:
KU_UNREACHABLE;
Expand Down
2 changes: 1 addition & 1 deletion src/binder/bound_statement_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace kuzu {
namespace binder {

std::unique_ptr<BoundStatementResult> BoundStatementResult::createSingleStringColumnResult(
std::string columnName) {
const std::string& columnName) {
auto result = std::make_unique<BoundStatementResult>();
auto value = std::make_unique<Value>(LogicalType{LogicalTypeID::STRING}, columnName);
auto stringColumn = std::make_shared<LiteralExpression>(std::move(value), columnName);
Expand Down
2 changes: 1 addition & 1 deletion src/binder/expression_binder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ std::shared_ptr<Expression> ExpressionBinder::bindExpression(
}

std::shared_ptr<Expression> ExpressionBinder::foldExpression(
std::shared_ptr<Expression> expression) {
const std::shared_ptr<Expression>& expression) {
auto value = evaluator::ExpressionEvaluatorUtils::evaluateConstantExpression(
expression, binder->memoryManager);
auto result = createLiteralExpression(std::move(value));
Expand Down
14 changes: 7 additions & 7 deletions src/binder/query/query_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,13 @@ void QueryGraph::addQueryNode(std::shared_ptr<NodeExpression> queryNode) {
return;
}
queryNodeNameToPosMap.insert({queryNode->getUniqueName(), queryNodes.size()});
queryNodes.push_back(queryNode);
queryNodes.push_back(std::move(queryNode));
}

void QueryGraph::addQueryRel(std::shared_ptr<RelExpression> queryRel) {
KU_ASSERT(!containsQueryRel(queryRel->getUniqueName()));
queryRelNameToPosMap.insert({queryRel->getUniqueName(), queryRels.size()});
queryRels.push_back(queryRel);
queryRels.push_back(std::move(queryRel));
}

void QueryGraph::merge(const QueryGraph& other) {
Expand Down Expand Up @@ -230,8 +230,8 @@ std::unique_ptr<QueryGraphCollection> QueryGraphCollection::copy() const {
return result;
}

void PropertyKeyValCollection::addKeyVal(
std::shared_ptr<Expression> variable, const std::string& propertyName, expression_pair keyVal) {
void PropertyKeyValCollection::addKeyVal(const std::shared_ptr<Expression>& variable,
const std::string& propertyName, expression_pair keyVal) {
if (!propertyKeyValMap.contains(variable)) {
propertyKeyValMap.insert({variable, std::unordered_map<std::string, expression_pair>{}});
}
Expand All @@ -249,7 +249,7 @@ std::vector<expression_pair> PropertyKeyValCollection::getKeyVals() const {
}

std::vector<expression_pair> PropertyKeyValCollection::getKeyVals(
std::shared_ptr<Expression> variable) const {
const std::shared_ptr<Expression>& variable) const {
std::vector<expression_pair> result;
if (!propertyKeyValMap.contains(variable)) {
return result;
Expand All @@ -261,7 +261,7 @@ std::vector<expression_pair> PropertyKeyValCollection::getKeyVals(
}

bool PropertyKeyValCollection::hasKeyVal(
std::shared_ptr<Expression> variable, const std::string& propertyName) const {
const std::shared_ptr<Expression>& variable, const std::string& propertyName) const {
if (!propertyKeyValMap.contains(variable)) {
return false;
}
Expand All @@ -272,7 +272,7 @@ bool PropertyKeyValCollection::hasKeyVal(
}

expression_pair PropertyKeyValCollection::getKeyVal(
std::shared_ptr<Expression> variable, const std::string& propertyName) const {
const std::shared_ptr<Expression>& variable, const std::string& propertyName) const {
KU_ASSERT(hasKeyVal(variable, propertyName));
return propertyKeyValMap.at(variable).at(propertyName);
}
Expand Down
1 change: 1 addition & 0 deletions src/catalog/catalog_content.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ table_id_t CatalogContent::addRdfGraphSchema(const BoundCreateTableInfo& info) {

std::vector<TableSchema*> CatalogContent::getTableSchemas() const {
std::vector<TableSchema*> result;
result.reserve(tableSchemas.size());
for (auto&& [_, schema] : tableSchemas) {
result.push_back(schema.get());
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/types/int128_t.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ std::string Int128_t::ToString(int128_t input) {
break;
}
input = divModPositive(input, 10, remainder);
result = std::string(1, '0' + remainder) + result;
result = std::string(1, '0' + remainder) + std::move(result);
}
if (result.empty()) {
result = "0";
Expand Down
12 changes: 1 addition & 11 deletions src/common/types/types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,6 @@ LogicalType::LogicalType(const LogicalType& other) {
}
}

LogicalType::LogicalType(LogicalType&& other) noexcept
: typeID{other.typeID}, physicalType{other.physicalType}, extraTypeInfo{
std::move(other.extraTypeInfo)} {}

LogicalType& LogicalType::operator=(const LogicalType& other) {
typeID = other.typeID;
physicalType = other.physicalType;
Expand Down Expand Up @@ -310,13 +306,6 @@ bool LogicalType::operator!=(const LogicalType& other) const {
return !((*this) == other);
}

LogicalType& LogicalType::operator=(LogicalType&& other) noexcept {
typeID = other.typeID;
physicalType = other.physicalType;
extraTypeInfo = std::move(other.extraTypeInfo);
return *this;
}

std::string LogicalType::toString() const {
switch (typeID) {
case LogicalTypeID::MAP: {
Expand Down Expand Up @@ -660,6 +649,7 @@ std::string LogicalTypeUtils::toString(LogicalTypeID dataTypeID) {

std::string LogicalTypeUtils::toString(const std::vector<LogicalType*>& dataTypes) {
std::vector<LogicalTypeID> dataTypeIDs;
dataTypeIDs.reserve(dataTypes.size());
for (auto& dataType : dataTypes) {
dataTypeIDs.push_back(dataType->typeID);
}
Expand Down
19 changes: 12 additions & 7 deletions src/common/types/value/value.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "common/types/value/value.h"

#include <utility>

#include "common/null_buffer.h"
#include "common/serializer/deserializer.h"
#include "common/serializer/serializer.h"
Expand Down Expand Up @@ -40,8 +42,8 @@ Value Value::createNullValue() {
return {};
}

Value Value::createNullValue(LogicalType dataType) {
return Value(std::move(dataType));
Value Value::createNullValue(const LogicalType& dataType) {
return Value(dataType);
}

Value Value::createDefaultValue(const LogicalType& dataType) {
Expand Down Expand Up @@ -86,7 +88,9 @@ Value Value::createDefaultValue(const LogicalType& dataType) {
case LogicalTypeID::FIXED_LIST: {
std::vector<std::unique_ptr<Value>> children;
auto childType = FixedListType::getChildType(&dataType);
for (auto i = 0u; i < FixedListType::getNumValuesInList(&dataType); ++i) {
auto listSize = FixedListType::getNumValuesInList(&dataType);
children.reserve(listSize);
for (auto i = 0u; i < listSize; ++i) {
children.push_back(std::make_unique<Value>(createDefaultValue(*childType)));
}
return Value(dataType, std::move(children));
Expand Down Expand Up @@ -206,12 +210,13 @@ Value::Value(uint8_t* val_) : isNull_{false} {
val.pointer = val_;
}

Value::Value(LogicalType type, const std::string& val_) : isNull_{false} {
Value::Value(const LogicalType& type, std::string val_) : isNull_{false} {
dataType = type.copy();
strVal = val_;
strVal = std::move(val_);
}

Value::Value(LogicalType dataType_, std::vector<std::unique_ptr<Value>> children) : isNull_{false} {
Value::Value(const LogicalType& dataType_, std::vector<std::unique_ptr<Value>> children)
: isNull_{false} {
dataType = dataType_.copy();
this->children = std::move(children);
childrenSize = this->children.size();
Expand Down Expand Up @@ -444,7 +449,7 @@ Value::Value() : isNull_{true} {
dataType = std::make_unique<LogicalType>(LogicalTypeID::ANY);
}

Value::Value(LogicalType dataType_) : isNull_{true} {
Value::Value(const LogicalType& dataType_) : isNull_{true} {
dataType = dataType_.copy();
}

Expand Down
2 changes: 1 addition & 1 deletion src/common/vector/value_vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ ValueVector::ValueVector(LogicalType dataType, storage::MemoryManager* memoryMan
auxiliaryBuffer = AuxiliaryBufferFactory::getAuxiliaryBuffer(this->dataType, memoryManager);
}

void ValueVector::setState(std::shared_ptr<DataChunkState> state_) {
void ValueVector::setState(const std::shared_ptr<DataChunkState>& state_) {
this->state = state_;
if (dataType.getPhysicalType() == PhysicalTypeID::STRUCT) {
auto childrenVectors = StructVector::getFieldVectors(this);
Expand Down
1 change: 1 addition & 0 deletions src/expression_evaluator/case_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ bool CaseExpressionEvaluator::select(SelectionVector& selVector) {

std::unique_ptr<ExpressionEvaluator> CaseExpressionEvaluator::clone() {
std::vector<std::unique_ptr<CaseAlternativeEvaluator>> clonedAlternativeEvaluators;
clonedAlternativeEvaluators.reserve(alternativeEvaluators.size());
for (auto& alternative : alternativeEvaluators) {
clonedAlternativeEvaluators.push_back(alternative->clone());
}
Expand Down
1 change: 1 addition & 0 deletions src/expression_evaluator/function_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ bool FunctionExpressionEvaluator::select(SelectionVector& selVector) {

std::unique_ptr<ExpressionEvaluator> FunctionExpressionEvaluator::clone() {
std::vector<std::unique_ptr<ExpressionEvaluator>> clonedChildren;
clonedChildren.reserve(children.size());
for (auto& child : children) {
clonedChildren.push_back(child->clone());
}
Expand Down
Loading