From 72c296a3c2db882e164036fd83746315985acf06 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 20 Dec 2022 19:46:27 -0800 Subject: [PATCH] [event_engine] Add SliceCast (#31831) * [event_engine] Add SliceCast * Automated change: Fix sanity tests * windows-fix * comments * comments * comments * build-fix * fix * Update port_platform.h Co-authored-by: ctiller --- BUILD | 2 + CMakeLists.txt | 3 + Makefile | 2 + build_autogenerated.yaml | 3 + gRPC-Core.podspec | 1 + grpc.gemspec | 1 + .../grpc/event_engine/internal/slice_cast.h | 55 +++++++++++++++++++ include/grpc/event_engine/slice.h | 13 ++++- include/grpc/event_engine/slice_buffer.h | 6 ++ include/grpc/support/port_platform.h | 13 +++++ package.xml | 1 + src/core/BUILD | 9 +++ src/core/lib/slice/slice.h | 24 +++++++- src/core/lib/slice/slice_buffer.h | 5 ++ test/core/slice/slice_test.cc | 9 +++ tools/buildgen/plugins/list_api.py | 2 +- tools/doxygen/Doxyfile.c++ | 1 + tools/doxygen/Doxyfile.c++.internal | 1 + tools/doxygen/Doxyfile.core | 1 + tools/doxygen/Doxyfile.core.internal | 1 + 20 files changed, 147 insertions(+), 6 deletions(-) create mode 100644 include/grpc/event_engine/internal/slice_cast.h diff --git a/BUILD b/BUILD index a3330873dc6..eb361ee9bb8 100644 --- a/BUILD +++ b/BUILD @@ -229,6 +229,7 @@ GRPC_PUBLIC_EVENT_ENGINE_HDRS = [ "include/grpc/event_engine/internal/memory_allocator_impl.h", "include/grpc/event_engine/slice.h", "include/grpc/event_engine/slice_buffer.h", + "include/grpc/event_engine/internal/slice_cast.h", ] GRPCXX_SRCS = [ @@ -1423,6 +1424,7 @@ grpc_cc_library( "//src/core:resource_quota_trace", "//src/core:slice", "//src/core:slice_buffer", + "//src/core:slice_cast", "//src/core:slice_refcount", "//src/core:socket_mutator", "//src/core:stats_data", diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c057e3e477..ee75debc48c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2463,6 +2463,7 @@ foreach(_hdr include/grpc/event_engine/endpoint_config.h include/grpc/event_engine/event_engine.h include/grpc/event_engine/internal/memory_allocator_impl.h + include/grpc/event_engine/internal/slice_cast.h include/grpc/event_engine/memory_allocator.h include/grpc/event_engine/memory_request.h include/grpc/event_engine/port.h @@ -3073,6 +3074,7 @@ foreach(_hdr include/grpc/event_engine/endpoint_config.h include/grpc/event_engine/event_engine.h include/grpc/event_engine/internal/memory_allocator_impl.h + include/grpc/event_engine/internal/slice_cast.h include/grpc/event_engine/memory_allocator.h include/grpc/event_engine/memory_request.h include/grpc/event_engine/port.h @@ -4539,6 +4541,7 @@ foreach(_hdr include/grpc/event_engine/endpoint_config.h include/grpc/event_engine/event_engine.h include/grpc/event_engine/internal/memory_allocator_impl.h + include/grpc/event_engine/internal/slice_cast.h include/grpc/event_engine/memory_allocator.h include/grpc/event_engine/memory_request.h include/grpc/event_engine/port.h diff --git a/Makefile b/Makefile index db1924586c2..95b48a4ed49 100644 --- a/Makefile +++ b/Makefile @@ -1680,6 +1680,7 @@ PUBLIC_HEADERS_C += \ include/grpc/event_engine/endpoint_config.h \ include/grpc/event_engine/event_engine.h \ include/grpc/event_engine/internal/memory_allocator_impl.h \ + include/grpc/event_engine/internal/slice_cast.h \ include/grpc/event_engine/memory_allocator.h \ include/grpc/event_engine/memory_request.h \ include/grpc/event_engine/port.h \ @@ -2150,6 +2151,7 @@ PUBLIC_HEADERS_C += \ include/grpc/event_engine/endpoint_config.h \ include/grpc/event_engine/event_engine.h \ include/grpc/event_engine/internal/memory_allocator_impl.h \ + include/grpc/event_engine/internal/slice_cast.h \ include/grpc/event_engine/memory_allocator.h \ include/grpc/event_engine/memory_request.h \ include/grpc/event_engine/port.h \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 4f0b28f7a5d..862322a421e 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -260,6 +260,7 @@ libs: - include/grpc/event_engine/endpoint_config.h - include/grpc/event_engine/event_engine.h - include/grpc/event_engine/internal/memory_allocator_impl.h + - include/grpc/event_engine/internal/slice_cast.h - include/grpc/event_engine/memory_allocator.h - include/grpc/event_engine/memory_request.h - include/grpc/event_engine/port.h @@ -1859,6 +1860,7 @@ libs: - include/grpc/event_engine/endpoint_config.h - include/grpc/event_engine/event_engine.h - include/grpc/event_engine/internal/memory_allocator_impl.h + - include/grpc/event_engine/internal/slice_cast.h - include/grpc/event_engine/memory_allocator.h - include/grpc/event_engine/memory_request.h - include/grpc/event_engine/port.h @@ -3383,6 +3385,7 @@ libs: - include/grpc/event_engine/endpoint_config.h - include/grpc/event_engine/event_engine.h - include/grpc/event_engine/internal/memory_allocator_impl.h + - include/grpc/event_engine/internal/slice_cast.h - include/grpc/event_engine/memory_allocator.h - include/grpc/event_engine/memory_request.h - include/grpc/event_engine/port.h diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index d08d6a6dd29..4a55c0e1cf9 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -111,6 +111,7 @@ Pod::Spec.new do |s| 'include/grpc/event_engine/endpoint_config.h', 'include/grpc/event_engine/event_engine.h', 'include/grpc/event_engine/internal/memory_allocator_impl.h', + 'include/grpc/event_engine/internal/slice_cast.h', 'include/grpc/event_engine/memory_allocator.h', 'include/grpc/event_engine/memory_request.h', 'include/grpc/event_engine/port.h', diff --git a/grpc.gemspec b/grpc.gemspec index 255cae8c491..4d12ccdbe37 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -54,6 +54,7 @@ Gem::Specification.new do |s| s.files += %w( include/grpc/event_engine/endpoint_config.h ) s.files += %w( include/grpc/event_engine/event_engine.h ) s.files += %w( include/grpc/event_engine/internal/memory_allocator_impl.h ) + s.files += %w( include/grpc/event_engine/internal/slice_cast.h ) s.files += %w( include/grpc/event_engine/memory_allocator.h ) s.files += %w( include/grpc/event_engine/memory_request.h ) s.files += %w( include/grpc/event_engine/port.h ) diff --git a/include/grpc/event_engine/internal/slice_cast.h b/include/grpc/event_engine/internal/slice_cast.h new file mode 100644 index 00000000000..ab9db95f937 --- /dev/null +++ b/include/grpc/event_engine/internal/slice_cast.h @@ -0,0 +1,55 @@ +// Copyright 2022 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GRPC_EVENT_ENGINE_INTERNAL_SLICE_CAST_H +#define GRPC_EVENT_ENGINE_INTERNAL_SLICE_CAST_H + +namespace grpc_event_engine { +namespace experimental { +namespace internal { + +// Opt-in trait class for slice conversions. +// Declare a specialization of this class for any types that are compatible +// with `SliceCast`. Both ways need to be declared (i.e. if +// ConstRefSliceCastable exists, you should declare +// ConstRefSliceCastable too). +// The type has no members, it's just the existance of the specialization that +// unlocks SliceCast usage for a type pair. +template +struct ConstRefSliceCastable; + +// This is strictly too wide, but consider all types to be SliceCast-able to +// themselves. +// Unfortunately this allows `const int& x = SliceCast(x);` which is kind +// of bogus. +template +struct ConstRefSliceCastable {}; + +// Cast to `const Result&` from `const T&` without any runtime checks. +// This is only valid if `sizeof(Result) == sizeof(T)`, and if `Result`, `T` are +// opted in as compatible via `ConstRefSliceCastable`. +template +const Result& SliceCast(const T& value, ConstRefSliceCastable = {}) { + // Insist upon sizes being equal to catch mismatches. + // We assume if sizes are opted in and sizes are equal then yes, these two + // types are expected to be layout compatible and actually appear to be. + static_assert(sizeof(Result) == sizeof(T), "size mismatch"); + return reinterpret_cast(value); +} + +} // namespace internal +} // namespace experimental +} // namespace grpc_event_engine + +#endif // GRPC_EVENT_ENGINE_INTERNAL_SLICE_CAST_H diff --git a/include/grpc/event_engine/slice.h b/include/grpc/event_engine/slice.h index 515cdfdcfd9..1e296a21876 100644 --- a/include/grpc/event_engine/slice.h +++ b/include/grpc/event_engine/slice.h @@ -25,6 +25,7 @@ #include "absl/strings/string_view.h" +#include #include #include @@ -212,8 +213,9 @@ class MutableSlice : public slice_detail::BaseSlice, uint8_t& operator[](size_t i) { return mutable_data()[i]; } }; -class Slice : public slice_detail::BaseSlice, - public slice_detail::CopyConstructors { +class GPR_MSVC_EMPTY_BASE_CLASS_WORKAROUND Slice + : public slice_detail::BaseSlice, + public slice_detail::CopyConstructors { public: Slice() = default; ~Slice(); @@ -280,6 +282,13 @@ class Slice : public slice_detail::BaseSlice, const uint8_t* begin, const uint8_t* end); }; +namespace internal { +template <> +struct ConstRefSliceCastable {}; +template <> +struct ConstRefSliceCastable {}; +} // namespace internal + } // namespace experimental } // namespace grpc_event_engine diff --git a/include/grpc/event_engine/slice_buffer.h b/include/grpc/event_engine/slice_buffer.h index 3df3394073c..d2a8ecd81a3 100644 --- a/include/grpc/event_engine/slice_buffer.h +++ b/include/grpc/event_engine/slice_buffer.h @@ -25,7 +25,9 @@ #include "absl/strings/string_view.h" #include "absl/utility/utility.h" +#include #include +#include #include #include #include @@ -117,6 +119,10 @@ class SliceBuffer { /// associated slice. Slice RefSlice(size_t index); + const Slice& operator[](size_t index) const { + return internal::SliceCast(slice_buffer_.slices[index]); + } + /// The total number of bytes held by the SliceBuffer size_t Length() { return slice_buffer_.length; } diff --git a/include/grpc/support/port_platform.h b/include/grpc/support/port_platform.h index 9c9244ce626..9f9cac0054f 100644 --- a/include/grpc/support/port_platform.h +++ b/include/grpc/support/port_platform.h @@ -761,6 +761,19 @@ extern void gpr_unreachable_code(const char* reason, const char* file, #define __STDC_FORMAT_MACROS #endif +/* MSVC doesn't do the empty base class optimization in debug builds by default, + * and because of ABI likely won't. + * This enables it for specific types, use as: + * class GPR_MSVC_EMPTY_BASE_CLASS_WORKAROUND Foo : public A, public B, public C + * {}; */ +#ifndef GPR_MSVC_EMPTY_BASE_CLASS_WORKAROUND +#ifdef GPR_WINDOWS +#define GPR_MSVC_EMPTY_BASE_CLASS_WORKAROUND __declspec(empty_bases) +#else +#define GPR_MSVC_EMPTY_BASE_CLASS_WORKAROUND +#endif +#endif + #define GRPC_CALLBACK_API_NONEXPERIMENTAL /* clang 11 with msan miscompiles destruction of [[no_unique_address]] members diff --git a/package.xml b/package.xml index 81c05fa6fa8..9a58ea75f2f 100644 --- a/package.xml +++ b/package.xml @@ -36,6 +36,7 @@ + diff --git a/src/core/BUILD b/src/core/BUILD index 04ab8a692ba..f779a7ef566 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -44,6 +44,13 @@ grpc_cc_library( language = "c++", ) +grpc_cc_library( + name = "slice_cast", + hdrs = [ + "//:include/grpc/event_engine/internal/slice_cast.h", + ], +) + grpc_cc_library( name = "event_engine_common", srcs = [ @@ -65,6 +72,7 @@ grpc_cc_library( deps = [ "slice", "slice_buffer", + "slice_cast", "slice_refcount", "//:event_engine_base_hdrs", "//:gpr", @@ -1109,6 +1117,7 @@ grpc_cc_library( ], visibility = ["@grpc:alt_grpc_base_legacy"], deps = [ + "slice_cast", "slice_refcount", "//:event_engine_base_hdrs", "//:gpr", diff --git a/src/core/lib/slice/slice.h b/src/core/lib/slice/slice.h index f95d0d87f1d..1199a7af2f7 100644 --- a/src/core/lib/slice/slice.h +++ b/src/core/lib/slice/slice.h @@ -25,6 +25,8 @@ #include "absl/strings/string_view.h" +#include +#include #include #include @@ -295,9 +297,10 @@ class MutableSlice : public slice_detail::BaseSlice, uint8_t& operator[](size_t i) { return mutable_data()[i]; } }; -class Slice : public slice_detail::BaseSlice, - public slice_detail::CopyConstructors, - public slice_detail::StaticConstructors { +class GPR_MSVC_EMPTY_BASE_CLASS_WORKAROUND Slice + : public slice_detail::BaseSlice, + public slice_detail::CopyConstructors, + public slice_detail::StaticConstructors { public: Slice() = default; ~Slice() { CSliceUnref(c_slice()); } @@ -403,4 +406,19 @@ class Slice : public slice_detail::BaseSlice, } // namespace grpc_core +namespace grpc_event_engine { +namespace experimental { +namespace internal { +template <> +struct ConstRefSliceCastable {}; +template <> +struct ConstRefSliceCastable {}; +template <> +struct ConstRefSliceCastable {}; +template <> +struct ConstRefSliceCastable {}; +} // namespace internal +} // namespace experimental +} // namespace grpc_event_engine + #endif // GRPC_CORE_LIB_SLICE_SLICE_H diff --git a/src/core/lib/slice/slice_buffer.h b/src/core/lib/slice/slice_buffer.h index e40246a8455..707247e983f 100644 --- a/src/core/lib/slice/slice_buffer.h +++ b/src/core/lib/slice/slice_buffer.h @@ -108,6 +108,11 @@ class SliceBuffer { /// associated slice. Slice RefSlice(size_t index) const; + const Slice& operator[](size_t index) const { + return grpc_event_engine::experimental::internal::SliceCast( + slice_buffer_.slices[index]); + } + /// The total number of bytes held by the SliceBuffer size_t Length() const { return slice_buffer_.length; } diff --git a/test/core/slice/slice_test.cc b/test/core/slice/slice_test.cc index 5e580b8ba60..0e216aec5cc 100644 --- a/test/core/slice/slice_test.cc +++ b/test/core/slice/slice_test.cc @@ -432,6 +432,15 @@ TEST(SliceTest, LetsGetMutable) { EXPECT_EQ(slice.as_string_view(), "ifnmp"); } +TEST(SliceTest, SliceCastWorks) { + using ::grpc_event_engine::experimental::internal::SliceCast; + Slice test = Slice::FromCopiedString("hello world!"); + const grpc_slice& slice = SliceCast(test); + EXPECT_EQ(&slice, &test.c_slice()); + const Slice& other = SliceCast(slice); + EXPECT_EQ(&other, &test); +} + } // namespace } // namespace grpc_core diff --git a/tools/buildgen/plugins/list_api.py b/tools/buildgen/plugins/list_api.py index da5ae56126a..63a635a909e 100755 --- a/tools/buildgen/plugins/list_api.py +++ b/tools/buildgen/plugins/list_api.py @@ -22,7 +22,7 @@ import sys import yaml -_RE_API = r'(?:GPRAPI|GRPCAPI|CENSUSAPI)([^;]*);' +_RE_API = r'(?:GPRAPI|GRPCAPI|CENSUSAPI)([^#;]*);' def list_c_apis(filenames): diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index a06e4cb11f9..5f4897627b8 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -882,6 +882,7 @@ include/grpc/compression.h \ include/grpc/event_engine/endpoint_config.h \ include/grpc/event_engine/event_engine.h \ include/grpc/event_engine/internal/memory_allocator_impl.h \ +include/grpc/event_engine/internal/slice_cast.h \ include/grpc/event_engine/memory_allocator.h \ include/grpc/event_engine/memory_request.h \ include/grpc/event_engine/port.h \ diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index a0482b3476f..820db7a49b6 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -882,6 +882,7 @@ include/grpc/compression.h \ include/grpc/event_engine/endpoint_config.h \ include/grpc/event_engine/event_engine.h \ include/grpc/event_engine/internal/memory_allocator_impl.h \ +include/grpc/event_engine/internal/slice_cast.h \ include/grpc/event_engine/memory_allocator.h \ include/grpc/event_engine/memory_request.h \ include/grpc/event_engine/port.h \ diff --git a/tools/doxygen/Doxyfile.core b/tools/doxygen/Doxyfile.core index 69cf9547508..aee1b329959 100644 --- a/tools/doxygen/Doxyfile.core +++ b/tools/doxygen/Doxyfile.core @@ -814,6 +814,7 @@ include/grpc/compression.h \ include/grpc/event_engine/endpoint_config.h \ include/grpc/event_engine/event_engine.h \ include/grpc/event_engine/internal/memory_allocator_impl.h \ +include/grpc/event_engine/internal/slice_cast.h \ include/grpc/event_engine/memory_allocator.h \ include/grpc/event_engine/memory_request.h \ include/grpc/event_engine/port.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index aa12755a652..c758dc95a0f 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -814,6 +814,7 @@ include/grpc/compression.h \ include/grpc/event_engine/endpoint_config.h \ include/grpc/event_engine/event_engine.h \ include/grpc/event_engine/internal/memory_allocator_impl.h \ +include/grpc/event_engine/internal/slice_cast.h \ include/grpc/event_engine/memory_allocator.h \ include/grpc/event_engine/memory_request.h \ include/grpc/event_engine/port.h \