diff --git a/BUILD b/BUILD index bcee657a33c..a0159cd8a3e 100644 --- a/BUILD +++ b/BUILD @@ -1573,7 +1573,6 @@ grpc_cc_library( hdrs = [ "src/core/lib/slice/slice_refcount.h", "src/core/lib/slice/slice_refcount_base.h", - "src/core/lib/slice/slice_utils.h", ], public_hdrs = [ "include/grpc/slice.h", @@ -1887,7 +1886,6 @@ grpc_cc_library( "src/core/lib/slice/percent_encoding.cc", "src/core/lib/slice/slice_api.cc", "src/core/lib/slice/slice_buffer.cc", - "src/core/lib/slice/slice_intern.cc", "src/core/lib/slice/slice_split.cc", "src/core/lib/surface/api_trace.cc", "src/core/lib/surface/builtins.cc", diff --git a/CMakeLists.txt b/CMakeLists.txt index 5bb95a35a76..320e209fedd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -735,7 +735,6 @@ if(gRPC_BUILD_TESTS) endif() add_dependencies(buildtests_c server_test) add_dependencies(buildtests_c slice_buffer_test) - add_dependencies(buildtests_c slice_intern_test) add_dependencies(buildtests_c slice_split_test) add_dependencies(buildtests_c slice_string_helpers_test) add_dependencies(buildtests_c sockaddr_resolver_test) @@ -2083,7 +2082,6 @@ add_library(grpc src/core/lib/slice/slice.cc src/core/lib/slice/slice_api.cc src/core/lib/slice/slice_buffer.cc - src/core/lib/slice/slice_intern.cc src/core/lib/slice/slice_refcount.cc src/core/lib/slice/slice_split.cc src/core/lib/slice/slice_string_helpers.cc @@ -2669,7 +2667,6 @@ add_library(grpc_unsecure src/core/lib/slice/slice.cc src/core/lib/slice/slice_api.cc src/core/lib/slice/slice_buffer.cc - src/core/lib/slice/slice_intern.cc src/core/lib/slice/slice_refcount.cc src/core/lib/slice/slice_split.cc src/core/lib/slice/slice_string_helpers.cc @@ -6708,33 +6705,6 @@ target_link_libraries(slice_buffer_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(slice_intern_test - test/core/slice/slice_intern_test.cc -) - -target_include_directories(slice_intern_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} -) - -target_link_libraries(slice_intern_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) diff --git a/Makefile b/Makefile index 59910ccdb0e..740f0fb05db 100644 --- a/Makefile +++ b/Makefile @@ -1566,7 +1566,6 @@ LIBGRPC_SRC = \ src/core/lib/slice/slice.cc \ src/core/lib/slice/slice_api.cc \ src/core/lib/slice/slice_buffer.cc \ - src/core/lib/slice/slice_intern.cc \ src/core/lib/slice/slice_refcount.cc \ src/core/lib/slice/slice_split.cc \ src/core/lib/slice/slice_string_helpers.cc \ @@ -1999,7 +1998,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/slice/slice.cc \ src/core/lib/slice/slice_api.cc \ src/core/lib/slice/slice_buffer.cc \ - src/core/lib/slice/slice_intern.cc \ src/core/lib/slice/slice_refcount.cc \ src/core/lib/slice/slice_split.cc \ src/core/lib/slice/slice_string_helpers.cc \ diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index fffd1f8301f..48433be2e88 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -932,7 +932,6 @@ libs: - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_split.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h - src/core/lib/surface/api_trace.h - src/core/lib/surface/builtins.h - src/core/lib/surface/call.h @@ -1534,7 +1533,6 @@ libs: - src/core/lib/slice/slice.cc - src/core/lib/slice/slice_api.cc - src/core/lib/slice/slice_buffer.cc - - src/core/lib/slice/slice_intern.cc - src/core/lib/slice/slice_refcount.cc - src/core/lib/slice/slice_split.cc - src/core/lib/slice/slice_string_helpers.cc @@ -1991,7 +1989,6 @@ libs: - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_split.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h - src/core/lib/surface/api_trace.h - src/core/lib/surface/builtins.h - src/core/lib/surface/call.h @@ -2268,7 +2265,6 @@ libs: - src/core/lib/slice/slice.cc - src/core/lib/slice/slice_api.cc - src/core/lib/slice/slice_buffer.cc - - src/core/lib/slice/slice_intern.cc - src/core/lib/slice/slice_refcount.cc - src/core/lib/slice/slice_split.cc - src/core/lib/slice/slice_string_helpers.cc @@ -3770,7 +3766,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h src: - src/core/lib/debug/trace.cc - src/core/lib/event_engine/memory_allocator.cc @@ -4021,15 +4016,6 @@ targets: deps: - grpc_test_util uses_polling: false -- name: slice_intern_test - build: test - language: c - headers: [] - src: - - test/core/slice/slice_intern_test.cc - deps: - - grpc_test_util - uses_polling: false - name: slice_split_test build: test language: c @@ -4052,7 +4038,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h src: - src/core/lib/debug/trace.cc - src/core/lib/slice/slice.cc @@ -4530,7 +4515,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h src: - src/core/lib/debug/trace.cc - src/core/lib/event_engine/memory_allocator.cc @@ -5023,7 +5007,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h src: - src/core/lib/debug/trace.cc - src/core/lib/event_engine/memory_allocator.cc @@ -5656,7 +5639,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h src: - src/core/lib/debug/trace.cc - src/core/lib/iomgr/combiner.cc @@ -5866,7 +5848,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h - test/core/promise/test_wakeup_schedulers.h src: - src/core/lib/debug/trace.cc @@ -6631,7 +6612,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h - test/core/resource_quota/call_checker.h src: - src/core/lib/debug/trace.cc @@ -6937,7 +6917,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h - test/core/promise/test_wakeup_schedulers.h src: - src/core/lib/debug/trace.cc @@ -7290,7 +7269,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h src: - src/core/lib/debug/trace.cc - src/core/lib/event_engine/memory_allocator.cc @@ -7727,7 +7705,6 @@ targets: - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h - src/core/lib/slice/slice_string_helpers.h - - src/core/lib/slice/slice_utils.h - test/core/util/build.h src: - src/core/lib/debug/trace.cc diff --git a/config.m4 b/config.m4 index e80b9a7cbc7..743428dc50e 100644 --- a/config.m4 +++ b/config.m4 @@ -628,7 +628,6 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/slice/slice.cc \ src/core/lib/slice/slice_api.cc \ src/core/lib/slice/slice_buffer.cc \ - src/core/lib/slice/slice_intern.cc \ src/core/lib/slice/slice_refcount.cc \ src/core/lib/slice/slice_split.cc \ src/core/lib/slice/slice_string_helpers.cc \ diff --git a/config.w32 b/config.w32 index bee239f8bb6..c16e37724f5 100644 --- a/config.w32 +++ b/config.w32 @@ -594,7 +594,6 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\slice\\slice.cc " + "src\\core\\lib\\slice\\slice_api.cc " + "src\\core\\lib\\slice\\slice_buffer.cc " + - "src\\core\\lib\\slice\\slice_intern.cc " + "src\\core\\lib\\slice\\slice_refcount.cc " + "src\\core\\lib\\slice\\slice_split.cc " + "src\\core\\lib\\slice\\slice_string_helpers.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index ff930718a5c..08b706e5d31 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -805,7 +805,6 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice_refcount_base.h', 'src/core/lib/slice/slice_split.h', 'src/core/lib/slice/slice_string_helpers.h', - 'src/core/lib/slice/slice_utils.h', 'src/core/lib/surface/api_trace.h', 'src/core/lib/surface/builtins.h', 'src/core/lib/surface/call.h', @@ -1539,7 +1538,6 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice_refcount_base.h', 'src/core/lib/slice/slice_split.h', 'src/core/lib/slice/slice_string_helpers.h', - 'src/core/lib/slice/slice_utils.h', 'src/core/lib/surface/api_trace.h', 'src/core/lib/surface/builtins.h', 'src/core/lib/surface/call.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 379ef662938..4058ada3151 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1327,7 +1327,6 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice.h', 'src/core/lib/slice/slice_api.cc', 'src/core/lib/slice/slice_buffer.cc', - 'src/core/lib/slice/slice_intern.cc', 'src/core/lib/slice/slice_internal.h', 'src/core/lib/slice/slice_refcount.cc', 'src/core/lib/slice/slice_refcount.h', @@ -1336,7 +1335,6 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice_split.h', 'src/core/lib/slice/slice_string_helpers.cc', 'src/core/lib/slice/slice_string_helpers.h', - 'src/core/lib/slice/slice_utils.h', 'src/core/lib/surface/api_trace.cc', 'src/core/lib/surface/api_trace.h', 'src/core/lib/surface/builtins.cc', @@ -2082,7 +2080,6 @@ Pod::Spec.new do |s| 'src/core/lib/slice/slice_refcount_base.h', 'src/core/lib/slice/slice_split.h', 'src/core/lib/slice/slice_string_helpers.h', - 'src/core/lib/slice/slice_utils.h', 'src/core/lib/surface/api_trace.h', 'src/core/lib/surface/builtins.h', 'src/core/lib/surface/call.h', diff --git a/grpc.def b/grpc.def index 629087f7c5d..e1e7f289302 100644 --- a/grpc.def +++ b/grpc.def @@ -190,7 +190,6 @@ EXPORTS grpc_slice_new_with_len grpc_slice_malloc grpc_slice_malloc_large - grpc_slice_intern grpc_slice_from_copied_string grpc_slice_from_copied_buffer grpc_slice_from_static_string @@ -201,8 +200,6 @@ EXPORTS grpc_slice_split_tail_maybe_ref grpc_slice_split_head grpc_empty_slice - grpc_slice_default_hash_impl - grpc_slice_default_eq_impl grpc_slice_eq grpc_slice_cmp grpc_slice_str_cmp @@ -210,7 +207,6 @@ EXPORTS grpc_slice_rchr grpc_slice_chr grpc_slice_slice - grpc_slice_hash grpc_slice_is_equivalent grpc_slice_dup grpc_slice_to_c_string diff --git a/grpc.gemspec b/grpc.gemspec index 8f526205b95..59eeb725d4a 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1246,7 +1246,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/slice/slice.h ) s.files += %w( src/core/lib/slice/slice_api.cc ) s.files += %w( src/core/lib/slice/slice_buffer.cc ) - s.files += %w( src/core/lib/slice/slice_intern.cc ) s.files += %w( src/core/lib/slice/slice_internal.h ) s.files += %w( src/core/lib/slice/slice_refcount.cc ) s.files += %w( src/core/lib/slice/slice_refcount.h ) @@ -1255,7 +1254,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/slice/slice_split.h ) s.files += %w( src/core/lib/slice/slice_string_helpers.cc ) s.files += %w( src/core/lib/slice/slice_string_helpers.h ) - s.files += %w( src/core/lib/slice/slice_utils.h ) s.files += %w( src/core/lib/surface/api_trace.cc ) s.files += %w( src/core/lib/surface/api_trace.h ) s.files += %w( src/core/lib/surface/builtins.cc ) diff --git a/grpc.gyp b/grpc.gyp index b6e21d846c3..649f7723c72 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -1014,7 +1014,6 @@ 'src/core/lib/slice/slice.cc', 'src/core/lib/slice/slice_api.cc', 'src/core/lib/slice/slice_buffer.cc', - 'src/core/lib/slice/slice_intern.cc', 'src/core/lib/slice/slice_refcount.cc', 'src/core/lib/slice/slice_split.cc', 'src/core/lib/slice/slice_string_helpers.cc', @@ -1419,7 +1418,6 @@ 'src/core/lib/slice/slice.cc', 'src/core/lib/slice/slice_api.cc', 'src/core/lib/slice/slice_buffer.cc', - 'src/core/lib/slice/slice_intern.cc', 'src/core/lib/slice/slice_refcount.cc', 'src/core/lib/slice/slice_split.cc', 'src/core/lib/slice/slice_string_helpers.cc', diff --git a/include/grpc/impl/codegen/slice.h b/include/grpc/impl/codegen/slice.h index 4412058104e..130e5efffd2 100644 --- a/include/grpc/impl/codegen/slice.h +++ b/include/grpc/impl/codegen/slice.h @@ -58,7 +58,10 @@ struct grpc_slice_refcount; Multiple grpc_slice values may share a ref count. If the slice does not have a refcount, it represents an inlined small piece - of data that is copied by value. */ + of data that is copied by value. + + As a special case, a slice can be given refcount == uintptr_t(1), meaning + that the slice represents external data that is not refcounted. */ struct grpc_slice { struct grpc_slice_refcount* refcount; union grpc_slice_data { diff --git a/include/grpc/slice.h b/include/grpc/slice.h index 65d20878831..15978fe54c5 100644 --- a/include/grpc/slice.h +++ b/include/grpc/slice.h @@ -69,12 +69,6 @@ GPRAPI grpc_slice grpc_slice_malloc_large(size_t length); #define GRPC_SLICE_MALLOC(len) grpc_slice_malloc(len) -/** Intern a slice: - - The return value for two invocations of this function with the same sequence - of bytes is a slice which points to the same memory. */ -GPRAPI grpc_slice grpc_slice_intern(grpc_slice slice); - /** Create a slice by copying a string. Does not preserve null terminators. Equivalent to: @@ -129,9 +123,6 @@ GPRAPI grpc_slice grpc_slice_split_head(grpc_slice* s, size_t split); GPRAPI grpc_slice grpc_empty_slice(void); -GPRAPI uint32_t grpc_slice_default_hash_impl(grpc_slice s); -GPRAPI int grpc_slice_default_eq_impl(grpc_slice a, grpc_slice b); - GPRAPI int grpc_slice_eq(grpc_slice a, grpc_slice b); /** Returns <0 if a < b, ==0 if a == b, >0 if a > b @@ -151,8 +142,6 @@ GPRAPI int grpc_slice_chr(grpc_slice s, char c); if it's not found */ GPRAPI int grpc_slice_slice(grpc_slice haystack, grpc_slice needle); -GPRAPI uint32_t grpc_slice_hash(grpc_slice s); - /** Do two slices point at the same memory, with the same length If a or b is inlined, actually compares data */ GPRAPI int grpc_slice_is_equivalent(grpc_slice a, grpc_slice b); diff --git a/package.xml b/package.xml index edf8715afbe..1cd14b1714e 100644 --- a/package.xml +++ b/package.xml @@ -1226,7 +1226,6 @@ - @@ -1235,7 +1234,6 @@ - diff --git a/src/core/ext/transport/binder/transport/binder_transport.cc b/src/core/ext/transport/binder/transport/binder_transport.cc index 964d6bef881..3206bec8e7f 100644 --- a/src/core/ext/transport/binder/transport/binder_transport.cc +++ b/src/core/ext/transport/binder/transport/binder_transport.cc @@ -36,7 +36,7 @@ #include "src/core/ext/transport/binder/wire_format/wire_reader_impl.h" #include "src/core/ext/transport/binder/wire_format/wire_writer.h" #include "src/core/lib/iomgr/exec_ctx.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/transport/byte_stream.h" #include "src/core/lib/transport/error_utils.h" #include "src/core/lib/transport/metadata_batch.h" diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index 18abbdacde7..cb17759bba5 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -647,17 +647,6 @@ class HPackParser::Input { // Helper to parse a string and turn it into a slice with appropriate memory // management characteristics class HPackParser::String { - public: - // Helper to specify a string should be internalized - struct Intern {}; - // Helper to specify a string should be externalized - struct Extern {}; - - private: - // Forward declare take functions... we'll need them in the public interface - Slice Take(Extern); - Slice Take(Intern); - public: String(const String&) = delete; String& operator=(const String&) = delete; @@ -671,11 +660,7 @@ class HPackParser::String { } // Take the value and leave this empty - // Use Intern/Extern to choose memory management - template - auto Take() -> decltype(this->Take(T())) { - return Take(T()); - } + Slice Take(); // Return a reference to the value as a string view absl::string_view string_view() const { @@ -962,13 +947,11 @@ class HPackParser::Parser { case 1: switch (cur & 0xf) { case 0: // literal key - return FinishHeaderOmitFromTable(ParseLiteralKey()); + return FinishHeaderOmitFromTable(ParseLiteralKey()); case 0xf: // varint encoded key index - return FinishHeaderOmitFromTable( - ParseVarIdxKey(0xf)); + return FinishHeaderOmitFromTable(ParseVarIdxKey(0xf)); default: // inline encoded key index - return FinishHeaderOmitFromTable( - ParseIdxKey(cur & 0xf)); + return FinishHeaderOmitFromTable(ParseIdxKey(cur & 0xf)); } // Update max table size. // First byte format: 001xxxxx @@ -995,23 +978,20 @@ class HPackParser::Parser { case 4: if (cur == 0x40) { // literal key - return FinishHeaderAndAddToTable(ParseLiteralKey()); + return FinishHeaderAndAddToTable(ParseLiteralKey()); } ABSL_FALLTHROUGH_INTENDED; case 5: case 6: // inline encoded key index - return FinishHeaderAndAddToTable( - ParseIdxKey(cur & 0x3f)); + return FinishHeaderAndAddToTable(ParseIdxKey(cur & 0x3f)); case 7: if (cur == 0x7f) { // varint encoded key index - return FinishHeaderAndAddToTable( - ParseVarIdxKey(0x3f)); + return FinishHeaderAndAddToTable(ParseVarIdxKey(0x3f)); } else { // inline encoded key index - return FinishHeaderAndAddToTable( - ParseIdxKey(cur & 0x3f)); + return FinishHeaderAndAddToTable(ParseIdxKey(cur & 0x3f)); } // Indexed Header Field Representation // First byte format: 1xxxxxxx @@ -1113,7 +1093,6 @@ class HPackParser::Parser { } // Parse a string encoded key and a string encoded value - template absl::optional ParseLiteralKey() { auto key = String::Parse(input_); if (!key.has_value()) return {}; @@ -1122,7 +1101,7 @@ class HPackParser::Parser { return {}; } auto key_string = key->string_view(); - auto value_slice = value->Take(); + auto value_slice = value->Take(); const auto transport_size = key_string.size() + value_slice.size() + hpack_constants::kEntryOverhead; return grpc_metadata_batch::Parse( @@ -1133,7 +1112,6 @@ class HPackParser::Parser { } // Parse an index encoded key and a string encoded value - template absl::optional ParseIdxKey(uint32_t index) { const auto* elem = table_->Lookup(index); if (GPR_UNLIKELY(elem == nullptr)) { @@ -1142,19 +1120,17 @@ class HPackParser::Parser { } auto value = ParseValueString(elem->is_binary_header()); if (GPR_UNLIKELY(!value.has_value())) return {}; - return elem->WithNewValue(value->Take(), - [=](absl::string_view error, const Slice& value) { - ReportMetadataParseError( - elem->key(), error, value.as_string_view()); - }); + return elem->WithNewValue( + value->Take(), [=](absl::string_view error, const Slice& value) { + ReportMetadataParseError(elem->key(), error, value.as_string_view()); + }); } // Parse a varint index encoded key and a string encoded value - template absl::optional ParseVarIdxKey(uint32_t offset) { auto index = input_->ParseVarint(offset); if (GPR_UNLIKELY(!index.has_value())) return {}; - return ParseIdxKey(*index); + return ParseIdxKey(*index); } // Parse a string, figuring out if it's binary or not by the key name. @@ -1250,7 +1226,7 @@ class HPackParser::Parser { const LogInfo log_info_; }; -Slice HPackParser::String::Take(Extern) { +Slice HPackParser::String::Take() { if (auto* p = absl::get_if(&value_)) { return p->Copy(); } else if (auto* p = absl::get_if>(&value_)) { @@ -1261,18 +1237,6 @@ Slice HPackParser::String::Take(Extern) { GPR_UNREACHABLE_CODE(return Slice()); } -Slice HPackParser::String::Take(Intern) { - ManagedMemorySlice m; - if (auto* p = absl::get_if(&value_)) { - m = ManagedMemorySlice(&p->c_slice()); - } else if (auto* p = absl::get_if>(&value_)) { - m = ManagedMemorySlice(reinterpret_cast(p->data()), p->size()); - } else if (auto* p = absl::get_if>(&value_)) { - m = ManagedMemorySlice(reinterpret_cast(p->data()), p->size()); - } - return Slice(m); -} - /* PUBLIC INTERFACE */ HPackParser::HPackParser() = default; diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc index 63dbd8e5824..d05f27b98c5 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.cc +++ b/src/core/ext/transport/chttp2/transport/parsing.cc @@ -28,8 +28,8 @@ #include "src/core/ext/transport/chttp2/transport/internal.h" #include "src/core/lib/profiling/timers.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" -#include "src/core/lib/slice/slice_utils.h" #include "src/core/lib/transport/http2_errors.h" #include "src/core/lib/transport/status_conversion.h" #include "src/core/lib/transport/timeout_encoding.h" diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index e82217e0d44..f25d8e0f5a7 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -410,11 +410,10 @@ static void convert_cronet_array_to_metadata( grpc_slice value; if (absl::EndsWith(header_array->headers[i].key, "-bin")) { value = grpc_slice_from_static_string(header_array->headers[i].value); - value = grpc_slice_intern(grpc_chttp2_base64_decode_with_length( - value, grpc_chttp2_base64_infer_length_after_decode(value))); + value = grpc_chttp2_base64_decode_with_length( + value, grpc_chttp2_base64_infer_length_after_decode(value)); } else { - value = grpc_slice_intern( - grpc_slice_from_static_string(header_array->headers[i].value)); + value = grpc_slice_from_static_string(header_array->headers[i].value); } mds->Append(header_array->headers[i].key, grpc_core::Slice(value), [&](absl::string_view error, const grpc_core::Slice& value) { diff --git a/src/core/ext/xds/xds_api.cc b/src/core/ext/xds/xds_api.cc index 6f885e18b4e..d9cc5b8828a 100644 --- a/src/core/ext/xds/xds_api.cc +++ b/src/core/ext/xds/xds_api.cc @@ -57,7 +57,7 @@ #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/socket_utils.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/uri/uri_parser.h" namespace grpc_core { diff --git a/src/core/ext/xds/xds_server_config_fetcher.cc b/src/core/ext/xds/xds_server_config_fetcher.cc index 699ba1d2549..e78c35e4a24 100644 --- a/src/core/ext/xds/xds_server_config_fetcher.cc +++ b/src/core/ext/xds/xds_server_config_fetcher.cc @@ -37,7 +37,7 @@ #include "src/core/lib/iomgr/sockaddr.h" #include "src/core/lib/iomgr/socket_utils.h" #include "src/core/lib/security/credentials/xds/xds_credentials.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/server.h" #include "src/core/lib/transport/error_utils.h" diff --git a/src/core/lib/compression/compression.cc b/src/core/lib/compression/compression.cc index ba86dbd48ac..ce39d7e8343 100644 --- a/src/core/lib/compression/compression.cc +++ b/src/core/lib/compression/compression.cc @@ -25,7 +25,7 @@ #include "src/core/lib/compression/compression_internal.h" #include "src/core/lib/gpr/useful.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/surface/api_trace.h" int grpc_compression_algorithm_is_message(grpc_compression_algorithm) { diff --git a/src/core/lib/compression/compression_internal.cc b/src/core/lib/compression/compression_internal.cc index 3ca36af31b8..01dd6c75218 100644 --- a/src/core/lib/compression/compression_internal.cc +++ b/src/core/lib/compression/compression_internal.cc @@ -33,7 +33,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gpr/useful.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/surface/api_trace.h" namespace grpc_core { diff --git a/src/core/lib/event_engine/memory_allocator.cc b/src/core/lib/event_engine/memory_allocator.cc index 8b6994e76d8..afb056ecd7b 100644 --- a/src/core/lib/event_engine/memory_allocator.cc +++ b/src/core/lib/event_engine/memory_allocator.cc @@ -26,28 +26,24 @@ namespace { // Reference count for a slice allocated by MemoryAllocator::MakeSlice. // Takes care of releasing memory back when the slice is destroyed. -class SliceRefCount { +class SliceRefCount : public grpc_slice_refcount { public: - static void Destroy(void* p) { - auto* rc = static_cast(p); - rc->~SliceRefCount(); - gpr_free(rc); - } SliceRefCount(std::shared_ptr allocator, size_t size) - : base_(grpc_slice_refcount::Type::REGULAR, &refs_, Destroy, this, - &base_), + : grpc_slice_refcount(Destroy), allocator_(std::move(allocator)), size_(size) { // Nothing to do here. } ~SliceRefCount() { allocator_->Release(size_); } - grpc_slice_refcount* base_refcount() { return &base_; } - private: - grpc_slice_refcount base_; - std::atomic refs_{1}; + static void Destroy(grpc_slice_refcount* p) { + auto* rc = static_cast(p); + rc->~SliceRefCount(); + gpr_free(rc); + } + std::shared_ptr allocator_; size_t size_; }; @@ -59,7 +55,7 @@ grpc_slice MemoryAllocator::MakeSlice(MemoryRequest request) { void* p = gpr_malloc(size); new (p) SliceRefCount(allocator_, size); grpc_slice slice; - slice.refcount = static_cast(p)->base_refcount(); + slice.refcount = static_cast(p); slice.data.refcounted.bytes = static_cast(p) + sizeof(SliceRefCount); slice.data.refcounted.length = size - sizeof(SliceRefCount); diff --git a/src/core/lib/iomgr/error.cc b/src/core/lib/iomgr/error.cc index f5147c62aac..a442f88f62e 100644 --- a/src/core/lib/iomgr/error.cc +++ b/src/core/lib/iomgr/error.cc @@ -35,7 +35,6 @@ #include "src/core/lib/gpr/useful.h" #include "src/core/lib/iomgr/error_internal.h" #include "src/core/lib/slice/slice_internal.h" -#include "src/core/lib/slice/slice_utils.h" grpc_core::DebugOnlyTraceFlag grpc_trace_error_refcount(false, "error_refcount"); diff --git a/src/core/lib/security/authorization/evaluate_args.cc b/src/core/lib/security/authorization/evaluate_args.cc index 8d84f45c513..1571d8be55a 100644 --- a/src/core/lib/security/authorization/evaluate_args.cc +++ b/src/core/lib/security/authorization/evaluate_args.cc @@ -22,7 +22,7 @@ #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/gprpp/host_port.h" #include "src/core/lib/security/credentials/tls/tls_utils.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" namespace grpc_core { diff --git a/src/core/lib/security/credentials/external/file_external_account_credentials.cc b/src/core/lib/security/credentials/external/file_external_account_credentials.cc index d596d29414c..a959e8c871f 100644 --- a/src/core/lib/security/credentials/external/file_external_account_credentials.cc +++ b/src/core/lib/security/credentials/external/file_external_account_credentials.cc @@ -21,7 +21,6 @@ #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/slice/slice_internal.h" -#include "src/core/lib/slice/slice_utils.h" namespace grpc_core { diff --git a/src/core/lib/slice/slice.cc b/src/core/lib/slice/slice.cc index e0c4cb038a0..2e78b4de9e6 100644 --- a/src/core/lib/slice/slice.cc +++ b/src/core/lib/slice/slice.cc @@ -18,6 +18,8 @@ #include +#include "src/core/lib/slice/slice.h" + #include #include @@ -27,6 +29,7 @@ #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/slice/slice_refcount_base.h" char* grpc_slice_to_c_string(grpc_slice slice) { char* out = static_cast(gpr_malloc(GRPC_SLICE_LENGTH(slice) + 1)); @@ -35,7 +38,9 @@ char* grpc_slice_to_c_string(grpc_slice slice) { return out; } -grpc_slice grpc_empty_slice(void) { return grpc_core::UnmanagedMemorySlice(); } +grpc_slice grpc_empty_slice(void) { + return grpc_core::slice_detail::EmptySlice(); +} grpc_slice grpc_slice_copy(grpc_slice s) { grpc_slice out = GRPC_SLICE_MALLOC(GRPC_SLICE_LENGTH(s)); @@ -46,30 +51,21 @@ grpc_slice grpc_slice_copy(grpc_slice s) { namespace grpc_core { -/* grpc_slice_from_static_string support structure - a refcount that does - nothing */ -grpc_slice_refcount kNoopRefcount(grpc_slice_refcount::Type::NOP); -static_assert(std::is_trivially_destructible::value, - "kNoopRefcount must be trivially destructible."); - /* grpc_slice_new support structures - we create a refcount object extended with the user provided data pointer & destroy function */ -class NewSliceRefcount { +class NewSliceRefcount : public grpc_slice_refcount { public: - static void Destroy(void* arg) { delete static_cast(arg); } - NewSliceRefcount(void (*destroy)(void*), void* user_data) - : base_(grpc_slice_refcount::Type::REGULAR, &refs_, Destroy, this, - &base_), + : grpc_slice_refcount(Destroy), user_destroy_(destroy), user_data_(user_data) {} ~NewSliceRefcount() { user_destroy_(user_data_); } - grpc_slice_refcount* base_refcount() { return &base_; } - private: - grpc_slice_refcount base_; - std::atomic refs_{1}; + static void Destroy(grpc_slice_refcount* arg) { + delete static_cast(arg); + } + void (*user_destroy_)(void*); void* user_data_; }; @@ -77,7 +73,8 @@ class NewSliceRefcount { } // namespace grpc_core size_t grpc_slice_memory_usage(grpc_slice s) { - if (s.refcount == nullptr || s.refcount == &grpc_core::kNoopRefcount) { + if (s.refcount == nullptr || + s.refcount == grpc_slice_refcount::NoopRefcount()) { return 0; } else { return s.data.refcounted.length; @@ -85,19 +82,18 @@ size_t grpc_slice_memory_usage(grpc_slice s) { } grpc_slice grpc_slice_from_static_buffer(const void* s, size_t len) { - return grpc_core::ExternallyManagedSlice(s, len); + return grpc_core::StaticSlice::FromStaticBuffer(s, len).TakeCSlice(); } grpc_slice grpc_slice_from_static_string(const char* s) { - return grpc_core::ExternallyManagedSlice(s, strlen(s)); + return grpc_core::StaticSlice::FromStaticString(s).TakeCSlice(); } grpc_slice grpc_slice_new_with_user_data(void* p, size_t len, void (*destroy)(void*), void* user_data) { grpc_slice slice; - slice.refcount = - (new grpc_core::NewSliceRefcount(destroy, user_data))->base_refcount(); + slice.refcount = new grpc_core::NewSliceRefcount(destroy, user_data); slice.data.refcounted.bytes = static_cast(p); slice.data.refcounted.length = len; return slice; @@ -111,68 +107,51 @@ grpc_slice grpc_slice_new(void* p, size_t len, void (*destroy)(void*)) { namespace grpc_core { /* grpc_slice_new_with_len support structures - we create a refcount object extended with the user provided data pointer & destroy function */ -class NewWithLenSliceRefcount { +class NewWithLenSliceRefcount : public grpc_slice_refcount { public: - static void Destroy(void* arg) { - delete static_cast(arg); - } - NewWithLenSliceRefcount(void (*destroy)(void*, size_t), void* user_data, size_t user_length) - : base_(grpc_slice_refcount::Type::REGULAR, &refs_, Destroy, this, - &base_), + : grpc_slice_refcount(Destroy), user_data_(user_data), user_length_(user_length), user_destroy_(destroy) {} ~NewWithLenSliceRefcount() { user_destroy_(user_data_, user_length_); } - grpc_slice_refcount* base_refcount() { return &base_; } - private: - grpc_slice_refcount base_; - std::atomic refs_{1}; + static void Destroy(grpc_slice_refcount* arg) { + delete static_cast(arg); + } + void* user_data_; size_t user_length_; void (*user_destroy_)(void*, size_t); }; /** grpc_slice_from_moved_(string|buffer) ref count .*/ -class MovedStringSliceRefCount { +class MovedStringSliceRefCount : public grpc_slice_refcount { public: explicit MovedStringSliceRefCount(UniquePtr&& str) - : base_(grpc_slice_refcount::Type::REGULAR, &refs_, Destroy, this, - &base_), - str_(std::move(str)) {} - - grpc_slice_refcount* base_refcount() { return &base_; } + : grpc_slice_refcount(Destroy), str_(std::move(str)) {} private: - static void Destroy(void* arg) { + static void Destroy(grpc_slice_refcount* arg) { delete static_cast(arg); } - grpc_slice_refcount base_; - std::atomic refs_{1}; UniquePtr str_; }; // grpc_slice_from_cpp_string() ref count. -class MovedCppStringSliceRefCount { +class MovedCppStringSliceRefCount : public grpc_slice_refcount { public: explicit MovedCppStringSliceRefCount(std::string&& str) - : base_(grpc_slice_refcount::Type::REGULAR, &refs_, Destroy, this, - &base_), - str_(std::move(str)) {} - - grpc_slice_refcount* base_refcount() { return &base_; } + : grpc_slice_refcount(Destroy), str_(std::move(str)) {} private: - static void Destroy(void* arg) { + static void Destroy(grpc_slice_refcount* arg) { delete static_cast(arg); } - grpc_slice_refcount base_; - std::atomic refs_{1}; std::string str_; }; @@ -181,44 +160,17 @@ class MovedCppStringSliceRefCount { grpc_slice grpc_slice_new_with_len(void* p, size_t len, void (*destroy)(void*, size_t)) { grpc_slice slice; - slice.refcount = (new grpc_core::NewWithLenSliceRefcount(destroy, p, len)) - ->base_refcount(); + slice.refcount = new grpc_core::NewWithLenSliceRefcount(destroy, p, len); slice.data.refcounted.bytes = static_cast(p); slice.data.refcounted.length = len; return slice; } -grpc_core::UnmanagedMemorySlice::UnmanagedMemorySlice(const char* source, - size_t length) { - if (length <= sizeof(data.inlined.bytes)) { - refcount = nullptr; - data.inlined.length = static_cast(length); - } else { - HeapInit(length); - } - if (length > 0) { - memcpy(GRPC_SLICE_START_PTR(*this), source, length); - } -} - -grpc_core::UnmanagedMemorySlice::UnmanagedMemorySlice(const char* source) - : grpc_core::UnmanagedMemorySlice::UnmanagedMemorySlice(source, - strlen(source)) {} - -grpc_slice grpc_slice_from_copied_buffer(const char* source, size_t length) { - grpc_slice slice; - if (length <= sizeof(slice.data.inlined.bytes)) { - slice.refcount = nullptr; - slice.data.inlined.length = length; - } else { - // Create a ref-counted slice. - slice = grpc_core::UnmanagedMemorySlice( - length, grpc_core::UnmanagedMemorySlice::ForceHeapAllocation()); - } - if (length > 0) { - memcpy(GRPC_SLICE_START_PTR(slice), source, length); - } - return slice; +grpc_slice grpc_slice_from_copied_buffer(const char* source, size_t len) { + if (len == 0) return grpc_empty_slice(); + grpc_slice out = grpc_slice_malloc(len); + memcpy(GRPC_SLICE_START_PTR(out), source, len); + return out; } grpc_slice grpc_slice_from_copied_string(const char* source) { @@ -234,8 +186,7 @@ grpc_slice grpc_slice_from_moved_buffer(grpc_core::UniquePtr p, slice.data.inlined.length = len; memcpy(GRPC_SLICE_START_PTR(slice), ptr, len); } else { - slice.refcount = (new grpc_core::MovedStringSliceRefCount(std::move(p))) - ->base_refcount(); + slice.refcount = new grpc_core::MovedStringSliceRefCount(std::move(p)); slice.data.refcounted.bytes = ptr; slice.data.refcounted.length = len; } @@ -257,94 +208,44 @@ grpc_slice grpc_slice_from_cpp_string(std::string str) { slice.data.refcounted.bytes = reinterpret_cast(const_cast(str.data())); slice.data.refcounted.length = str.size(); - slice.refcount = - (new grpc_core::MovedCppStringSliceRefCount(std::move(str))) - ->base_refcount(); + slice.refcount = new grpc_core::MovedCppStringSliceRefCount(std::move(str)); } return slice; } -namespace { - -class MallocRefCount { - public: - static void Destroy(void* arg) { - MallocRefCount* r = static_cast(arg); - r->~MallocRefCount(); - gpr_free(r); - } - - MallocRefCount() - : base_(grpc_slice_refcount::Type::REGULAR, &refs_, Destroy, this, - &base_) {} - ~MallocRefCount() = default; - - grpc_slice_refcount* base_refcount() { return &base_; } - - private: - grpc_slice_refcount base_; - std::atomic refs_{1}; -}; - -} // namespace - grpc_slice grpc_slice_malloc_large(size_t length) { - return grpc_core::UnmanagedMemorySlice( - length, grpc_core::UnmanagedMemorySlice::ForceHeapAllocation()); -} - -void grpc_core::UnmanagedMemorySlice::HeapInit(size_t length) { - /* Memory layout used by the slice created here: - - +-----------+----------------------------------------------------------+ - | refcount | bytes | - +-----------+----------------------------------------------------------+ - - refcount is a malloc_refcount - bytes is an array of bytes of the requested length - Both parts are placed in the same allocation returned from gpr_malloc */ - auto* rc = - static_cast(gpr_malloc(sizeof(MallocRefCount) + length)); - - /* Initial refcount on rc is 1 - and it's up to the caller to release - this reference. */ - new (rc) MallocRefCount(); - - /* Build up the slice to be returned. */ - /* The slices refcount points back to the allocated block. */ - refcount = rc->base_refcount(); - /* The data bytes are placed immediately after the refcount struct */ - data.refcounted.bytes = reinterpret_cast(rc + 1); - /* And the length of the block is set to the requested length */ - data.refcounted.length = length; + grpc_slice slice; + uint8_t* memory = new uint8_t[sizeof(grpc_slice_refcount) + length]; + slice.refcount = new (memory) grpc_slice_refcount( + [](grpc_slice_refcount* p) { delete[] reinterpret_cast(p); }); + slice.data.refcounted.bytes = memory + sizeof(grpc_slice_refcount); + slice.data.refcounted.length = length; + return slice; } grpc_slice grpc_slice_malloc(size_t length) { - return grpc_core::UnmanagedMemorySlice(length); -} - -grpc_core::UnmanagedMemorySlice::UnmanagedMemorySlice(size_t length) { - if (length > sizeof(data.inlined.bytes)) { - HeapInit(length); + if (length <= GRPC_SLICE_INLINED_SIZE) { + grpc_slice slice; + slice.refcount = nullptr; + slice.data.inlined.length = length; + return slice; } else { - /* small slice: just inline the data */ - refcount = nullptr; - data.inlined.length = static_cast(length); + return grpc_slice_malloc_large(length); } } -template -static Slice sub_no_ref(const Slice& source, size_t begin, size_t end) { - Slice subset; +static grpc_slice sub_no_ref(const grpc_slice& source, size_t begin, + size_t end) { + grpc_slice subset; GPR_ASSERT(end >= begin); - if (source.refcount) { + if (source.refcount != nullptr) { /* Enforce preconditions */ GPR_ASSERT(source.data.refcounted.length >= end); /* Build the result */ - subset.refcount = source.refcount->sub_refcount(); + subset.refcount = source.refcount; /* Point into the source array */ subset.data.refcounted.bytes = source.data.refcounted.bytes + begin; subset.data.refcounted.length = end - begin; @@ -363,11 +264,6 @@ grpc_slice grpc_slice_sub_no_ref(grpc_slice source, size_t begin, size_t end) { return sub_no_ref(source, begin, end); } -grpc_core::UnmanagedMemorySlice grpc_slice_sub_no_ref( - const grpc_core::UnmanagedMemorySlice& source, size_t begin, size_t end) { - return sub_no_ref(source, begin, end); -} - grpc_slice grpc_slice_sub(grpc_slice source, size_t begin, size_t end) { grpc_slice subset; @@ -379,7 +275,9 @@ grpc_slice grpc_slice_sub(grpc_slice source, size_t begin, size_t end) { } else { subset = grpc_slice_sub_no_ref(source, begin, end); /* Bump the refcount */ - subset.refcount->Ref(); + if (subset.refcount != grpc_slice_refcount::NoopRefcount()) { + subset.refcount->Ref(); + } } return subset; } @@ -397,6 +295,12 @@ grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice* source, size_t split, memcpy(tail.data.inlined.bytes, source->data.inlined.bytes + split, tail.data.inlined.length); source->data.inlined.length = static_cast(split); + } else if (source->refcount == grpc_slice_refcount::NoopRefcount()) { + /* refcount == NoopRefcount(), so we can just split in-place */ + tail.refcount = grpc_slice_refcount::NoopRefcount(); + tail.data.refcounted.bytes = source->data.refcounted.bytes + split; + tail.data.refcounted.length = source->data.refcounted.length - split; + source->data.refcounted.length = split; } else { size_t tail_length = source->data.refcounted.length - split; GPR_ASSERT(source->data.refcounted.length >= split); @@ -407,23 +311,22 @@ grpc_slice grpc_slice_split_tail_maybe_ref(grpc_slice* source, size_t split, tail.data.inlined.length = static_cast(tail_length); memcpy(tail.data.inlined.bytes, source->data.refcounted.bytes + split, tail_length); - source->refcount = source->refcount->sub_refcount(); } else { /* Build the result */ switch (ref_whom) { case GRPC_SLICE_REF_TAIL: - tail.refcount = source->refcount->sub_refcount(); - source->refcount = &grpc_core::kNoopRefcount; + tail.refcount = source->refcount; + source->refcount = grpc_slice_refcount::NoopRefcount(); break; case GRPC_SLICE_REF_HEAD: - tail.refcount = &grpc_core::kNoopRefcount; - source->refcount = source->refcount->sub_refcount(); + tail.refcount = grpc_slice_refcount::NoopRefcount(); break; case GRPC_SLICE_REF_BOTH: - tail.refcount = source->refcount->sub_refcount(); - source->refcount = source->refcount->sub_refcount(); + tail.refcount = source->refcount; /* Bump the refcount */ - tail.refcount->Ref(); + if (tail.refcount != grpc_slice_refcount::NoopRefcount()) { + tail.refcount->Ref(); + } break; } /* Point into the source array */ @@ -459,20 +362,20 @@ grpc_slice grpc_slice_split_head(grpc_slice* source, size_t split) { head.refcount = nullptr; head.data.inlined.length = static_cast(split); memcpy(head.data.inlined.bytes, source->data.refcounted.bytes, split); - source->refcount = source->refcount->sub_refcount(); source->data.refcounted.bytes += split; source->data.refcounted.length -= split; } else { GPR_ASSERT(source->data.refcounted.length >= split); /* Build the result */ - head.refcount = source->refcount->sub_refcount(); + head.refcount = source->refcount; /* Bump the refcount */ - head.refcount->Ref(); + if (head.refcount != grpc_slice_refcount::NoopRefcount()) { + head.refcount->Ref(); + } /* Point into the source array */ head.data.refcounted.bytes = source->data.refcounted.bytes; head.data.refcounted.length = split; - source->refcount = source->refcount->sub_refcount(); source->data.refcounted.bytes += split; source->data.refcounted.length -= split; } @@ -480,21 +383,13 @@ grpc_slice grpc_slice_split_head(grpc_slice* source, size_t split) { return head; } -int grpc_slice_default_eq_impl(grpc_slice a, grpc_slice b) { +int grpc_slice_eq(grpc_slice a, grpc_slice b) { if (GRPC_SLICE_LENGTH(a) != GRPC_SLICE_LENGTH(b)) return false; if (GRPC_SLICE_LENGTH(a) == 0) return true; return 0 == memcmp(GRPC_SLICE_START_PTR(a), GRPC_SLICE_START_PTR(b), GRPC_SLICE_LENGTH(a)); } -int grpc_slice_eq(grpc_slice a, grpc_slice b) { - if (a.refcount && b.refcount && - a.refcount->GetType() == b.refcount->GetType()) { - return a.refcount->Eq(a, b); - } - return grpc_slice_default_eq_impl(a, b); -} - int grpc_slice_differs_refcounted(const grpc_slice& a, const grpc_slice& b_not_inline) { size_t a_len; diff --git a/src/core/lib/slice/slice.h b/src/core/lib/slice/slice.h index 357ea708ccc..c4c9284bec6 100644 --- a/src/core/lib/slice/slice.h +++ b/src/core/lib/slice/slice.h @@ -25,6 +25,8 @@ #include "src/core/lib/gpr/string.h" #include "src/core/lib/slice/slice_internal.h" +#include "src/core/lib/slice/slice_refcount.h" +#include "src/core/lib/slice/slice_refcount_base.h" // Herein lies grpc_core::Slice and its team of thin wrappers around grpc_slice. // They aim to keep you safe by providing strong guarantees around lifetime and @@ -103,6 +105,8 @@ class BaseSlice { return grpc_slice_is_equivalent(slice_, other.slice_); } + uint32_t Hash() const { return grpc_slice_hash_internal(slice_); } + protected: BaseSlice() : slice_(EmptySlice()) {} explicit BaseSlice(const grpc_slice& slice) : slice_(slice) {} @@ -162,7 +166,7 @@ inline bool operator!=(const grpc_slice& a, const BaseSlice& b) { template struct CopyConstructors { static Out FromCopiedString(const char* s) { - return Out(grpc_slice_from_copied_string(s)); + return FromCopiedBuffer(s, strlen(s)); } static Out FromCopiedString(absl::string_view s) { return FromCopiedBuffer(s.data(), s.size()); @@ -171,7 +175,7 @@ struct CopyConstructors { return Out(grpc_slice_from_cpp_string(std::move(s))); } static Out FromCopiedBuffer(const char* p, size_t len) { - return Out(UnmanagedMemorySlice(p, len)); + return Out(grpc_slice_from_copied_buffer(p, len)); } template @@ -190,11 +194,20 @@ struct CopyConstructors { template struct StaticConstructors { static Out FromStaticString(const char* s) { - return Out(grpc_slice_from_static_string(s)); + return FromStaticBuffer(s, strlen(s)); } static Out FromStaticString(absl::string_view s) { - return Out(ExternallyManagedSlice(s.data(), s.size())); + return FromStaticBuffer(s.data(), s.size()); + } + + static Out FromStaticBuffer(const void* s, size_t len) { + grpc_slice slice; + slice.refcount = grpc_slice_refcount::NoopRefcount(); + slice.data.refcounted.bytes = + const_cast(static_cast(s)); + slice.data.refcounted.length = len; + return Out(slice); } }; @@ -206,11 +219,8 @@ class StaticSlice : public slice_detail::BaseSlice, StaticSlice() = default; explicit StaticSlice(const grpc_slice& slice) : slice_detail::BaseSlice(slice) { - GPR_DEBUG_ASSERT(slice.refcount->GetType() == - grpc_slice_refcount::Type::NOP); + GPR_DEBUG_ASSERT(slice.refcount == grpc_slice_refcount::NoopRefcount()); } - explicit StaticSlice(const StaticMetadataSlice& slice) - : slice_detail::BaseSlice(slice) {} StaticSlice(const StaticSlice& other) : slice_detail::BaseSlice(other.c_slice()) {} @@ -232,8 +242,7 @@ class MutableSlice : public slice_detail::BaseSlice, MutableSlice() = default; explicit MutableSlice(const grpc_slice& slice) : slice_detail::BaseSlice(slice) { - GPR_DEBUG_ASSERT(slice.refcount == nullptr || - slice.refcount->IsRegularUnique()); + GPR_DEBUG_ASSERT(slice.refcount == nullptr || slice.refcount->IsUnique()); } ~MutableSlice() { grpc_slice_unref_internal(c_slice()); } @@ -297,7 +306,7 @@ class Slice : public slice_detail::BaseSlice, if (c_slice().refcount == nullptr) { return Slice(c_slice()); } - if (c_slice().refcount->GetType() == grpc_slice_refcount::Type::NOP) { + if (c_slice().refcount == grpc_slice_refcount::NoopRefcount()) { return Slice(grpc_slice_copy(c_slice())); } return Slice(TakeCSlice()); @@ -309,7 +318,7 @@ class Slice : public slice_detail::BaseSlice, if (c_slice().refcount == nullptr) { return Slice(c_slice()); } - if (c_slice().refcount->GetType() == grpc_slice_refcount::Type::NOP) { + if (c_slice().refcount == grpc_slice_refcount::NoopRefcount()) { return Slice(grpc_slice_copy(c_slice())); } return Slice(grpc_slice_ref_internal(c_slice())); @@ -327,8 +336,8 @@ class Slice : public slice_detail::BaseSlice, if (c_slice().refcount == nullptr) { return MutableSlice(c_slice()); } - if (c_slice().refcount->GetType() == grpc_slice_refcount::Type::REGULAR && - c_slice().refcount->IsRegularUnique()) { + if (c_slice().refcount != grpc_slice_refcount::NoopRefcount() && + c_slice().refcount->IsUnique()) { return MutableSlice(TakeCSlice()); } return MutableSlice(grpc_slice_copy(c_slice())); @@ -359,11 +368,15 @@ class Slice : public slice_detail::BaseSlice, const uint8_t* begin, const uint8_t* end) { grpc_slice out; out.refcount = r; - r->Ref(); + if (r != grpc_slice_refcount::NoopRefcount()) r->Ref(); out.data.refcounted.bytes = const_cast(begin); out.data.refcounted.length = end - begin; return Slice(out); } + + static Slice FromExternalString(absl::string_view str) { + return FromStaticString(str); + } }; } // namespace grpc_core diff --git a/src/core/lib/slice/slice_intern.cc b/src/core/lib/slice/slice_intern.cc deleted file mode 100644 index 2ab13d1458b..00000000000 --- a/src/core/lib/slice/slice_intern.cc +++ /dev/null @@ -1,269 +0,0 @@ -/* - * - * Copyright 2016 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. - * - */ - -#include - -#include -#include - -#include -#include - -#include "src/core/lib/gpr/murmur_hash.h" -#include "src/core/lib/gprpp/sync.h" -#include "src/core/lib/iomgr/iomgr_internal.h" /* for iomgr_abort_on_leaks() */ -#include "src/core/lib/profiling/timers.h" -#include "src/core/lib/slice/slice_internal.h" -#include "src/core/lib/slice/slice_string_helpers.h" -#include "src/core/lib/slice/slice_utils.h" - -#define LOG2_SHARD_COUNT 5 -#define SHARD_COUNT (1 << LOG2_SHARD_COUNT) -#define INITIAL_SHARD_CAPACITY 8 - -#define TABLE_IDX(hash, capacity) (((hash) >> LOG2_SHARD_COUNT) % (capacity)) -#define SHARD_IDX(hash) ((hash) & ((1 << LOG2_SHARD_COUNT) - 1)) - -using grpc_core::InternedSliceRefcount; - -typedef struct slice_shard { - grpc_core::Mutex mu; - InternedSliceRefcount** strs; - size_t count; - size_t capacity; -} slice_shard; - -static slice_shard* g_shards; - -namespace grpc_core { - -/* hash seed: decided at initialization time */ -uint32_t g_hash_seed; -static bool g_forced_hash_seed = false; - -InternedSliceRefcount::~InternedSliceRefcount() { - slice_shard* shard = &g_shards[SHARD_IDX(this->hash)]; - MutexLock lock(&shard->mu); - InternedSliceRefcount** prev_next; - InternedSliceRefcount* cur; - for (prev_next = &shard->strs[TABLE_IDX(this->hash, shard->capacity)], - cur = *prev_next; - cur != this; prev_next = &cur->bucket_next, cur = cur->bucket_next) { - } - *prev_next = cur->bucket_next; - shard->count--; -} - -} // namespace grpc_core - -static void grow_shard(slice_shard* shard) { - GPR_TIMER_SCOPE("grow_strtab", 0); - - size_t capacity = shard->capacity * 2; - size_t i; - InternedSliceRefcount** strtab; - InternedSliceRefcount *s, *next; - - strtab = static_cast( - gpr_zalloc(sizeof(InternedSliceRefcount*) * capacity)); - - for (i = 0; i < shard->capacity; i++) { - for (s = shard->strs[i]; s; s = next) { - size_t idx = TABLE_IDX(s->hash, capacity); - next = s->bucket_next; - s->bucket_next = strtab[idx]; - strtab[idx] = s; - } - } - gpr_free(shard->strs); - shard->strs = strtab; - shard->capacity = capacity; -} - -grpc_core::InternedSlice::InternedSlice(InternedSliceRefcount* s) { - refcount = &s->base; - data.refcounted.bytes = reinterpret_cast(s + 1); - data.refcounted.length = s->length; -} - -uint32_t grpc_slice_default_hash_impl(grpc_slice s) { - return gpr_murmur_hash3(GRPC_SLICE_START_PTR(s), GRPC_SLICE_LENGTH(s), - grpc_core::g_hash_seed); -} - -uint32_t grpc_slice_hash(grpc_slice s) { return grpc_slice_hash_internal(s); } - -grpc_slice grpc_slice_intern(grpc_slice slice) { - /* TODO(arjunroy): At present, this is capable of returning either a static or - an interned slice. This yields weirdness like the constructor for - ManagedMemorySlice instantiating itself as an instance of a derived type - (StaticMetadataSlice or InternedSlice). Should reexamine. */ - return grpc_core::ManagedMemorySlice(&slice); -} - -// Helper methods to enable us to select appropriately overloaded slice methods -// whether we're dealing with a slice, or a buffer with length, when interning -// strings. Helpers for FindOrCreateInternedSlice(). -static const char* GetBuffer(const std::pair& buflen) { - return buflen.first; -} -static size_t GetLength(const std::pair& buflen) { - return buflen.second; -} -static const void* GetBuffer(const grpc_slice& slice) { - return GRPC_SLICE_START_PTR(slice); -} -static size_t GetLength(const grpc_slice& slice) { - return GRPC_SLICE_LENGTH(slice); -} - -// Creates an interned slice for a string that does not currently exist in the -// intern table. SliceArgs is either a const grpc_slice& or a const -// pair&. Hash is the pre-computed hash value. We must -// already hold the shard lock. Helper for FindOrCreateInternedSlice(). -// -// Returns: a newly interned slice. -template -static InternedSliceRefcount* InternNewStringLocked(slice_shard* shard, - size_t shard_idx, - uint32_t hash, - const SliceArgs& args) { - /* string data goes after the internal_string header */ - size_t len = GetLength(args); - const void* buffer = GetBuffer(args); - InternedSliceRefcount* s = - static_cast(gpr_malloc(sizeof(*s) + len)); - new (s) grpc_core::InternedSliceRefcount(len, hash, shard->strs[shard_idx]); - // TODO(arjunroy): Investigate why hpack tried to intern the nullptr string. - // https://github.com/grpc/grpc/pull/20110#issuecomment-526729282 - if (len > 0) { - memcpy(reinterpret_cast(s + 1), buffer, len); - } - shard->strs[shard_idx] = s; - shard->count++; - if (shard->count > shard->capacity * 2) { - grow_shard(shard); - } - return s; -} - -// Attempt to see if the provided slice or string matches an existing interned -// slice. SliceArgs... is either a const grpc_slice& or a string and length. In -// either case, hash is the pre-computed hash value. We must already hold the -// shard lock. Helper for FindOrCreateInternedSlice(). -// -// Returns: a pre-existing matching static slice, or null. -template -static InternedSliceRefcount* MatchInternedSliceLocked(uint32_t hash, - size_t idx, - const SliceArgs& args) { - InternedSliceRefcount* s; - slice_shard* shard = &g_shards[SHARD_IDX(hash)]; - /* search for an existing string */ - for (s = shard->strs[idx]; s; s = s->bucket_next) { - if (s->hash == hash && grpc_core::InternedSlice(s) == args) { - if (grpc_core::IncrementIfNonzero(&s->refcnt)) { - return s; - } - } - } - return nullptr; -} - -// Attempt to see if the provided slice or string matches an existing interned -// slice, and failing that, create an interned slice with its contents. Returns -// either the existing matching interned slice or the newly created one. -// SliceArgs is either a const grpc_slice& or const pair&. -// In either case, hash is the pre-computed hash value. We do not hold the -// shard lock here, but do take it. -// -// Returns: an interned slice, either pre-existing/matched or newly created. -template -static InternedSliceRefcount* FindOrCreateInternedSlice(uint32_t hash, - const SliceArgs& args) { - slice_shard* shard = &g_shards[SHARD_IDX(hash)]; - grpc_core::MutexLock lock(&shard->mu); - const size_t idx = TABLE_IDX(hash, shard->capacity); - InternedSliceRefcount* s = MatchInternedSliceLocked(hash, idx, args); - if (s == nullptr) { - s = InternNewStringLocked(shard, idx, hash, args); - } - return s; -} - -grpc_core::ManagedMemorySlice::ManagedMemorySlice(const char* string) - : grpc_core::ManagedMemorySlice::ManagedMemorySlice(string, - strlen(string)) {} - -grpc_core::ManagedMemorySlice::ManagedMemorySlice(const char* buf, size_t len) { - GPR_TIMER_SCOPE("grpc_slice_intern", 0); - const uint32_t hash = gpr_murmur_hash3(buf, len, g_hash_seed); - *this = grpc_core::InternedSlice(FindOrCreateInternedSlice( - hash, std::pair(buf, len))); -} - -grpc_core::ManagedMemorySlice::ManagedMemorySlice(const grpc_slice* slice_ptr) { - GPR_TIMER_SCOPE("grpc_slice_intern", 0); - const grpc_slice& slice = *slice_ptr; - const uint32_t hash = grpc_slice_hash_internal(slice); - *this = grpc_core::InternedSlice(FindOrCreateInternedSlice(hash, slice)); -} - -void grpc_test_only_set_slice_hash_seed(uint32_t seed) { - grpc_core::g_hash_seed = seed; - grpc_core::g_forced_hash_seed = true; -} - -void grpc_slice_intern_init(void) { - if (!grpc_core::g_forced_hash_seed) { - grpc_core::g_hash_seed = - static_cast(gpr_now(GPR_CLOCK_REALTIME).tv_nsec); - } - g_shards = new slice_shard[SHARD_COUNT]; - for (size_t i = 0; i < SHARD_COUNT; i++) { - slice_shard* shard = &g_shards[i]; - shard->count = 0; - shard->capacity = INITIAL_SHARD_CAPACITY; - shard->strs = static_cast( - gpr_zalloc(sizeof(*shard->strs) * shard->capacity)); - } -} - -void grpc_slice_intern_shutdown(void) { - for (size_t i = 0; i < SHARD_COUNT; i++) { - slice_shard* shard = &g_shards[i]; - /* TODO(ctiller): GPR_ASSERT(shard->count == 0); */ - if (shard->count != 0) { - gpr_log(GPR_DEBUG, "WARNING: %" PRIuPTR " metadata strings were leaked", - shard->count); - for (size_t j = 0; j < shard->capacity; j++) { - for (InternedSliceRefcount* s = shard->strs[j]; s; s = s->bucket_next) { - char* text = grpc_dump_slice(grpc_core::InternedSlice(s), - GPR_DUMP_HEX | GPR_DUMP_ASCII); - gpr_log(GPR_DEBUG, "LEAKED: %s", text); - gpr_free(text); - } - } - if (grpc_iomgr_abort_on_leaks()) { - abort(); - } - } - gpr_free(shard->strs); - } - delete[] g_shards; -} diff --git a/src/core/lib/slice/slice_internal.h b/src/core/lib/slice/slice_internal.h index ae664edc6ce..d4f22bbfec0 100644 --- a/src/core/lib/slice/slice_internal.h +++ b/src/core/lib/slice/slice_internal.h @@ -23,6 +23,8 @@ #include +#include "absl/strings/string_view.h" + #include #include #include @@ -31,7 +33,6 @@ #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/slice/slice_refcount.h" -#include "src/core/lib/slice/slice_utils.h" void grpc_slice_buffer_reset_and_unref_internal(grpc_slice_buffer* sb); void grpc_slice_buffer_partial_unref_internal(grpc_slice_buffer* sb, @@ -52,21 +53,6 @@ void grpc_slice_buffer_remove_first(grpc_slice_buffer* sb); void grpc_slice_buffer_sub_first(grpc_slice_buffer* sb, size_t begin, size_t end); -/* Check if a slice is interned */ -bool grpc_slice_is_interned(const grpc_slice& slice); -inline bool grpc_slice_is_interned(const grpc_slice& slice) { - return (slice.refcount && - (slice.refcount->GetType() == grpc_slice_refcount::Type::INTERNED)); -} - -inline bool grpc_slice_static_interned_equal(const grpc_slice& a, - const grpc_slice& b) { - GPR_DEBUG_ASSERT(grpc_slice_is_interned(a) && grpc_slice_is_interned(b)); - return a.refcount == b.refcount; -} - -void grpc_slice_intern_init(void); -void grpc_slice_intern_shutdown(void); void grpc_test_only_set_slice_hash_seed(uint32_t seed); // if slice matches a static slice, returns the static slice // otherwise returns the passed in slice (without reffing it) @@ -77,21 +63,11 @@ grpc_slice grpc_slice_maybe_static_intern(grpc_slice slice, uint32_t grpc_static_slice_hash(grpc_slice s); int grpc_static_slice_eq(grpc_slice a, grpc_slice b); -inline uint32_t grpc_slice_hash_refcounted(const grpc_slice& s) { - GPR_DEBUG_ASSERT(s.refcount != nullptr); - return s.refcount->Hash(s); -} - -inline uint32_t grpc_slice_default_hash_internal(const grpc_slice& s) { +inline uint32_t grpc_slice_hash_internal(const grpc_slice& s) { return gpr_murmur_hash3(GRPC_SLICE_START_PTR(s), GRPC_SLICE_LENGTH(s), grpc_core::g_hash_seed); } -inline uint32_t grpc_slice_hash_internal(const grpc_slice& s) { - return s.refcount == nullptr ? grpc_slice_default_hash_internal(s) - : grpc_slice_hash_refcounted(s); -} - grpc_slice grpc_slice_from_moved_buffer(grpc_core::UniquePtr p, size_t len); grpc_slice grpc_slice_from_moved_string(grpc_core::UniquePtr p); @@ -102,9 +78,6 @@ grpc_slice grpc_slice_from_cpp_string(std::string str); // 0. All other slices will return the size of the allocated chars. size_t grpc_slice_memory_usage(grpc_slice s); -grpc_core::UnmanagedMemorySlice grpc_slice_sub_no_ref( - const grpc_core::UnmanagedMemorySlice& source, size_t begin, size_t end); - namespace grpc_core { struct SliceHash { @@ -113,6 +86,15 @@ struct SliceHash { } }; +extern uint32_t g_hash_seed; + +// Converts grpc_slice to absl::string_view. +inline absl::string_view StringViewFromSlice(const grpc_slice& slice) { + return absl::string_view( + reinterpret_cast(GRPC_SLICE_START_PTR(slice)), + GRPC_SLICE_LENGTH(slice)); +} + } // namespace grpc_core inline bool operator==(const grpc_slice& s1, const grpc_slice& s2) { diff --git a/src/core/lib/slice/slice_refcount.cc b/src/core/lib/slice/slice_refcount.cc index 10de62b72e7..dc42de1d864 100644 --- a/src/core/lib/slice/slice_refcount.cc +++ b/src/core/lib/slice/slice_refcount.cc @@ -15,3 +15,21 @@ #include #include "src/core/lib/slice/slice_refcount.h" + +#include + +namespace grpc_core { + +uint32_t g_hash_seed = []() { + auto now = std::chrono::system_clock::now(); + auto now_since_epoch = now.time_since_epoch(); + auto now_ns = + std::chrono::duration_cast(now_since_epoch); + return static_cast(now_ns.count()); +}(); + +} // namespace grpc_core + +void grpc_test_only_set_slice_hash_seed(uint32_t seed) { + grpc_core::g_hash_seed = seed; +} diff --git a/src/core/lib/slice/slice_refcount.h b/src/core/lib/slice/slice_refcount.h index a2f5f918ca2..c3a90ca6f2a 100644 --- a/src/core/lib/slice/slice_refcount.h +++ b/src/core/lib/slice/slice_refcount.h @@ -27,88 +27,18 @@ namespace grpc_core { extern uint32_t g_hash_seed; -extern grpc_slice_refcount kNoopRefcount; - -// TODO(ctiller): when this is removed, remove the std::atomic* in -// grpc_slice_refcount and just put it there directly. -struct InternedSliceRefcount { - static void Destroy(void* arg) { - auto* rc = static_cast(arg); - rc->~InternedSliceRefcount(); - gpr_free(rc); - } - - InternedSliceRefcount(size_t length, uint32_t hash, - InternedSliceRefcount* bucket_next) - : base(grpc_slice_refcount::Type::INTERNED, &refcnt, Destroy, this, &sub), - sub(grpc_slice_refcount::Type::REGULAR, &refcnt, Destroy, this, &sub), - length(length), - hash(hash), - bucket_next(bucket_next) {} - - ~InternedSliceRefcount(); - - grpc_slice_refcount base; - grpc_slice_refcount sub; - const size_t length; - std::atomic refcnt{1}; - const uint32_t hash; - InternedSliceRefcount* bucket_next; -}; } // namespace grpc_core -inline size_t grpc_refcounted_slice_length(const grpc_slice& slice) { - GPR_DEBUG_ASSERT(slice.refcount != nullptr); - return slice.data.refcounted.length; -} - -inline const uint8_t* grpc_refcounted_slice_data(const grpc_slice& slice) { - GPR_DEBUG_ASSERT(slice.refcount != nullptr); - return slice.data.refcounted.bytes; -} - -inline int grpc_slice_refcount::Eq(const grpc_slice& a, const grpc_slice& b) { - GPR_DEBUG_ASSERT(a.refcount != nullptr); - GPR_DEBUG_ASSERT(a.refcount == this); - switch (ref_type_) { - case Type::INTERNED: - return a.refcount == b.refcount; - case Type::NOP: - case Type::REGULAR: - break; - } - if (grpc_refcounted_slice_length(a) != GRPC_SLICE_LENGTH(b)) return false; - if (grpc_refcounted_slice_length(a) == 0) return true; - return 0 == memcmp(grpc_refcounted_slice_data(a), GRPC_SLICE_START_PTR(b), - grpc_refcounted_slice_length(a)); -} - -inline uint32_t grpc_slice_refcount::Hash(const grpc_slice& slice) { - GPR_DEBUG_ASSERT(slice.refcount != nullptr); - GPR_DEBUG_ASSERT(slice.refcount == this); - switch (ref_type_) { - case Type::INTERNED: - return reinterpret_cast(slice.refcount) - ->hash; - case Type::NOP: - case Type::REGULAR: - break; - } - return gpr_murmur_hash3(grpc_refcounted_slice_data(slice), - grpc_refcounted_slice_length(slice), - grpc_core::g_hash_seed); -} - inline const grpc_slice& grpc_slice_ref_internal(const grpc_slice& slice) { - if (slice.refcount) { + if (reinterpret_cast(slice.refcount) > 1) { slice.refcount->Ref(); } return slice; } inline void grpc_slice_unref_internal(const grpc_slice& slice) { - if (slice.refcount) { + if (reinterpret_cast(slice.refcount) > 1) { slice.refcount->Unref(); } } diff --git a/src/core/lib/slice/slice_refcount_base.h b/src/core/lib/slice/slice_refcount_base.h index 5d81fcd8fc8..79aaf9c1f04 100644 --- a/src/core/lib/slice/slice_refcount_base.h +++ b/src/core/lib/slice/slice_refcount_base.h @@ -23,143 +23,39 @@ #include // grpc_slice_refcount : A reference count for grpc_slice. -// -// Non-inlined grpc_slice objects are refcounted. Historically this was -// implemented via grpc_slice_refcount, a C-style polymorphic class using a -// manually managed vtable of operations. Subclasses would define their own -// vtable; the 'virtual' methods (ref, unref, equals and hash) would simply call -// the function pointers in the vtable as necessary. -// -// Unfortunately, this leads to some inefficiencies in the generated code that -// can be improved upon. For example, equality checking for interned slices is a -// simple equality check on the refcount pointer. With the vtable approach, this -// would translate to roughly the following (high-level) instructions: -// -// grpc_slice_equals(slice1, slice2): -// load vtable->eq -> eq_func -// call eq_func(slice1, slice2) -// -// interned_slice_equals(slice1, slice2) -// load slice1.ref -> r1 -// load slice2.ref -> r2 -// cmp r1, r2 -> retval -// ret retval -// -// This leads to a function call for a function defined in another translation -// unit, which imposes memory barriers, which reduces the compiler's ability to -// optimize (in addition to the added overhead of call/ret). Additionally, it -// may be harder to reason about branch prediction when we're jumping to -// essentially arbitrarily provided function pointers. -// -// In addition, it is arguable that while virtualization was helpful for -// Equals()/Hash() methods, that it was fundamentally unnecessary for -// Ref()/Unref(). -// -// Instead, grpc_slice_refcount provides the same functionality as the C-style -// virtual class, but in a de-virtualized manner - Eq(), Hash(), Ref() and -// Unref() are provided within this header file. Fastpaths for Eq()/Hash() -// (interned and static metadata slices), as well as the Ref() operation, can -// all be inlined without any memory barriers. -// -// It does this by: -// 1. Using grpc_core::RefCount<> (header-only) for Ref/Unref. Two special cases -// need support: No-op ref/unref (eg. static metadata slices) and stream -// slice references (where all the slices share the streamref). This is in -// addition to the normal case of '1 slice, 1 ref'. -// To support these cases, we explicitly track a nullable pointer to the -// underlying RefCount<>. No-op ref/unref is used by checking the pointer for -// null, and doing nothing if it is. Both stream slice refs and 'normal' -// slices use the same path for Ref/Unref (by targeting the non-null -// pointer). -// -// 2. introducing the notion of grpc_slice_refcount::Type. This describes if a -// slice ref is used by a static metadata slice, an interned slice, or other -// slices. We switch on the slice ref type in order to provide fastpaths for -// Equals() and Hash(). -// -// In total, this saves us roughly 1-2% latency for unary calls, with smaller -// calls benefitting. The effect is present, but not as useful, for larger calls -// where the cost of sending the data dominates. -// TODO(arjunroy): Investigate if this can be removed with strongly typed -// grpc_slices. struct grpc_slice_refcount { public: - enum class Type { - INTERNED, // Refcount for an interned slice. - NOP, // No-Op - REGULAR // Refcount for non-static-metadata, non-interned slices. - }; - typedef void (*DestroyerFn)(void*); + typedef void (*DestroyerFn)(grpc_slice_refcount*); - grpc_slice_refcount() = default; + static grpc_slice_refcount* NoopRefcount() { + return reinterpret_cast(1); + } - explicit grpc_slice_refcount(Type t) : ref_type_(t) {} + grpc_slice_refcount() = default; - explicit grpc_slice_refcount(grpc_slice_refcount* sub) : sub_refcount_(sub) {} // Regular constructor for grpc_slice_refcount. // // Parameters: - // 1. grpc_slice_refcount::Type type - // Whether we are the refcount for a static - // metadata slice, an interned slice, or any other kind of slice. - // - // 2. std::atomic* ref - // The pointer to the actual underlying grpc_core::RefCount. - // TODO(ctiller): remove the pointer indirection and just put the refcount on - // this object once we remove interning. - // - // 3. DestroyerFn destroyer_fn - // Called when the refcount goes to 0, with destroyer_arg as parameter. - // - // 4. void* destroyer_arg - // Argument for the virtualized destructor. - // - // 5. grpc_slice_refcount* sub - // Argument used for interned slices. - grpc_slice_refcount(grpc_slice_refcount::Type type, std::atomic* ref, - DestroyerFn destroyer_fn, void* destroyer_arg, - grpc_slice_refcount* sub) - : ref_(ref), - ref_type_(type), - sub_refcount_(sub), - dest_fn_(destroyer_fn), - destroy_fn_arg_(destroyer_arg) {} - // Initializer for static refcounts. - grpc_slice_refcount(grpc_slice_refcount* sub, Type type) - : ref_type_(type), sub_refcount_(sub) {} + // 1. DestroyerFn destroyer_fn + // Called when the refcount goes to 0, with 'this' as parameter. + explicit grpc_slice_refcount(DestroyerFn destroyer_fn) + : destroyer_fn_(destroyer_fn) {} - Type GetType() const { return ref_type_; } - - int Eq(const grpc_slice& a, const grpc_slice& b); - - uint32_t Hash(const grpc_slice& slice); - void Ref() { - if (ref_ == nullptr) return; - ref_->fetch_add(1, std::memory_order_relaxed); - } + void Ref() { ref_.fetch_add(1, std::memory_order_relaxed); } void Unref() { - if (ref_ == nullptr) return; - if (ref_->fetch_sub(1, std::memory_order_acq_rel) == 1) { - dest_fn_(destroy_fn_arg_); + if (ref_.fetch_sub(1, std::memory_order_acq_rel) == 1) { + destroyer_fn_(this); } } - // Only for type REGULAR, is this the only instance? + // Is this the only instance? // For this to be useful the caller needs to ensure that if this is the only // instance, no other instance could be created during this call. - bool IsRegularUnique() { - GPR_DEBUG_ASSERT(ref_type_ == Type::REGULAR); - return ref_->load(std::memory_order_relaxed) == 1; - } - - grpc_slice_refcount* sub_refcount() const { return sub_refcount_; } + bool IsUnique() const { return ref_.load(std::memory_order_relaxed) == 1; } private: - std::atomic* ref_ = nullptr; - const Type ref_type_ = Type::REGULAR; - grpc_slice_refcount* sub_refcount_ = this; - DestroyerFn dest_fn_ = nullptr; - void* destroy_fn_arg_ = nullptr; + std::atomic ref_{1}; + DestroyerFn destroyer_fn_ = nullptr; }; #endif // GRPC_CORE_LIB_SLICE_SLICE_REFCOUNT_BASE_H diff --git a/src/core/lib/slice/slice_utils.h b/src/core/lib/slice/slice_utils.h deleted file mode 100644 index d9a4253c1aa..00000000000 --- a/src/core/lib/slice/slice_utils.h +++ /dev/null @@ -1,200 +0,0 @@ -/* - * - * Copyright 2019 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_CORE_LIB_SLICE_SLICE_UTILS_H -#define GRPC_CORE_LIB_SLICE_SLICE_UTILS_H - -#include - -#include - -#include "absl/strings/string_view.h" - -#include - -#include "src/core/lib/gpr/murmur_hash.h" - -namespace grpc_core { -extern uint32_t g_hash_seed; -} // namespace grpc_core - -// When we compare two slices, and we know the latter is not inlined, we can -// short circuit our comparison operator. We specifically use differs() -// semantics instead of equals() semantics due to more favourable code -// generation when using differs(). Specifically, we may use the output of -// grpc_slice_differs_refcounted for control flow. If we use differs() -// semantics, we end with a tailcall to memcmp(). If we use equals() semantics, -// we need to invert the result that memcmp provides us, which costs several -// instructions to do so. If we're using the result for control flow (i.e. -// branching based on the output) then we're just performing the extra -// operations to invert the result pointlessly. Concretely, we save 6 ops on -// x86-64/clang with differs(). -int grpc_slice_differs_refcounted(const grpc_slice& a, - const grpc_slice& b_not_inline); - -// When we compare two slices, and we *know* that one of them is static or -// interned, we can short circuit our slice equality function. The second slice -// here must be static or interned; slice a can be any slice, inlined or not. -inline bool grpc_slice_eq_static_interned(const grpc_slice& a, - const grpc_slice& b_static_interned) { - if (a.refcount == b_static_interned.refcount) { - return true; - } - return !grpc_slice_differs_refcounted(a, b_static_interned); -} - -// TODO(arjunroy): These type declarations ought to be in -// src/core/lib/slice/slice_internal.h instead; they are here due to a circular -// header depedency between slice_internal.h and -// src/core/lib/transport/metadata.h. We need to fix this circular reference and -// when we do, move these type declarations. -// -// Internal slice type declarations. -// Externally, a grpc_slice is a grpc_slice is a grpc_slice. -// Internally, we may have heap allocated slices, static slices, interned -// slices, and inlined slices. If we know the specific type of slice -// we're dealing with, we can save cycles (e.g. fast-paths when we know we don't -// need to take a reference on a slice). Rather than introducing new methods -// ad-hoc in these cases, we rely on type-system backed overloads to keep -// internal APIs clean. -// -// For each overload, the definition and layout of the underlying slice does not -// change; this is purely type-system information. -namespace grpc_core { - -// There are two main types of slices: those that have their memory -// managed by the slice library and those that do not. -// -// The following types of slices are not managed: -// - inlined slices (i.e., refcount is null) -// - slices that have a custom refcount type (i.e., not STATIC or INTERNED) -// - slices where the memory is managed by some external agent. The slice is not -// ref-counted by grpc, and the programmer is responsible for ensuring the -// data is valid for the duration of the period that grpc may access it. -// -// The following types of slices are managed: -// - static metadata slices (i.e., refcount type is STATIC) -// - interned slices (i.e., refcount type is INTERNED) -// -// This categorization is reflected in the following hierarchy: -// -// - grpc_slice -// > - UnmanagedMemorySlice -// > - ExternallyManagedSlice -// - ManagedMemorySlice -// > - InternedSlice -// - StaticMetadataSlice -// -struct ManagedMemorySlice : public grpc_slice { - ManagedMemorySlice() { - refcount = nullptr; - data.refcounted.bytes = nullptr; - data.refcounted.length = 0; - } - explicit ManagedMemorySlice(const char* string); - ManagedMemorySlice(const char* buf, size_t len); - explicit ManagedMemorySlice(const grpc_slice* slice); - bool operator==(const grpc_slice& other) const { - if (refcount == other.refcount) { - return true; - } - return !grpc_slice_differs_refcounted(other, *this); - } - bool operator!=(const grpc_slice& other) const { return !(*this == other); } - bool operator==(std::pair buflen) const { - return data.refcounted.length == buflen.second && buflen.first != nullptr && - memcmp(buflen.first, data.refcounted.bytes, buflen.second) == 0; - } -}; -struct UnmanagedMemorySlice : public grpc_slice { - // TODO(arjunroy): Can we use a default=false param instead of this enum? - enum class ForceHeapAllocation {}; - UnmanagedMemorySlice() { - refcount = nullptr; - data.inlined.length = 0; - } - explicit UnmanagedMemorySlice(const char* source); - UnmanagedMemorySlice(const char* source, size_t length); - // The first constructor creates a slice that may be heap allocated, or - // inlined in the slice structure if length is small enough - // (< GRPC_SLICE_INLINED_SIZE). The second constructor forces heap alloc. - explicit UnmanagedMemorySlice(size_t length); - explicit UnmanagedMemorySlice(size_t length, const ForceHeapAllocation&) { - HeapInit(length); - } - - private: - void HeapInit(size_t length); -}; - -extern grpc_slice_refcount kNoopRefcount; - -struct ExternallyManagedSlice : public UnmanagedMemorySlice { - ExternallyManagedSlice() - : ExternallyManagedSlice(&kNoopRefcount, 0, nullptr) {} - explicit ExternallyManagedSlice(const char* s) - : ExternallyManagedSlice(s, strlen(s)) {} - ExternallyManagedSlice(const void* s, size_t len) - : ExternallyManagedSlice( - &kNoopRefcount, len, - reinterpret_cast(const_cast(s))) {} - ExternallyManagedSlice(grpc_slice_refcount* ref, size_t length, - uint8_t* bytes) { - refcount = ref; - data.refcounted.length = length; - data.refcounted.bytes = bytes; - } - bool operator==(const grpc_slice& other) const { - return data.refcounted.length == GRPC_SLICE_LENGTH(other) && - memcmp(data.refcounted.bytes, GRPC_SLICE_START_PTR(other), - data.refcounted.length) == 0; - } - bool operator!=(const grpc_slice& other) const { return !(*this == other); } - uint32_t Hash() { - return gpr_murmur_hash3(data.refcounted.bytes, data.refcounted.length, - g_hash_seed); - } -}; - -struct StaticMetadataSlice : public ManagedMemorySlice { - StaticMetadataSlice(grpc_slice_refcount* ref, size_t length, - const uint8_t* bytes) { - refcount = ref; - data.refcounted.length = length; - // NB: grpc_slice may or may not point to a static slice, but we are - // definitely pointing to static data here. Since we are not changing - // the underlying C-type, we need a const_cast here. - data.refcounted.bytes = const_cast(bytes); - } -}; - -struct InternedSliceRefcount; -struct InternedSlice : public ManagedMemorySlice { - explicit InternedSlice(InternedSliceRefcount* s); -}; - -// Converts grpc_slice to absl::string_view. -inline absl::string_view StringViewFromSlice(const grpc_slice& slice) { - return absl::string_view( - reinterpret_cast(GRPC_SLICE_START_PTR(slice)), - GRPC_SLICE_LENGTH(slice)); -} - -} // namespace grpc_core - -#endif /* GRPC_CORE_LIB_SLICE_SLICE_UTILS_H */ diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 52e5a161f90..06b517fab4c 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -50,9 +50,9 @@ #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/profiling/timers.h" #include "src/core/lib/resource_quota/arena.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_split.h" #include "src/core/lib/slice/slice_string_helpers.h" -#include "src/core/lib/slice/slice_utils.h" #include "src/core/lib/surface/api_trace.h" #include "src/core/lib/surface/call_test_only.h" #include "src/core/lib/surface/channel.h" diff --git a/src/core/lib/surface/init.cc b/src/core/lib/surface/init.cc index 9c0a588f880..191ef4f0610 100644 --- a/src/core/lib/surface/init.cc +++ b/src/core/lib/surface/init.cc @@ -104,7 +104,6 @@ void grpc_init(void) { grpc_core::Fork::GlobalInit(); grpc_fork_handlers_auto_register(); grpc_stats_init(); - grpc_slice_intern_init(); grpc_core::channelz::ChannelzRegistry::Init(); grpc_security_pre_init(); grpc_core::ApplicationCallbackExecCtx::GlobalInit(); @@ -140,7 +139,6 @@ void grpc_shutdown_internal_locked(void) grpc_iomgr_shutdown(); gpr_timers_global_destroy(); grpc_tracer_shutdown(); - grpc_slice_intern_shutdown(); grpc_core::channelz::ChannelzRegistry::Shutdown(); grpc_stats_shutdown(); grpc_core::Fork::GlobalShutdown(); diff --git a/src/core/lib/surface/server.cc b/src/core/lib/surface/server.cc index 9c4c669e413..da38f98900a 100644 --- a/src/core/lib/surface/server.cc +++ b/src/core/lib/surface/server.cc @@ -982,19 +982,7 @@ class Server::ChannelData::ConnectivityWatcher // Server::ChannelData::~ChannelData() { - if (registered_methods_ != nullptr) { - for (const ChannelRegisteredMethod& crm : *registered_methods_) { - grpc_slice_unref_internal(crm.method); - GPR_DEBUG_ASSERT(crm.method.refcount == &kNoopRefcount || - crm.method.refcount == nullptr); - if (crm.has_host) { - grpc_slice_unref_internal(crm.host); - GPR_DEBUG_ASSERT(crm.host.refcount == &kNoopRefcount || - crm.host.refcount == nullptr); - } - } - registered_methods_.reset(); - } + registered_methods_.reset(); if (server_ != nullptr) { if (server_->channelz_node_ != nullptr && channelz_socket_uuid_ != 0) { server_->channelz_node_->RemoveChildSocket(channelz_socket_uuid_); @@ -1027,11 +1015,11 @@ void Server::ChannelData::InitTransport(RefCountedPtr server, registered_methods_ = absl::make_unique>(slots); for (std::unique_ptr& rm : server_->registered_methods_) { - ExternallyManagedSlice host; - ExternallyManagedSlice method(rm->method.c_str()); + Slice host; + Slice method = Slice::FromExternalString(rm->method); const bool has_host = !rm->host.empty(); if (has_host) { - host = ExternallyManagedSlice(rm->host.c_str()); + host = Slice::FromExternalString(rm->host.c_str()); } uint32_t hash = MixHash32(has_host ? host.Hash() : 0, method.Hash()); uint32_t probes = 0; @@ -1046,9 +1034,9 @@ void Server::ChannelData::InitTransport(RefCountedPtr server, crm->flags = rm->flags; crm->has_host = has_host; if (has_host) { - crm->host = host; + crm->host = std::move(host); } - crm->method = method; + crm->method = std::move(method); } GPR_ASSERT(slots <= UINT32_MAX); registered_method_max_probes_ = max_probes; diff --git a/src/core/lib/surface/server.h b/src/core/lib/surface/server.h index 4e83c3ef45c..276d917fc06 100644 --- a/src/core/lib/surface/server.h +++ b/src/core/lib/surface/server.h @@ -176,8 +176,8 @@ class Server : public InternallyRefCounted, RegisteredMethod* server_registered_method = nullptr; uint32_t flags; bool has_host; - ExternallyManagedSlice method; - ExternallyManagedSlice host; + Slice method; + Slice host; }; class RequestMatcherInterface; diff --git a/src/cpp/ext/filters/census/client_filter.cc b/src/cpp/ext/filters/census/client_filter.cc index e1e26d19f72..16438b3f3cd 100644 --- a/src/cpp/ext/filters/census/client_filter.cc +++ b/src/cpp/ext/filters/census/client_filter.cc @@ -100,9 +100,9 @@ void OpenCensusCallTracer::OpenCensusCallAttemptTracer:: size_t tracing_len = TraceContextSerialize(context_.Context(), tracing_buf, kMaxTraceContextLen); if (tracing_len > 0) { - send_initial_metadata->Set(grpc_core::GrpcTraceBinMetadata(), - grpc_core::Slice(grpc_core::UnmanagedMemorySlice( - tracing_buf, tracing_len))); + send_initial_metadata->Set( + grpc_core::GrpcTraceBinMetadata(), + grpc_core::Slice::FromCopiedBuffer(tracing_buf, tracing_len)); } grpc_slice tags = grpc_empty_slice(); // TODO(unknown): Add in tagging serialization. diff --git a/src/cpp/ext/filters/census/server_filter.cc b/src/cpp/ext/filters/census/server_filter.cc index 2653c552952..011cd420755 100644 --- a/src/cpp/ext/filters/census/server_filter.cc +++ b/src/cpp/ext/filters/census/server_filter.cc @@ -128,7 +128,7 @@ void CensusServerCallData::StartTransportStreamOpBatch( if (len > 0) { op->send_trailing_metadata()->batch()->Set( grpc_core::GrpcServerStatsBinMetadata(), - grpc_core::Slice(grpc_core::UnmanagedMemorySlice(stats_buf_, len))); + grpc_core::Slice::FromCopiedBuffer(stats_buf_, len)); } } // Call next op. diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index eb1c8ce0d22..3cae45aee80 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -603,7 +603,6 @@ CORE_SOURCE_FILES = [ 'src/core/lib/slice/slice.cc', 'src/core/lib/slice/slice_api.cc', 'src/core/lib/slice/slice_buffer.cc', - 'src/core/lib/slice/slice_intern.cc', 'src/core/lib/slice/slice_refcount.cc', 'src/core/lib/slice/slice_split.cc', 'src/core/lib/slice/slice_string_helpers.cc', diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 72eedf5263f..d54a5b76519 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -213,7 +213,6 @@ grpc_slice_new_with_user_data_type grpc_slice_new_with_user_data_import; grpc_slice_new_with_len_type grpc_slice_new_with_len_import; grpc_slice_malloc_type grpc_slice_malloc_import; grpc_slice_malloc_large_type grpc_slice_malloc_large_import; -grpc_slice_intern_type grpc_slice_intern_import; grpc_slice_from_copied_string_type grpc_slice_from_copied_string_import; grpc_slice_from_copied_buffer_type grpc_slice_from_copied_buffer_import; grpc_slice_from_static_string_type grpc_slice_from_static_string_import; @@ -224,8 +223,6 @@ grpc_slice_split_tail_type grpc_slice_split_tail_import; grpc_slice_split_tail_maybe_ref_type grpc_slice_split_tail_maybe_ref_import; grpc_slice_split_head_type grpc_slice_split_head_import; grpc_empty_slice_type grpc_empty_slice_import; -grpc_slice_default_hash_impl_type grpc_slice_default_hash_impl_import; -grpc_slice_default_eq_impl_type grpc_slice_default_eq_impl_import; grpc_slice_eq_type grpc_slice_eq_import; grpc_slice_cmp_type grpc_slice_cmp_import; grpc_slice_str_cmp_type grpc_slice_str_cmp_import; @@ -233,7 +230,6 @@ grpc_slice_buf_start_eq_type grpc_slice_buf_start_eq_import; grpc_slice_rchr_type grpc_slice_rchr_import; grpc_slice_chr_type grpc_slice_chr_import; grpc_slice_slice_type grpc_slice_slice_import; -grpc_slice_hash_type grpc_slice_hash_import; grpc_slice_is_equivalent_type grpc_slice_is_equivalent_import; grpc_slice_dup_type grpc_slice_dup_import; grpc_slice_to_c_string_type grpc_slice_to_c_string_import; @@ -503,7 +499,6 @@ void grpc_rb_load_imports(HMODULE library) { grpc_slice_new_with_len_import = (grpc_slice_new_with_len_type) GetProcAddress(library, "grpc_slice_new_with_len"); grpc_slice_malloc_import = (grpc_slice_malloc_type) GetProcAddress(library, "grpc_slice_malloc"); grpc_slice_malloc_large_import = (grpc_slice_malloc_large_type) GetProcAddress(library, "grpc_slice_malloc_large"); - grpc_slice_intern_import = (grpc_slice_intern_type) GetProcAddress(library, "grpc_slice_intern"); grpc_slice_from_copied_string_import = (grpc_slice_from_copied_string_type) GetProcAddress(library, "grpc_slice_from_copied_string"); grpc_slice_from_copied_buffer_import = (grpc_slice_from_copied_buffer_type) GetProcAddress(library, "grpc_slice_from_copied_buffer"); grpc_slice_from_static_string_import = (grpc_slice_from_static_string_type) GetProcAddress(library, "grpc_slice_from_static_string"); @@ -514,8 +509,6 @@ void grpc_rb_load_imports(HMODULE library) { grpc_slice_split_tail_maybe_ref_import = (grpc_slice_split_tail_maybe_ref_type) GetProcAddress(library, "grpc_slice_split_tail_maybe_ref"); grpc_slice_split_head_import = (grpc_slice_split_head_type) GetProcAddress(library, "grpc_slice_split_head"); grpc_empty_slice_import = (grpc_empty_slice_type) GetProcAddress(library, "grpc_empty_slice"); - grpc_slice_default_hash_impl_import = (grpc_slice_default_hash_impl_type) GetProcAddress(library, "grpc_slice_default_hash_impl"); - grpc_slice_default_eq_impl_import = (grpc_slice_default_eq_impl_type) GetProcAddress(library, "grpc_slice_default_eq_impl"); grpc_slice_eq_import = (grpc_slice_eq_type) GetProcAddress(library, "grpc_slice_eq"); grpc_slice_cmp_import = (grpc_slice_cmp_type) GetProcAddress(library, "grpc_slice_cmp"); grpc_slice_str_cmp_import = (grpc_slice_str_cmp_type) GetProcAddress(library, "grpc_slice_str_cmp"); @@ -523,7 +516,6 @@ void grpc_rb_load_imports(HMODULE library) { grpc_slice_rchr_import = (grpc_slice_rchr_type) GetProcAddress(library, "grpc_slice_rchr"); grpc_slice_chr_import = (grpc_slice_chr_type) GetProcAddress(library, "grpc_slice_chr"); grpc_slice_slice_import = (grpc_slice_slice_type) GetProcAddress(library, "grpc_slice_slice"); - grpc_slice_hash_import = (grpc_slice_hash_type) GetProcAddress(library, "grpc_slice_hash"); grpc_slice_is_equivalent_import = (grpc_slice_is_equivalent_type) GetProcAddress(library, "grpc_slice_is_equivalent"); grpc_slice_dup_import = (grpc_slice_dup_type) GetProcAddress(library, "grpc_slice_dup"); grpc_slice_to_c_string_import = (grpc_slice_to_c_string_type) GetProcAddress(library, "grpc_slice_to_c_string"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index 965d7c69b9c..30f1a2e601f 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -614,9 +614,6 @@ extern grpc_slice_malloc_type grpc_slice_malloc_import; typedef grpc_slice(*grpc_slice_malloc_large_type)(size_t length); extern grpc_slice_malloc_large_type grpc_slice_malloc_large_import; #define grpc_slice_malloc_large grpc_slice_malloc_large_import -typedef grpc_slice(*grpc_slice_intern_type)(grpc_slice slice); -extern grpc_slice_intern_type grpc_slice_intern_import; -#define grpc_slice_intern grpc_slice_intern_import typedef grpc_slice(*grpc_slice_from_copied_string_type)(const char* source); extern grpc_slice_from_copied_string_type grpc_slice_from_copied_string_import; #define grpc_slice_from_copied_string grpc_slice_from_copied_string_import @@ -647,12 +644,6 @@ extern grpc_slice_split_head_type grpc_slice_split_head_import; typedef grpc_slice(*grpc_empty_slice_type)(void); extern grpc_empty_slice_type grpc_empty_slice_import; #define grpc_empty_slice grpc_empty_slice_import -typedef uint32_t(*grpc_slice_default_hash_impl_type)(grpc_slice s); -extern grpc_slice_default_hash_impl_type grpc_slice_default_hash_impl_import; -#define grpc_slice_default_hash_impl grpc_slice_default_hash_impl_import -typedef int(*grpc_slice_default_eq_impl_type)(grpc_slice a, grpc_slice b); -extern grpc_slice_default_eq_impl_type grpc_slice_default_eq_impl_import; -#define grpc_slice_default_eq_impl grpc_slice_default_eq_impl_import typedef int(*grpc_slice_eq_type)(grpc_slice a, grpc_slice b); extern grpc_slice_eq_type grpc_slice_eq_import; #define grpc_slice_eq grpc_slice_eq_import @@ -674,9 +665,6 @@ extern grpc_slice_chr_type grpc_slice_chr_import; typedef int(*grpc_slice_slice_type)(grpc_slice haystack, grpc_slice needle); extern grpc_slice_slice_type grpc_slice_slice_import; #define grpc_slice_slice grpc_slice_slice_import -typedef uint32_t(*grpc_slice_hash_type)(grpc_slice s); -extern grpc_slice_hash_type grpc_slice_hash_import; -#define grpc_slice_hash grpc_slice_hash_import typedef int(*grpc_slice_is_equivalent_type)(grpc_slice a, grpc_slice b); extern grpc_slice_is_equivalent_type grpc_slice_is_equivalent_import; #define grpc_slice_is_equivalent grpc_slice_is_equivalent_import diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc index 2d83fdda98d..e1c357596f1 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.cc +++ b/test/core/end2end/fuzzers/api_fuzzer.cc @@ -381,11 +381,6 @@ class Call : public std::enable_shared_from_this { template grpc_slice ReadSlice(const T& s) { grpc_slice slice = grpc_slice_from_cpp_string(s.value()); - if (s.intern()) { - auto interned_slice = grpc_slice_intern(slice); - grpc_slice_unref(slice); - slice = interned_slice; - } unref_slices_.push_back(slice); return slice; } diff --git a/test/core/end2end/fuzzers/api_fuzzer.proto b/test/core/end2end/fuzzers/api_fuzzer.proto index af867c3a931..eab3551cb01 100644 --- a/test/core/end2end/fuzzers/api_fuzzer.proto +++ b/test/core/end2end/fuzzers/api_fuzzer.proto @@ -19,13 +19,15 @@ package api_fuzzer; message Empty {} message ByteSlice { + reserved 2; + reserved "intern"; bytes value = 1; - bool intern = 2; } message StringSlice { + reserved 2; + reserved "intern"; string value = 1; - bool intern = 2; } message ResourceQuota {} diff --git a/test/core/slice/BUILD b/test/core/slice/BUILD index 192443a4a29..a51b697deed 100644 --- a/test/core/slice/BUILD +++ b/test/core/slice/BUILD @@ -95,18 +95,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "slice_intern_test", - srcs = ["slice_intern_test.cc"], - language = "C++", - uses_polling = False, - deps = [ - "//:gpr", - "//:grpc", - "//test/core/util:grpc_test_util", - ], -) - grpc_cc_test( name = "slice_string_helpers_test", srcs = ["slice_string_helpers_test.cc"], diff --git a/test/core/slice/slice_intern_test.cc b/test/core/slice/slice_intern_test.cc deleted file mode 100644 index 47e79c62d96..00000000000 --- a/test/core/slice/slice_intern_test.cc +++ /dev/null @@ -1,67 +0,0 @@ -/* - * - * Copyright 2015 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. - * - */ - -#include - -#include -#include - -#include -#include -#include -#include - -#include "src/core/lib/gprpp/memory.h" -#include "src/core/lib/slice/slice_internal.h" -#include "test/core/util/test_config.h" - -#define LOG_TEST_NAME(x) gpr_log(GPR_INFO, "%s", x); - -static void test_slice_interning(void) { - LOG_TEST_NAME("test_slice_interning"); - - grpc_init(); - grpc_slice src1 = grpc_slice_from_copied_string("hello123456789123456789"); - grpc_slice src2 = grpc_slice_from_copied_string("hello123456789123456789"); - - // Explicitly checking that the slices are at different addresses prevents - // failure with windows opt 64bit build. - // See https://github.com/grpc/grpc/issues/20519 - GPR_ASSERT(&src1 != &src2); - GPR_ASSERT(GRPC_SLICE_START_PTR(src1) != GRPC_SLICE_START_PTR(src2)); - - grpc_slice interned1 = grpc_slice_intern(src1); - grpc_slice interned2 = grpc_slice_intern(src2); - GPR_ASSERT(GRPC_SLICE_START_PTR(interned1) == - GRPC_SLICE_START_PTR(interned2)); - GPR_ASSERT(GRPC_SLICE_START_PTR(interned1) != GRPC_SLICE_START_PTR(src1)); - GPR_ASSERT(GRPC_SLICE_START_PTR(interned2) != GRPC_SLICE_START_PTR(src2)); - grpc_slice_unref(src1); - grpc_slice_unref(src2); - grpc_slice_unref(interned1); - grpc_slice_unref(interned2); - grpc_shutdown(); -} - -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(argc, argv); - grpc_init(); - test_slice_interning(); - grpc_shutdown(); - return 0; -} diff --git a/test/core/slice/slice_test.cc b/test/core/slice/slice_test.cc index 1f5ea505054..1bf2de65d12 100644 --- a/test/core/slice/slice_test.cc +++ b/test/core/slice/slice_test.cc @@ -358,8 +358,7 @@ size_t SumSlice(const Slice& slice) { TEST(SliceTest, ExternalAsOwned) { auto external_string = absl::make_unique(RandomString(1024)); - Slice slice(ExternallyManagedSlice(external_string->data(), - external_string->length())); + Slice slice = Slice::FromExternalString(*external_string); const auto initial_sum = SumSlice(slice); Slice owned = slice.AsOwned(); EXPECT_EQ(initial_sum, SumSlice(owned)); @@ -375,9 +374,7 @@ TEST(SliceTest, ExternalAsOwned) { TEST(SliceTest, ExternalTakeOwned) { std::unique_ptr external_string( new std::string(RandomString(1024))); - SumSlice(Slice(ExternallyManagedSlice(external_string->data(), - external_string->length())) - .TakeOwned()); + SumSlice(Slice::FromExternalString(*external_string).TakeOwned()); } TEST(SliceTest, StaticSlice) { diff --git a/test/core/surface/public_headers_must_be_c89.c b/test/core/surface/public_headers_must_be_c89.c index df34da9a40f..2a28a1f704f 100644 --- a/test/core/surface/public_headers_must_be_c89.c +++ b/test/core/surface/public_headers_must_be_c89.c @@ -257,7 +257,6 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_slice_new_with_len); printf("%lx", (unsigned long) grpc_slice_malloc); printf("%lx", (unsigned long) grpc_slice_malloc_large); - printf("%lx", (unsigned long) grpc_slice_intern); printf("%lx", (unsigned long) grpc_slice_from_copied_string); printf("%lx", (unsigned long) grpc_slice_from_copied_buffer); printf("%lx", (unsigned long) grpc_slice_from_static_string); @@ -268,8 +267,6 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_slice_split_tail_maybe_ref); printf("%lx", (unsigned long) grpc_slice_split_head); printf("%lx", (unsigned long) grpc_empty_slice); - printf("%lx", (unsigned long) grpc_slice_default_hash_impl); - printf("%lx", (unsigned long) grpc_slice_default_eq_impl); printf("%lx", (unsigned long) grpc_slice_eq); printf("%lx", (unsigned long) grpc_slice_cmp); printf("%lx", (unsigned long) grpc_slice_str_cmp); @@ -277,7 +274,6 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_slice_rchr); printf("%lx", (unsigned long) grpc_slice_chr); printf("%lx", (unsigned long) grpc_slice_slice); - printf("%lx", (unsigned long) grpc_slice_hash); printf("%lx", (unsigned long) grpc_slice_is_equivalent); printf("%lx", (unsigned long) grpc_slice_dup); printf("%lx", (unsigned long) grpc_slice_to_c_string); diff --git a/test/core/transport/chttp2/hpack_encoder_test.cc b/test/core/transport/chttp2/hpack_encoder_test.cc index 9d11f41a7bf..c49c4372326 100644 --- a/test/core/transport/chttp2/hpack_encoder_test.cc +++ b/test/core/transport/chttp2/hpack_encoder_test.cc @@ -50,7 +50,6 @@ int g_failure = 0; typedef struct { bool eof; bool use_true_binary_metadata; - bool only_intern_key; } verify_params; /* verify that the output frames that are generated by encoding the stream @@ -169,11 +168,8 @@ static void verify(const verify_params params, const char* expected, for (i = 0; i < nheaders; i++) { char* key = va_arg(l, char*); char* value = va_arg(l, char*); - grpc_slice value_slice = grpc_slice_from_static_string(value); - if (!params.only_intern_key) { - value_slice = grpc_slice_intern(value_slice); - } - b.Append(key, grpc_core::Slice(value_slice), CrashOnAppendError); + b.Append(key, grpc_core::Slice::FromStaticString(value), + CrashOnAppendError); } va_end(l); @@ -212,7 +208,6 @@ static void test_basic_headers() { verify_params params = { false, false, - false, }; verify(params, "000005 0104 deadbeef 00 0161 0161", 1, "a", "a"); verify(params, "00000a 0104 deadbeef 00 0161 0161 00 0162 0163", 2, "a", "a", diff --git a/test/core/util/evaluate_args_test_util.h b/test/core/util/evaluate_args_test_util.h index 27f5b445f63..6beadf56860 100644 --- a/test/core/util/evaluate_args_test_util.h +++ b/test/core/util/evaluate_args_test_util.h @@ -33,12 +33,11 @@ class EvaluateArgsTestUtil { ~EvaluateArgsTestUtil() { delete channel_args_; } void AddPairToMetadata(const char* key, const char* value) { - metadata_.Append( - key, Slice(grpc_slice_intern(grpc_slice_from_static_string(value))), - [](absl::string_view, const Slice&) { - // We should never ever see an error here. - abort(); - }); + metadata_.Append(key, Slice::FromStaticString(value), + [](absl::string_view, const Slice&) { + // We should never ever see an error here. + abort(); + }); } void SetLocalEndpoint(absl::string_view local_uri) { diff --git a/test/cpp/end2end/channelz_service_test.cc b/test/cpp/end2end/channelz_service_test.cc index 1f9423565bf..027a08c9bfe 100644 --- a/test/cpp/end2end/channelz_service_test.cc +++ b/test/cpp/end2end/channelz_service_test.cc @@ -38,7 +38,7 @@ #include "src/core/lib/iomgr/load_file.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" #include "src/core/lib/security/security_connector/ssl_utils.h" -#include "src/core/lib/slice/slice_utils.h" +#include "src/core/lib/slice/slice_internal.h" #include "src/cpp/client/secure_credentials.h" #include "src/proto/grpc/channelz/channelz.grpc.pb.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index ff98ccc6814..3d489fe6d4f 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2225,7 +2225,6 @@ src/core/lib/slice/slice.cc \ src/core/lib/slice/slice.h \ src/core/lib/slice/slice_api.cc \ src/core/lib/slice/slice_buffer.cc \ -src/core/lib/slice/slice_intern.cc \ src/core/lib/slice/slice_internal.h \ src/core/lib/slice/slice_refcount.cc \ src/core/lib/slice/slice_refcount.h \ @@ -2234,7 +2233,6 @@ src/core/lib/slice/slice_split.cc \ src/core/lib/slice/slice_split.h \ src/core/lib/slice/slice_string_helpers.cc \ src/core/lib/slice/slice_string_helpers.h \ -src/core/lib/slice/slice_utils.h \ src/core/lib/surface/api_trace.cc \ src/core/lib/surface/api_trace.h \ src/core/lib/surface/builtins.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 2ef8f04f4a4..f3b325c106f 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -2024,7 +2024,6 @@ src/core/lib/slice/slice.cc \ src/core/lib/slice/slice.h \ src/core/lib/slice/slice_api.cc \ src/core/lib/slice/slice_buffer.cc \ -src/core/lib/slice/slice_intern.cc \ src/core/lib/slice/slice_internal.h \ src/core/lib/slice/slice_refcount.cc \ src/core/lib/slice/slice_refcount.h \ @@ -2033,7 +2032,6 @@ src/core/lib/slice/slice_split.cc \ src/core/lib/slice/slice_split.h \ src/core/lib/slice/slice_string_helpers.cc \ src/core/lib/slice/slice_string_helpers.h \ -src/core/lib/slice/slice_utils.h \ src/core/lib/surface/README.md \ src/core/lib/surface/api_trace.cc \ src/core/lib/surface/api_trace.h \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 62622f970fe..d5b6104a6b7 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2151,30 +2151,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "slice_intern_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false,