Skip to content

Commit

Permalink
[Feature] upgrade c++17 to c++20 (StarRocks#26432)
Browse files Browse the repository at this point in the history
upgrade to c++20 to benefit from std::coroutine

---------

Signed-off-by: Zhuhe Fang <[email protected]>
  • Loading branch information
fzhedu authored Jul 17, 2023
1 parent 6f9b36b commit a20a7d0
Show file tree
Hide file tree
Showing 32 changed files with 78 additions and 69 deletions.
1 change: 0 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ Checks: >
modernize-concat-nested-namespaces,
modernize-deprecated-headers,
modernize-deprecated-ios-base-aliases,
modernize-loop-convert,
modernize-make-shared,
modernize-make-unique,
modernize-pass-by-value,
Expand Down
4 changes: 2 additions & 2 deletions be/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} $ENV{STARROCKS_CXX_LINKER_
# -fno-omit-frame-pointers: Keep frame pointer for functions in register
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wall -Wno-sign-compare -Wno-unknown-pragmas -pthread -Wno-register")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-strict-aliasing -fno-omit-frame-pointer")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -std=gnu++17 -D__STDC_FORMAT_MACROS")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -std=gnu++20 -D__STDC_FORMAT_MACROS")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-deprecated -Wno-vla -Wno-comment")

if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
Expand All @@ -522,7 +522,7 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-documentation-unknown-command -Wno-old-style-cast")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-c++20-designator -Wno-mismatched-tags")
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "14.0.0")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-bitwise-instead-of-logical")
set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-bitwise-instead-of-logical -Wno-inconsistent-missing-override -Wno-ambiguous-reversed-operator")
endif()
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "16.0.0")
# ignore warning from apache-orc
Expand Down
11 changes: 7 additions & 4 deletions be/src/column/column_hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,12 @@ class SliceHashWithSeed<PhmapSeed2> {

#if defined(__SSE2__) && !defined(ADDRESS_SANITIZER)

// NOTE: This function will access 15 excessive bytes after p1 and p2.
// NOTE: This function will access 15 excessive bytes after p1 and p2, which should has padding bytes when allocating
// memory. if withoud pad, please use memequal.
// NOTE: typename T must be uint8_t or int8_t
template <typename T>
typename std::enable_if<sizeof(T) == 1, bool>::type memequal(const T* p1, size_t size1, const T* p2, size_t size2) {
typename std::enable_if<sizeof(T) == 1, bool>::type memequal_padded(const T* p1, size_t size1, const T* p2,
size_t size2) {
if (size1 != size2) {
return false;
}
Expand All @@ -249,15 +251,16 @@ typename std::enable_if<sizeof(T) == 1, bool>::type memequal(const T* p1, size_t
#else

template <typename T>
typename std::enable_if<sizeof(T) == 1, bool>::type memequal(const T* p1, size_t size1, const T* p2, size_t size2) {
typename std::enable_if<sizeof(T) == 1, bool>::type memequal_padded(const T* p1, size_t size1, const T* p2,
size_t size2) {
return (size1 == size2) && (memcmp(p1, p2, size1) == 0);
}
#endif

static constexpr uint16_t SLICE_MEMEQUAL_OVERFLOW_PADDING = 15;
class SliceEqual {
public:
bool operator()(const Slice& x, const Slice& y) const { return memequal(x.data, x.size, y.data, y.size); }
bool operator()(const Slice& x, const Slice& y) const { return memequal_padded(x.data, x.size, y.data, y.size); }
};

class SliceNormalEqual {
Expand Down
1 change: 1 addition & 0 deletions be/src/column/column_pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "common/type_list.h"
#include "gutil/dynamic_annotations.h"
#include "runtime/current_thread.h"
#include "util/json.h"

namespace starrocks {

Expand Down
4 changes: 2 additions & 2 deletions be/src/column/hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class EqualOnSliceWithHash {
bool operator()(const SliceWithHash& x, const SliceWithHash& y) const {
// by comparing hash value first, we can avoid comparing real data
// which may touch another memory area and has bad cache locality.
return x.hash == y.hash && memequal(x.data, x.size, y.data, y.size);
return x.hash == y.hash && memequal_padded(x.data, x.size, y.data, y.size);
}
};

Expand All @@ -74,7 +74,7 @@ class TEqualOnSliceWithHash {
bool operator()(const TSliceWithHash<seed>& x, const TSliceWithHash<seed>& y) const {
// by comparing hash value first, we can avoid comparing real data
// which may touch another memory area and has bad cache locality.
return x.hash == y.hash && memequal(x.data, x.size, y.data, y.size);
return x.hash == y.hash && memequal_padded(x.data, x.size, y.data, y.size);
}
};

Expand Down
2 changes: 1 addition & 1 deletion be/src/connector/binlog_connector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Status BinlogDataSource::get_next(RuntimeState* state, ChunkPtr* chunk) {
return _mock_chunk_test(chunk);
}
#endif
if (_need_seek_binlog.load(std::memory_order::memory_order_acquire)) {
if (_need_seek_binlog.load(std::memory_order::acquire)) {
if (!_is_stream_pipeline) {
RETURN_IF_ERROR(_prepare_non_stream_pipeline());
}
Expand Down
2 changes: 1 addition & 1 deletion be/src/exec/except_hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ExceptSliceFlag {

struct ExceptSliceFlagEqual {
bool operator()(const ExceptSliceFlag& x, const ExceptSliceFlag& y) const {
return memequal(x.slice.data, x.slice.size, y.slice.data, y.slice.size);
return memequal_padded(x.slice.data, x.slice.size, y.slice.data, y.slice.size);
}
};

Expand Down
2 changes: 1 addition & 1 deletion be/src/exec/intersect_hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class IntersectSliceFlag {

struct IntersectSliceFlagEqual {
bool operator()(const IntersectSliceFlag& x, const IntersectSliceFlag& y) const {
return memequal(x.slice.data, x.slice.size, y.slice.data, y.slice.size);
return memequal_padded(x.slice.data, x.slice.size, y.slice.data, y.slice.size);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Status SchemaBeCompactionsScanner::start(RuntimeState* state) {
auto o_id = get_backend_id();
_be_id = o_id.has_value() ? o_id.value() : -1;
_infos.clear();
CompactionInfo info;
CompactionInformation info;
auto compaction_manager = StorageEngine::instance()->compaction_manager();
info.candidates_num = compaction_manager->candidates_size();
info.base_compaction_concurrency = compaction_manager->base_compaction_concurrency();
Expand Down
4 changes: 2 additions & 2 deletions be/src/exec/schema_scanner/schema_be_compactions_scanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace starrocks {

struct CompactionInfo {
struct CompactionInformation {
int64_t candidates_num = 0;
int64_t base_compaction_concurrency = 0;
int64_t cumulative_compaction_concurrency = 0;
Expand All @@ -43,7 +43,7 @@ class SchemaBeCompactionsScanner : public SchemaScanner {
Status fill_chunk(ChunkPtr* chunk);

int64_t _be_id{0};
std::vector<CompactionInfo> _infos;
std::vector<CompactionInformation> _infos;
size_t _cur_idx{0};
static SchemaScanner::ColumnDesc _s_columns[];
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <climits>

#include "exec/schema_scanner/schema_helper.h"
#include "gutil/strings/substitute.h"
#include "http/http_client.h"
#include "runtime/runtime_state.h"
#include "runtime/string_value.h"
Expand Down Expand Up @@ -75,7 +76,7 @@ Status SchemaLoadTrackingLogsScanner::fill_chunk(ChunkPtr* chunk) {
auto& info = _result.trackingLoads[_cur_idx];
for (const auto& [slot_id, index] : slot_id_to_index_map) {
if (slot_id < 1 || slot_id > 5) {
return Status::InternalError(fmt::format("invalid slot id:{}}", slot_id));
return Status::InternalError(strings::Substitute("invalid slot id: $0", slot_id));
}
ColumnPtr column = (*chunk)->get_column_by_slot_id(slot_id);
switch (slot_id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "exec/schema_scanner/schema_routine_load_jobs_scanner.h"

#include "exec/schema_scanner/schema_helper.h"
#include "gutil/strings/substitute.h"
#include "http/http_client.h"
#include "runtime/runtime_state.h"
#include "runtime/string_value.h"
Expand Down Expand Up @@ -79,7 +80,7 @@ Status SchemaRoutineLoadJobsScanner::fill_chunk(ChunkPtr* chunk) {
auto& info = _result.loads[_cur_idx];
for (const auto& [slot_id, index] : slot_id_to_index_map) {
if (slot_id < 1 || slot_id > 19) {
return Status::InternalError(fmt::format("invalid slot id:{}}", slot_id));
return Status::InternalError(strings::Substitute("invalid slot id: $0", slot_id));
}
ColumnPtr column = (*chunk)->get_column_by_slot_id(slot_id);
switch (slot_id) {
Expand Down
2 changes: 1 addition & 1 deletion be/src/exec/spill/spiller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ Status PartitionedSpillerWriter::flush(RuntimeState* state, bool is_final_flush,
DCHECK_EQ(_running_flush_tasks, 0);
_running_flush_tasks++;

auto task = [this, state, guard = guard, splitting_partitions = std::move(splitting_partitions),
auto task = [this, guard = guard, splitting_partitions = std::move(splitting_partitions),
spilling_partitions = std::move(spilling_partitions), trace = TraceInfo(state)]() {
SCOPED_SET_TRACE_INFO({}, trace.query_id, trace.fragment_id);
RETURN_IF(!guard.scoped_begin(), Status::Cancelled("cancelled"));
Expand Down
2 changes: 1 addition & 1 deletion be/src/exprs/cast_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1450,7 +1450,7 @@ StatusOr<ColumnPtr> MustNullExpr::evaluate_checked(ExprContext* context, Chunk*
// only null
auto column = ColumnHelper::create_column(_type, true);
column->append_nulls(1);
auto only_null = std::move(ConstColumn::create(column, 1));
auto only_null = ConstColumn::create(column, 1);
if (ptr != nullptr) {
only_null->resize(ptr->num_rows());
}
Expand Down
4 changes: 2 additions & 2 deletions be/src/formats/orc/apache-orc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ if (NOT MSVC)
endif ()
message(STATUS "compiler ${CMAKE_CXX_COMPILER_ID} version ${CMAKE_CXX_COMPILER_VERSION}")
if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
set (CMAKE_CXX_STANDARD 17)
set (CMAKE_CXX_STANDARD 20)
set (WARN_FLAGS "-Weverything -Wno-c++98-compat -Wno-missing-prototypes")
set (WARN_FLAGS "${WARN_FLAGS} -Wno-c++98-compat-pedantic -Wno-padded")
set (WARN_FLAGS "${WARN_FLAGS} -Wno-covered-switch-default")
Expand All @@ -138,7 +138,7 @@ elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
CMAKE_CXX_COMPILER_VERSION VERSION_LESS "4.7")
set (CXX11_FLAGS "-std=c++0x")
else ()
set (CMAKE_CXX_STANDARD 17)
set (CMAKE_CXX_STANDARD 20)
endif ()
elseif (MSVC)
add_definitions (-D_SCL_SECURE_NO_WARNINGS)
Expand Down
2 changes: 0 additions & 2 deletions be/src/fs/fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ struct SequentialFileOptions {
};

struct RandomAccessFileOptions {
RandomAccessFileOptions() = default;

// Don't cache remote file locally on read requests.
// This options can be ignored if the underlying filesystem does not support local cache.
bool skip_fill_local_cache = false;
Expand Down
2 changes: 1 addition & 1 deletion be/src/fs/fs_s3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static Status to_status(Aws::S3::S3Errors error, const std::string& msg) {
case Aws::S3::S3Errors::NO_SUCH_UPLOAD:
return Status::NotFound(fmt::format("no such upload: {}", msg));
default:
return Status::InternalError(fmt::format(msg));
return Status::InternalError(msg);
}
}

Expand Down
9 changes: 6 additions & 3 deletions be/src/fs/fs_starlet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,9 @@ class StarletFileSystem : public FileSystem {
if (!fs_st.ok()) {
return to_status(fs_st.status());
}

auto file_st = (*fs_st)->open(pair.first, ReadOptions{.skip_fill_local_cache = opts.skip_fill_local_cache});
auto opt = ReadOptions();
opt.skip_fill_local_cache = opts.skip_fill_local_cache;
auto file_st = (*fs_st)->open(pair.first, std::move(opt));

if (!file_st.ok()) {
return to_status(file_st.status());
Expand All @@ -274,7 +275,9 @@ class StarletFileSystem : public FileSystem {
if (!fs_st.ok()) {
return to_status(fs_st.status());
}
auto file_st = (*fs_st)->open(pair.first, ReadOptions{.skip_fill_local_cache = opts.skip_fill_local_cache});
auto opt = ReadOptions();
opt.skip_fill_local_cache = opts.skip_fill_local_cache;
auto file_st = (*fs_st)->open(pair.first, std::move(opt));

if (!file_st.ok()) {
return to_status(file_st.status());
Expand Down
10 changes: 6 additions & 4 deletions be/src/gutil/stl_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -756,8 +756,10 @@ BinaryComposeBinary<F, G1, G2> BinaryCompose2(F f, G1 g1, G2 g2) {
template <typename T, typename Alloc = std::allocator<T> >
class STLCountingAllocator : public Alloc {
public:
typedef typename Alloc::pointer pointer;
typedef typename Alloc::size_type size_type;
using AllocatorTraits = std::allocator_traits<Alloc>;

typedef typename AllocatorTraits::pointer pointer;
typedef typename AllocatorTraits::size_type size_type;

STLCountingAllocator() {}
STLCountingAllocator(int64* b) : bytes_used_(b) {} // TODO(user): explicit?
Expand All @@ -766,10 +768,10 @@ class STLCountingAllocator : public Alloc {
template <class U>
STLCountingAllocator(const STLCountingAllocator<U>& x) : Alloc(x), bytes_used_(x.bytes_used()) {}

pointer allocate(size_type n, std::allocator<void>::const_pointer hint = nullptr) {
pointer allocate(size_type n) {
assert(bytes_used_ != NULL);
*bytes_used_ += n * sizeof(T);
return Alloc::allocate(n, hint);
return Alloc::allocate(n);
}

void deallocate(pointer p, size_type n) {
Expand Down
2 changes: 1 addition & 1 deletion be/src/storage/persistent_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ struct StringHasher2 {
class EqualOnStringWithHash {
public:
bool operator()(const std::string& lhs, const std::string& rhs) const {
return memequal(lhs.data(), lhs.size() - kIndexValueSize, rhs.data(), rhs.size() - kIndexValueSize);
return memequal_padded(lhs.data(), lhs.size() - kIndexValueSize, rhs.data(), rhs.size() - kIndexValueSize);
}
};

Expand Down
4 changes: 3 additions & 1 deletion be/src/util/download_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <boost/algorithm/string/predicate.hpp>

#include "fmt/format.h"
#include "gutil/strings/substitute.h"
#include "http/http_client.h"
#include "util/defer_op.h"
#include "util/md5.h"
Expand Down Expand Up @@ -47,7 +48,8 @@ Status DownloadUtil::download(const std::string& url, const std::string& tmp_fil
auto res = fwrite(data, length, 1, fp);
if (res != 1) {
LOG(ERROR) << fmt::format("fail to write data to file {}, error={}", tmp_file, ferror(fp));
status = Status::InternalError(fmt::format("file to write data when downloading file from {}" + url));
status =
Status::InternalError(strings::Substitute("file to write data when downloading file from $0", url));
return false;
}
return true;
Expand Down
5 changes: 3 additions & 2 deletions be/src/util/memcmp.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ inline int compare(T lhs, T rhs) {
}
}

// mem_equal is used to optimize the comparastion between the two strings.
// memequal is used to optimize the comparison between the two strings.
// 1. If the length is equal and larger than 16, use SSE4.1
// 2. If the length is small than 16, convert the address to int16/int32/int64
// to comparasion
// to comparison
// so it does not need to consider extra padding bytes for SIMD, which is required by memequal_padded().
// TODO: If know the size in advance, call the function by constant parameter
// like memequal(p1, 10, p2, 10) is efficient

Expand Down
Loading

0 comments on commit a20a7d0

Please sign in to comment.