Skip to content

Commit

Permalink
Switch to use C++20 standard
Browse files Browse the repository at this point in the history
Fixes as a result:
- fix warnings with deprecated usage of volatile variables - replace
  with std::atomic
- Replace volatile member and conditional variable implementation of
  Semaphore with a wrapping of std::counting_semaphore (new in C++20)
- replace usage of deprecated std::is_pod_v
- fix deprecated usage of implicit capture of ‘this’ via ‘[=]’
  • Loading branch information
czentgr committed Aug 27, 2024
1 parent d33cdb2 commit 569a770
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 37 deletions.
16 changes: 15 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,21 @@ endif()
# Required so velox code can be used in a dynamic library
set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)

set(CMAKE_CXX_STANDARD 17)
# For C++20 support we need GNU GCC11 (or later versions) or Clang/AppleClang 15
# (or later versions) to get support for the used features.
if(NOT
(("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND ${CMAKE_CXX_COMPILER_VERSION}
VERSION_GREATER_EQUAL 11)
OR (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang"
OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 15)))
message(
FATAL_ERROR
"Unsupported compiler ${CMAKE_CXX_COMPILER_ID} with version ${CMAKE_CXX_COMPILER_VERSION} found."
)
endif()

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED True)

execute_process(
Expand Down
36 changes: 7 additions & 29 deletions velox/common/base/Semaphore.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,55 +16,33 @@

#pragma once

#include <condition_variable>
#include <cstdint>
#include <functional>
#include <memory>
#include <mutex>

#include "velox/common/base/Exceptions.h"
#include "velox/common/future/VeloxPromise.h"
#include <semaphore>

namespace facebook::velox {

/// Similar to std::counting_semaphore in C++ 20.
/// Wrapper with mutex around std::counting_semaphore in C++ 20.
/// std::counting_semaphore is not bound to a thread but the mutex is.
class Semaphore {
public:
Semaphore(int32_t count) : count_(count) {}
Semaphore(int32_t count) : sem_(count) {}

// If count > 0, decrements count. Otherwise blocks until count is incremented
// by release() from another thread.
void acquire() {
std::unique_lock l(mutex_);
if (count_) {
--count_;
} else {
++numWaiting_;
cv_.wait(l, [&]() { return count_ > 0; });
--numWaiting_;
--count_;
}
sem_.acquire();
}

/// Increments count. Releases one blocking call to acquire, if any.
void release() {
std::lock_guard<std::mutex> l(mutex_);
++count_;
if (numWaiting_ > 0) {
cv_.notify_one();
}
}

int32_t count() {
std::lock_guard<std::mutex> l(mutex_);
return count_;
sem_.release();
}

private:
std::counting_semaphore<> sem_;
std::mutex mutex_;
std::condition_variable cv_;
volatile int32_t count_;
volatile int32_t numWaiting_{0};
};

} // namespace facebook::velox
2 changes: 1 addition & 1 deletion velox/common/base/tests/Memcpy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ int main(int argc, char** argv) {
Semaphore sem(0);
std::vector<CopyCallable> ops;
ops.resize(FLAGS_threads);
volatile uint64_t totalSum = 0;
std::atomic<uint64_t> totalSum = 0;
uint64_t totalUsec = 0;
for (auto repeat = 0; repeat < FLAGS_repeats; ++repeat) {
// Read once through 'other' to clear cache effects.
Expand Down
2 changes: 1 addition & 1 deletion velox/dwio/common/SelectiveColumnReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ class SelectiveColumnReader {
template <typename T>
inline void addValue(T value) {
static_assert(
std::is_pod_v<T>,
std::is_standard_layout_v<T>,
"General case of addValue is only for primitive types");
VELOX_DCHECK_NOT_NULL(rawValues_);
VELOX_DCHECK_LE((numValues_ + 1) * sizeof(T), values_->capacity());
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/tests/ExprEncodingsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ class ExprEncodingsTest
execCtx_->pool(),
vector->type(),
vector->size(),
std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet /*rows*/) {
auto indices =
makeIndices(vector->size(), [](auto row) { return row; });
return wrapInDictionary(indices, vector->size(), vector);
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/tests/ExprTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3954,7 +3954,7 @@ TEST_P(ParameterizedExprTest, cseOverLazyDictionary) {
pool(),
BIGINT(),
5,
std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet /*rows*/) {
return wrapInDictionary(
makeIndicesInReverse(5),
makeFlatVector<int64_t>({8, 9, 10, 11, 12}));
Expand Down
2 changes: 1 addition & 1 deletion velox/vector/tests/utils/VectorMaker.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class VectorMaker {
pool_,
CppToType<T>::create(),
size,
std::make_unique<SimpleVectorLoader>([=](RowSet rowSet) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet rowSet) {
// Populate requested rows with correct data and fill in gaps with
// "garbage".
SelectivityVector rows(rowSet.back() + 1, false);
Expand Down
4 changes: 2 additions & 2 deletions velox/vector/tests/utils/VectorTestBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ class VectorTestBase {
pool(),
CppToType<T>::create(),
size,
std::make_unique<SimpleVectorLoader>([=](RowSet rows) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet rows) {
if (expectedSize.has_value()) {
VELOX_CHECK_EQ(rows.size(), *expectedSize);
}
Expand All @@ -799,7 +799,7 @@ class VectorTestBase {
pool(),
vector->type(),
vector->size(),
std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet /*rows*/) {
auto indices =
makeIndices(vector->size(), [](auto row) { return row; });
return wrapInDictionary(indices, vector->size(), vector);
Expand Down

0 comments on commit 569a770

Please sign in to comment.