From 2295cde4e568c41178d3c5509b70fd14ec971b7f Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Thu, 20 Feb 2020 09:57:33 -0800 Subject: [PATCH] Enable absl completely --- BUILD | 5 - bazel/grpc_build_system.bzl | 4 - src/core/lib/channel/channelz.h | 8 +- src/core/lib/gprpp/inlined_vector.h | 217 +----------------- src/core/lib/gprpp/optional.h | 37 --- src/core/lib/gprpp/string_view.h | 124 +--------- .../linux/grpc_bazel_build_in_docker.sh | 4 - 7 files changed, 8 insertions(+), 391 deletions(-) diff --git a/BUILD b/BUILD index 1f1d9affc34..856bd0d3fee 100644 --- a/BUILD +++ b/BUILD @@ -72,11 +72,6 @@ config_setting( values = {"cpu": "darwin"}, ) -config_setting( - name = "grpc_disable_absl", - values = {"define": "GRPC_USE_ABSL=0"}, -) - python_config_settings() # This should be updated along with build.yaml diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 732279d86d5..0abbc1d401b 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -100,10 +100,6 @@ def grpc_cc_library( "//:grpc_allow_exceptions": ["GRPC_ALLOW_EXCEPTIONS=1"], "//:grpc_disallow_exceptions": ["GRPC_ALLOW_EXCEPTIONS=0"], "//conditions:default": [], - }) + - select({ - "//:grpc_disable_absl": ["GRPC_USE_ABSL=0"], - "//conditions:default": [], }), hdrs = hdrs + public_hdrs, deps = deps + _get_external_deps(external_deps), diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h index 11b1c7f1cfa..73a9b57a45c 100644 --- a/src/core/lib/channel/channelz.h +++ b/src/core/lib/channel/channelz.h @@ -148,17 +148,13 @@ class CallCountingHelper { // Make sure the size is exactly one cache line. uint8_t padding[GPR_CACHELINE_SIZE - 3 * sizeof(Atomic) - sizeof(Atomic)]; - } -#if GRPC_USE_ABSL + }; // TODO(soheilhy,veblush): Revist this after abseil integration. // This has a problem when using abseil inlined_vector because it // carries an alignment attribute properly but our allocator doesn't // respect this. To avoid UBSAN errors, this should be removed with // abseil inlined_vector. - ; -#else - GPR_ALIGN_STRUCT(GPR_CACHELINE_SIZE); -#endif + // GPR_ALIGN_STRUCT(GPR_CACHELINE_SIZE); struct CounterData { int64_t calls_started = 0; diff --git a/src/core/lib/gprpp/inlined_vector.h b/src/core/lib/gprpp/inlined_vector.h index 449bb0b339a..3cad99d3429 100644 --- a/src/core/lib/gprpp/inlined_vector.h +++ b/src/core/lib/gprpp/inlined_vector.h @@ -24,229 +24,14 @@ #include #include -#include "src/core/lib/gprpp/memory.h" - -#if GRPC_USE_ABSL #include "absl/container/inlined_vector.h" -#endif +#include "src/core/lib/gprpp/memory.h" namespace grpc_core { -#if GRPC_USE_ABSL - template > using InlinedVector = absl::InlinedVector; -#else - -// NOTE: We eventually want to use absl::InlinedVector here. However, -// there are currently build problems that prevent us from using absl. -// In the interim, we define a custom implementation as a place-holder, -// with the intent to eventually replace this with the absl -// implementation. -// -// This place-holder implementation does not implement the full set of -// functionality from the absl version; it has just the methods that we -// currently happen to need in gRPC. If additional functionality is -// needed before this gets replaced with the absl version, it can be -// added, with the following proviso: -// -// ANY METHOD ADDED HERE MUST COMPLY WITH THE INTERFACE IN THE absl -// IMPLEMENTATION! -// -// TODO(nnoble, roth): Replace this with absl::InlinedVector once we -// integrate absl into the gRPC build system in a usable way. -template -class InlinedVector { - public: - InlinedVector() { init_data(); } - ~InlinedVector() { destroy_elements(); } - - // copy constructor - InlinedVector(const InlinedVector& v) { - init_data(); - copy_from(v); - } - - InlinedVector& operator=(const InlinedVector& v) { - if (this != &v) { - clear(); - copy_from(v); - } - return *this; - } - - // move constructor - InlinedVector(InlinedVector&& v) { - init_data(); - move_from(v); - } - - InlinedVector& operator=(InlinedVector&& v) { - if (this != &v) { - clear(); - move_from(v); - } - return *this; - } - - T* data() { - return dynamic_ != nullptr ? dynamic_ : reinterpret_cast(inline_); - } - - const T* data() const { - return dynamic_ != nullptr ? dynamic_ : reinterpret_cast(inline_); - } - - T& operator[](size_t offset) { - assert(offset < size_); - return data()[offset]; - } - - const T& operator[](size_t offset) const { - assert(offset < size_); - return data()[offset]; - } - - bool operator==(const InlinedVector& other) const { - if (size_ != other.size_) return false; - for (size_t i = 0; i < size_; ++i) { - // Note that this uses == instead of != so that the data class doesn't - // have to implement !=. - if (!(data()[i] == other.data()[i])) return false; - } - return true; - } - - bool operator!=(const InlinedVector& other) const { - return !(*this == other); - } - - void reserve(size_t capacity) { - if (capacity > capacity_) { - T* new_dynamic = - std::alignment_of::value == 0 - ? static_cast(gpr_malloc(sizeof(T) * capacity)) - : static_cast(gpr_malloc_aligned( - sizeof(T) * capacity, std::alignment_of::value)); - move_elements(data(), new_dynamic, size_); - free_dynamic(); - dynamic_ = new_dynamic; - capacity_ = capacity; - } - } - - void resize(size_t new_size) { - while (new_size > size_) emplace_back(); - while (new_size < size_) pop_back(); - } - - template - void emplace_back(Args&&... args) { - if (size_ == capacity_) { - reserve(capacity_ * 2); - } - new (&(data()[size_])) T(std::forward(args)...); - ++size_; - } - - void push_back(const T& value) { emplace_back(value); } - - void push_back(T&& value) { emplace_back(std::move(value)); } - - void pop_back() { - assert(!empty()); - size_t s = size(); - T& value = data()[s - 1]; - value.~T(); - size_--; - } - - T& front() { return data()[0]; } - const T& front() const { return data()[0]; } - - T& back() { return data()[size_ - 1]; } - const T& back() const { return data()[size_ - 1]; } - - size_t size() const { return size_; } - bool empty() const { return size_ == 0; } - - size_t capacity() const { return capacity_; } - - void clear() { - destroy_elements(); - init_data(); - } - - private: - void copy_from(const InlinedVector& v) { - // if v is allocated, make sure we have enough capacity. - if (v.dynamic_ != nullptr) { - reserve(v.capacity_); - } - // copy over elements - for (size_t i = 0; i < v.size_; ++i) { - new (&(data()[i])) T(v[i]); - } - // copy over metadata - size_ = v.size_; - capacity_ = v.capacity_; - } - - void move_from(InlinedVector& v) { - // if v is allocated, then we steal its dynamic array; otherwise, we - // move the elements individually. - if (v.dynamic_ != nullptr) { - dynamic_ = v.dynamic_; - } else { - move_elements(v.data(), data(), v.size_); - } - // copy over metadata - size_ = v.size_; - capacity_ = v.capacity_; - // null out the original - v.init_data(); - } - - static void move_elements(T* src, T* dst, size_t num_elements) { - for (size_t i = 0; i < num_elements; ++i) { - new (&dst[i]) T(std::move(src[i])); - src[i].~T(); - } - } - - void init_data() { - dynamic_ = nullptr; - size_ = 0; - capacity_ = N; - } - - void destroy_elements() { - for (size_t i = 0; i < size_; ++i) { - T& value = data()[i]; - value.~T(); - } - free_dynamic(); - } - - void free_dynamic() { - if (dynamic_ != nullptr) { - if (std::alignment_of::value == 0) { - gpr_free(dynamic_); - } else { - gpr_free_aligned(dynamic_); - } - } - } - - typename std::aligned_storage::type inline_[N]; - T* dynamic_; - size_t size_; - size_t capacity_; -}; - -#endif - } // namespace grpc_core #endif /* GRPC_CORE_LIB_GPRPP_INLINED_VECTOR_H */ diff --git a/src/core/lib/gprpp/optional.h b/src/core/lib/gprpp/optional.h index cb8f71f4bf6..f4535bb03a8 100644 --- a/src/core/lib/gprpp/optional.h +++ b/src/core/lib/gprpp/optional.h @@ -21,8 +21,6 @@ #include -#if GRPC_USE_ABSL - #include "absl/types/optional.h" namespace grpc_core { @@ -32,39 +30,4 @@ using Optional = absl::optional; } // namespace grpc_core -#else - -#include - -namespace grpc_core { - -/* A make-shift alternative for absl::Optional. This can be removed in favor of - * that once absl dependencies can be introduced. */ -template -class Optional { - public: - Optional() : value_() {} - - template - T& emplace(Args&&... args) { - value_ = T(std::forward(args)...); - set_ = true; - return value_; - } - - bool has_value() const { return set_; } - - void reset() { set_ = false; } - - T value() const { return value_; } - - private: - T value_; - bool set_ = false; -}; - -} /* namespace grpc_core */ - -#endif - #endif /* GRPC_CORE_LIB_GPRPP_OPTIONAL_H */ diff --git a/src/core/lib/gprpp/string_view.h b/src/core/lib/gprpp/string_view.h index a718b79231c..bd4724bc1e9 100644 --- a/src/core/lib/gprpp/string_view.h +++ b/src/core/lib/gprpp/string_view.h @@ -30,134 +30,20 @@ #include #include +#include "absl/strings/string_view.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/memory.h" -#if GRPC_USE_ABSL -#include "absl/strings/string_view.h" -#endif - namespace grpc_core { -#if GRPC_USE_ABSL - using StringView = absl::string_view; -#else - -// Provides a light-weight view over a char array or a slice, similar but not -// identical to absl::string_view. -// -// Any method that has the same name as absl::string_view MUST HAVE identical -// semantics to what absl::string_view provides. -// -// Methods that are not part of absl::string_view API, must be clearly -// annotated. -// -// StringView does not own the buffers that back the view. Callers must ensure -// the buffer stays around while the StringView is accessible. -// -// Pass StringView by value in functions, since it is exactly two pointers in -// size. -// -// The interface used here is not identical to absl::string_view. Notably, we -// need to support slices while we cannot support std::string, and gpr string -// style functions such as strdup() and cmp(). Once we switch to -// absl::string_view this class will inherit from absl::string_view and add the -// gRPC-specific APIs. -class StringView final { - public: - static constexpr size_t npos = std::numeric_limits::max(); - - constexpr StringView(const char* ptr, size_t size) : ptr_(ptr), size_(size) {} - constexpr StringView(const char* ptr) - : StringView(ptr, ptr == nullptr ? 0 : strlen(ptr)) {} - constexpr StringView() : StringView(nullptr, 0) {} - - template - StringView( - const std::basic_string, Allocator>& str) - : StringView(str.data(), str.size()) {} - - constexpr const char* data() const { return ptr_; } - constexpr size_t size() const { return size_; } - constexpr bool empty() const { return size_ == 0; } - - StringView substr(size_t start, size_t size = npos) { - GPR_DEBUG_ASSERT(start + size <= size_); - return StringView(ptr_ + start, std::min(size, size_ - start)); - } - - constexpr const char& operator[](size_t i) const { return ptr_[i]; } - - const char& front() const { return ptr_[0]; } - const char& back() const { return ptr_[size_ - 1]; } - - void remove_prefix(size_t n) { - GPR_DEBUG_ASSERT(n <= size_); - ptr_ += n; - size_ -= n; - } - - void remove_suffix(size_t n) { - GPR_DEBUG_ASSERT(n <= size_); - size_ -= n; - } - - size_t find(char c, size_t pos = 0) const { - if (empty() || pos >= size_) return npos; - const char* result = - static_cast(memchr(ptr_ + pos, c, size_ - pos)); - return result != nullptr ? result - ptr_ : npos; - } - - void clear() { - ptr_ = nullptr; - size_ = 0; - } - - // Converts to `std::basic_string`. - template - explicit operator std::basic_string, Allocator>() - const { - if (data() == nullptr) return {}; - return std::basic_string, Allocator>(data(), - size()); - } - - // Compares with other. - inline int compare(StringView other) { - const size_t len = GPR_MIN(size(), other.size()); - const int ret = strncmp(data(), other.data(), len); - if (ret != 0) return ret; - if (size() == other.size()) return 0; - if (size() < other.size()) return -1; - return 1; - } - - private: - const char* ptr_; - size_t size_; -}; - -inline bool operator==(StringView lhs, StringView rhs) { - return lhs.size() == rhs.size() && - strncmp(lhs.data(), rhs.data(), lhs.size()) == 0; -} - -inline bool operator!=(StringView lhs, StringView rhs) { return !(lhs == rhs); } - -inline bool operator<(StringView lhs, StringView rhs) { - return lhs.compare(rhs) < 0; -} - -#endif // GRPC_USE_ABSL - // Converts grpc_slice to StringView. -inline StringView StringViewFromSlice(const grpc_slice& slice) { - return StringView(reinterpret_cast(GRPC_SLICE_START_PTR(slice)), - GRPC_SLICE_LENGTH(slice)); +inline absl::string_view StringViewFromSlice(const grpc_slice& slice) { + return absl::string_view( + reinterpret_cast(GRPC_SLICE_START_PTR(slice)), + GRPC_SLICE_LENGTH(slice)); } // Creates a dup of the string viewed by this class. diff --git a/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh b/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh index c11dfc15bcd..24598f43f02 100755 --- a/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh +++ b/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh @@ -25,7 +25,3 @@ git clone /var/local/jenkins/grpc /var/local/git/grpc ${name}') cd /var/local/git/grpc bazel build --spawn_strategy=standalone --genrule_strategy=standalone :all test/... examples/... - -# This is a temporary build test before absl is proven safe to work with gRPC. -# TODO(veblush): Remove this after abseil integration is finally done. -bazel build --spawn_strategy=standalone --genrule_strategy=standalone --define=GRPC_USE_ABSL=0 :grpc