diff --git a/BUILD b/BUILD index 17d763d72ee..ed081cd1a17 100644 --- a/BUILD +++ b/BUILD @@ -1541,7 +1541,6 @@ grpc_cc_library( ], deps = [ "gpr_base", - "ref_counted", ], ) @@ -1552,6 +1551,7 @@ grpc_cc_library( "src/core/lib/slice/slice_string_helpers.cc", ], hdrs = [ + "src/core/lib/slice/slice.h", "src/core/lib/slice/slice_internal.h", "src/core/lib/slice/slice_string_helpers.h", ], diff --git a/CMakeLists.txt b/CMakeLists.txt index d21fed5cbef..ad44908acd0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -696,7 +696,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_c status_conversion_test) add_dependencies(buildtests_c stream_compression_test) add_dependencies(buildtests_c stream_map_test) - add_dependencies(buildtests_c stream_owned_slice_test) add_dependencies(buildtests_c string_test) add_dependencies(buildtests_c sync_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -710,7 +709,6 @@ if(gRPC_BUILD_TESTS) endif() add_dependencies(buildtests_c test_core_gpr_time_test) add_dependencies(buildtests_c test_core_security_credentials_test) - add_dependencies(buildtests_c test_core_slice_slice_test) add_dependencies(buildtests_c thd_test) add_dependencies(buildtests_c threadpool_test) add_dependencies(buildtests_c time_averaged_stats_test) @@ -939,6 +937,7 @@ if(gRPC_BUILD_TESTS) endif() add_dependencies(buildtests_cxx string_ref_test) add_dependencies(buildtests_cxx table_test) + add_dependencies(buildtests_cxx test_core_slice_slice_test) add_dependencies(buildtests_cxx test_cpp_client_credentials_test) add_dependencies(buildtests_cxx test_cpp_server_credentials_test) add_dependencies(buildtests_cxx test_cpp_util_slice_test) @@ -2208,6 +2207,7 @@ endif() if(gRPC_BUILD_TESTS) add_library(grpc_test_util + test/core/util/build.cc test/core/util/cmdline.cc test/core/util/fuzzer_util.cc test/core/util/grpc_profiler.cc @@ -2276,6 +2276,7 @@ endif() if(gRPC_BUILD_TESTS) add_library(grpc_test_util_unsecure + test/core/util/build.cc test/core/util/cmdline.cc test/core/util/fuzzer_util.cc test/core/util/grpc_profiler.cc @@ -7041,33 +7042,6 @@ target_link_libraries(stream_map_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(stream_owned_slice_test - test/core/transport/stream_owned_slice_test.cc -) - -target_include_directories(stream_owned_slice_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(stream_owned_slice_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) @@ -7264,38 +7238,6 @@ target_link_libraries(test_core_security_credentials_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(test_core_slice_slice_test - src/core/lib/debug/trace.cc - src/core/lib/slice/slice.cc - src/core/lib/slice/slice_refcount.cc - src/core/lib/slice/slice_string_helpers.cc - src/core/lib/slice/static_slice.cc - test/core/slice/slice_test.cc -) - -target_include_directories(test_core_slice_slice_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(test_core_slice_slice_test - ${_gRPC_ALLTARGETS_LIBRARIES} - gpr -) - - endif() if(gRPC_BUILD_TESTS) @@ -15728,6 +15670,47 @@ target_link_libraries(table_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(test_core_slice_slice_test + src/core/lib/debug/trace.cc + src/core/lib/slice/slice.cc + src/core/lib/slice/slice_refcount.cc + src/core/lib/slice/slice_string_helpers.cc + src/core/lib/slice/static_slice.cc + test/core/slice/slice_test.cc + test/core/util/build.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + +target_include_directories(test_core_slice_slice_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} + third_party/googletest/googletest/include + third_party/googletest/googletest + third_party/googletest/googlemock/include + third_party/googletest/googlemock + ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(test_core_slice_slice_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + gpr +) + + endif() if(gRPC_BUILD_TESTS) @@ -16794,6 +16777,7 @@ if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.h ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.h + test/core/util/build.cc test/core/util/cmdline.cc test/core/util/fuzzer_util.cc test/core/util/grpc_profiler.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 14f759345a8..9bffdfc3e4a 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -914,6 +914,7 @@ libs: - src/core/lib/security/util/json_util.h - src/core/lib/slice/b64.h - src/core/lib/slice/percent_encoding.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -1583,6 +1584,7 @@ libs: language: c public_headers: [] headers: + - test/core/util/build.h - test/core/util/cmdline.h - test/core/util/evaluate_args_test_util.h - test/core/util/fuzzer_util.h @@ -1605,6 +1607,7 @@ libs: - test/core/util/tls_utils.h - test/core/util/tracer_util.h src: + - test/core/util/build.cc - test/core/util/cmdline.cc - test/core/util/fuzzer_util.cc - test/core/util/grpc_profiler.cc @@ -1636,6 +1639,7 @@ libs: language: c public_headers: [] headers: + - test/core/util/build.h - test/core/util/cmdline.h - test/core/util/evaluate_args_test_util.h - test/core/util/fuzzer_util.h @@ -1657,6 +1661,7 @@ libs: - test/core/util/test_tcp_server.h - test/core/util/tracer_util.h src: + - test/core/util/build.cc - test/core/util/cmdline.cc - test/core/util/fuzzer_util.cc - test/core/util/grpc_profiler.cc @@ -1938,6 +1943,7 @@ libs: - src/core/lib/resource_quota/trace.h - src/core/lib/slice/b64.h - src/core/lib/slice/percent_encoding.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -3732,6 +3738,7 @@ targets: - src/core/lib/promise/seq.h - src/core/lib/resource_quota/memory_quota.h - src/core/lib/resource_quota/trace.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -4049,6 +4056,7 @@ targets: - src/core/lib/gprpp/atomic_utils.h - src/core/lib/gprpp/ref_counted.h - src/core/lib/gprpp/ref_counted_ptr.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -4142,15 +4150,6 @@ targets: - test/core/transport/chttp2/stream_map_test.cc deps: - grpc_test_util -- name: stream_owned_slice_test - build: test - language: c - headers: [] - src: - - test/core/transport/stream_owned_slice_test.cc - deps: - - grpc_test_util - uses_polling: false - name: string_test build: test language: c @@ -4223,30 +4222,6 @@ targets: - test/core/security/credentials_test.cc deps: - grpc_test_util -- name: test_core_slice_slice_test - build: test - language: c - headers: - - src/core/lib/debug/trace.h - - src/core/lib/gprpp/atomic_utils.h - - src/core/lib/gprpp/ref_counted.h - - src/core/lib/gprpp/ref_counted_ptr.h - - src/core/lib/slice/slice_internal.h - - 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/core/lib/slice/static_slice.h - src: - - src/core/lib/debug/trace.cc - - src/core/lib/slice/slice.cc - - src/core/lib/slice/slice_refcount.cc - - src/core/lib/slice/slice_string_helpers.cc - - src/core/lib/slice/static_slice.cc - - test/core/slice/slice_test.cc - deps: - - gpr - uses_polling: false - name: thd_test build: test language: c @@ -5725,6 +5700,7 @@ targets: - src/core/lib/promise/detail/status.h - src/core/lib/promise/exec_ctx_wakeup_scheduler.h - src/core/lib/promise/poll.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -6807,6 +6783,7 @@ targets: - src/core/lib/promise/seq.h - src/core/lib/resource_quota/memory_quota.h - src/core/lib/resource_quota/trace.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -7530,6 +7507,7 @@ targets: - src/core/lib/resource_quota/resource_quota.h - src/core/lib/resource_quota/thread_quota.h - src/core/lib/resource_quota/trace.h + - src/core/lib/slice/slice.h - src/core/lib/slice/slice_internal.h - src/core/lib/slice/slice_refcount.h - src/core/lib/slice/slice_refcount_base.h @@ -7986,6 +7964,34 @@ targets: - absl/types:optional - absl/utility:utility uses_polling: false +- name: test_core_slice_slice_test + gtest: true + build: test + language: c++ + headers: + - src/core/lib/debug/trace.h + - src/core/lib/gprpp/atomic_utils.h + - src/core/lib/gprpp/ref_counted.h + - src/core/lib/gprpp/ref_counted_ptr.h + - src/core/lib/slice/slice.h + - src/core/lib/slice/slice_internal.h + - 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/core/lib/slice/static_slice.h + - test/core/util/build.h + src: + - src/core/lib/debug/trace.cc + - src/core/lib/slice/slice.cc + - src/core/lib/slice/slice_refcount.cc + - src/core/lib/slice/slice_string_helpers.cc + - src/core/lib/slice/static_slice.cc + - test/core/slice/slice_test.cc + - test/core/util/build.cc + deps: + - gpr + uses_polling: false - name: test_cpp_client_credentials_test gtest: true build: test @@ -8536,6 +8542,7 @@ targets: build: test language: c++ headers: + - test/core/util/build.h - test/core/util/cmdline.h - test/core/util/evaluate_args_test_util.h - test/core/util/fuzzer_util.h @@ -8560,6 +8567,7 @@ targets: - src/proto/grpc/testing/echo.proto - src/proto/grpc/testing/echo_messages.proto - src/proto/grpc/testing/simple_messages.proto + - test/core/util/build.cc - test/core/util/cmdline.cc - test/core/util/fuzzer_util.cc - test/core/util/grpc_profiler.cc diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index cd196de9df2..7901c6ebe4a 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -773,6 +773,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.h', 'src/core/lib/slice/b64.h', 'src/core/lib/slice/percent_encoding.h', + 'src/core/lib/slice/slice.h', 'src/core/lib/slice/slice_internal.h', 'src/core/lib/slice/slice_refcount.h', 'src/core/lib/slice/slice_refcount_base.h', @@ -1484,6 +1485,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.h', 'src/core/lib/slice/b64.h', 'src/core/lib/slice/percent_encoding.h', + 'src/core/lib/slice/slice.h', 'src/core/lib/slice/slice_internal.h', 'src/core/lib/slice/slice_refcount.h', 'src/core/lib/slice/slice_refcount_base.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 1c4f9b6888c..4f32d80154b 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1276,6 +1276,7 @@ Pod::Spec.new do |s| 'src/core/lib/slice/percent_encoding.cc', 'src/core/lib/slice/percent_encoding.h', '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', @@ -2008,6 +2009,7 @@ Pod::Spec.new do |s| 'src/core/lib/security/util/json_util.h', 'src/core/lib/slice/b64.h', 'src/core/lib/slice/percent_encoding.h', + 'src/core/lib/slice/slice.h', 'src/core/lib/slice/slice_internal.h', 'src/core/lib/slice/slice_refcount.h', 'src/core/lib/slice/slice_refcount_base.h', @@ -2270,6 +2272,8 @@ Pod::Spec.new do |s| 'test/core/end2end/tests/trailing_metadata.cc', 'test/core/end2end/tests/write_buffering.cc', 'test/core/end2end/tests/write_buffering_at_end.cc', + 'test/core/util/build.cc', + 'test/core/util/build.h', 'test/core/util/cmdline.cc', 'test/core/util/cmdline.h', 'test/core/util/evaluate_args_test_util.h', diff --git a/grpc.gemspec b/grpc.gemspec index 43c4e6be18b..2be20dec14a 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1196,6 +1196,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/slice/percent_encoding.cc ) s.files += %w( src/core/lib/slice/percent_encoding.h ) s.files += %w( src/core/lib/slice/slice.cc ) + 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 ) diff --git a/grpc.gyp b/grpc.gyp index a4194c1cc12..07e2df3d6da 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -1084,6 +1084,7 @@ 'grpc', ], 'sources': [ + 'test/core/util/build.cc', 'test/core/util/cmdline.cc', 'test/core/util/fuzzer_util.cc', 'test/core/util/grpc_profiler.cc', @@ -1117,6 +1118,7 @@ 'grpc_unsecure', ], 'sources': [ + 'test/core/util/build.cc', 'test/core/util/cmdline.cc', 'test/core/util/fuzzer_util.cc', 'test/core/util/grpc_profiler.cc', diff --git a/package.xml b/package.xml index 31ab5a475eb..61b10179b93 100644 --- a/package.xml +++ b/package.xml @@ -1176,6 +1176,7 @@ + diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 4b79a3b813c..30707e06ab7 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -2428,16 +2428,9 @@ class ClientChannel::LoadBalancedCall::Metadata std::vector> TestOnlyCopyToVector() override { - std::vector> result; - batch_->ForEach([&](grpc_mdelem md) { - auto key = std::string(StringViewFromSlice(GRPC_MDKEY(md))); - if (key != ":path") { - result.push_back( - std::make_pair(std::move(key), - std::string(StringViewFromSlice(GRPC_MDVALUE(md))))); - } - }); - return result; + Encoder encoder; + batch_->Encode(&encoder); + return encoder.Take(); } absl::optional Lookup(absl::string_view key, @@ -2446,6 +2439,33 @@ class ClientChannel::LoadBalancedCall::Metadata } private: + class Encoder { + public: + void Encode(grpc_mdelem md) { + auto key = StringViewFromSlice(GRPC_MDKEY(md)); + if (key != ":path") { + out_.emplace_back(std::string(key), + std::string(StringViewFromSlice(GRPC_MDVALUE(md)))); + } + } + + template + void Encode(Which, const typename Which::ValueType& value) { + auto value_slice = Which::Encode(value); + out_.emplace_back(std::string(Which::key()), + std::string(value_slice.as_string_view())); + } + + void Encode(GrpcTimeoutMetadata, grpc_millis) {} + + std::vector> Take() { + return std::move(out_); + } + + private: + std::vector> out_; + }; + LoadBalancedCall* lb_call_; grpc_metadata_batch* batch_; }; diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index 0804f907093..37fb8f1fe06 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -79,7 +79,6 @@ struct call_data { grpc_linked_mdelem method; grpc_linked_mdelem scheme; grpc_linked_mdelem content_type; - grpc_linked_mdelem user_agent; // State for handling recv_initial_metadata ops. grpc_metadata_batch* recv_initial_metadata; grpc_error_handle recv_initial_metadata_error = GRPC_ERROR_NONE; @@ -104,7 +103,7 @@ struct call_data { struct channel_data { grpc_mdelem static_scheme; - grpc_mdelem user_agent; + grpc_core::Slice user_agent; size_t max_payload_size_for_get; }; } // namespace @@ -445,9 +444,6 @@ static void http_client_start_transport_stream_op_batch( remove_if_present( batch->payload->send_initial_metadata.send_initial_metadata, GRPC_BATCH_CONTENT_TYPE); - remove_if_present( - batch->payload->send_initial_metadata.send_initial_metadata, - GRPC_BATCH_USER_AGENT); /* Send : prefixed headers, which have to be before any application layer headers. */ @@ -466,11 +462,8 @@ static void http_client_start_transport_stream_op_batch( &calld->content_type, GRPC_MDELEM_CONTENT_TYPE_APPLICATION_SLASH_GRPC, GRPC_BATCH_CONTENT_TYPE); if (error != GRPC_ERROR_NONE) goto done; - error = grpc_metadata_batch_add_tail( - batch->payload->send_initial_metadata.send_initial_metadata, - &calld->user_agent, GRPC_MDELEM_REF(channeld->user_agent), - GRPC_BATCH_USER_AGENT); - if (error != GRPC_ERROR_NONE) goto done; + batch->payload->send_initial_metadata.send_initial_metadata->Set( + grpc_core::UserAgentMetadata(), channeld->user_agent.Ref()); } done: @@ -534,8 +527,8 @@ static size_t max_payload_size_from_args(const grpc_channel_args* args) { return kMaxPayloadSizeForGet; } -static grpc_core::ManagedMemorySlice user_agent_from_args( - const grpc_channel_args* args, const char* transport_name) { +static grpc_core::Slice user_agent_from_args(const grpc_channel_args* args, + const char* transport_name) { std::vector user_agent_fields; for (size_t i = 0; args && i < args->num_args; i++) { @@ -565,29 +558,27 @@ static grpc_core::ManagedMemorySlice user_agent_from_args( } std::string user_agent_string = absl::StrJoin(user_agent_fields, " "); - return grpc_core::ManagedMemorySlice(user_agent_string.c_str()); + return grpc_core::Slice::FromCopiedString(user_agent_string.c_str()); } /* Constructor for channel_data */ static grpc_error_handle http_client_init_channel_elem( grpc_channel_element* elem, grpc_channel_element_args* args) { channel_data* chand = static_cast(elem->channel_data); + new (chand) channel_data(); GPR_ASSERT(!args->is_last); GPR_ASSERT(args->optional_transport != nullptr); chand->static_scheme = scheme_from_args(args->channel_args); chand->max_payload_size_for_get = max_payload_size_from_args(args->channel_args); - chand->user_agent = grpc_mdelem_from_slices( - GRPC_MDSTR_USER_AGENT, - user_agent_from_args(args->channel_args, - args->optional_transport->vtable->name)); + chand->user_agent = grpc_core::Slice(user_agent_from_args( + args->channel_args, args->optional_transport->vtable->name)); return GRPC_ERROR_NONE; } /* Destructor for channel data */ static void http_client_destroy_channel_elem(grpc_channel_element* elem) { - channel_data* chand = static_cast(elem->channel_data); - GRPC_MDELEM_UNREF(chand->user_agent); + static_cast(elem->channel_data)->~channel_data(); } const grpc_channel_filter grpc_http_client_filter = { diff --git a/src/core/ext/filters/http/server/http_server_filter.cc b/src/core/ext/filters/http/server/http_server_filter.cc index 0a2108d0051..abf6d190077 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -329,9 +329,8 @@ static grpc_error_handle hs_filter_incoming_metadata(grpc_call_element* elem, } channel_data* chand = static_cast(elem->channel_data); - if (!chand->surface_user_agent && - b->legacy_index()->named.user_agent != nullptr) { - b->Remove(GRPC_BATCH_USER_AGENT); + if (!chand->surface_user_agent) { + b->Remove(grpc_core::UserAgentMetadata()); } return error; diff --git a/src/core/ext/transport/binder/transport/binder_transport.cc b/src/core/ext/transport/binder/transport/binder_transport.cc index 68d393f11ea..1d0f94edfa2 100644 --- a/src/core/ext/transport/binder/transport/binder_transport.cc +++ b/src/core/ext/transport/binder/transport/binder_transport.cc @@ -124,7 +124,7 @@ static void AssignMetadata(grpc_metadata_batch* mb, const grpc_binder::Metadata& md) { mb->Clear(); for (auto& p : md) { - mb->Append(p.first, grpc_slice_from_cpp_string(p.second)); + mb->Append(p.first, grpc_core::Slice::FromCopiedString(p.second)); } } diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc index fe0910adba4..d70a30cc683 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc @@ -476,12 +476,20 @@ void HPackCompressor::Framer::EncodeDynamic(grpc_mdelem elem) { void HPackCompressor::Framer::Encode(TeMetadata, TeMetadata::ValueType value) { GPR_ASSERT(value == TeMetadata::ValueType::kTrailers); - if (compressor_->table_.ConvertableToDynamicIndex(compressor_->te_index_)) { - EmitIndexed(compressor_->table_.DynamicIndex(compressor_->te_index_)); + EncodeAlwaysIndexed( + &compressor_->te_index_, GRPC_MDSTR_TE, GRPC_MDSTR_TRAILERS, + 2 /* te */ + 8 /* trailers */ + hpack_constants::kEntryOverhead); +} + +void HPackCompressor::Framer::EncodeAlwaysIndexed(uint32_t* index, + const grpc_slice& key, + const grpc_slice& value, + uint32_t transport_length) { + if (compressor_->table_.ConvertableToDynamicIndex(*index)) { + EmitIndexed(compressor_->table_.DynamicIndex(*index)); } else { - compressor_->te_index_ = compressor_->table_.AllocateIndex( - 2 /* te */ + 8 /* trailers */ + hpack_constants::kEntryOverhead); - EmitLitHdrWithNonBinaryStringKeyIncIdx(GRPC_MDSTR_TE, GRPC_MDSTR_TRAILERS); + *index = compressor_->table_.AllocateIndex(transport_length); + EmitLitHdrWithNonBinaryStringKeyIncIdx(key, value); } } @@ -496,6 +504,16 @@ void HPackCompressor::Framer::Encode(GrpcTimeoutMetadata, GRPC_MDELEM_UNREF(mdelem); } +void HPackCompressor::Framer::Encode(UserAgentMetadata, const Slice& slice) { + if (!slice.is_equivalent(compressor_->user_agent_)) { + compressor_->user_agent_ = slice.Ref(); + compressor_->user_agent_index_ = 0; + } + EncodeAlwaysIndexed( + &compressor_->user_agent_index_, GRPC_MDSTR_USER_AGENT, slice.c_slice(), + 10 /* user-agent */ + slice.size() + hpack_constants::kEntryOverhead); +} + void HPackCompressor::SetMaxUsableSize(uint32_t max_table_size) { max_usable_size_ = max_table_size; SetMaxTableSize(std::min(table_.max_size(), max_table_size)); diff --git a/src/core/ext/transport/chttp2/transport/hpack_encoder.h b/src/core/ext/transport/chttp2/transport/hpack_encoder.h index 172578d2854..0702a391b53 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.h +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.h @@ -119,6 +119,7 @@ class HPackCompressor { void Encode(grpc_mdelem md); void Encode(GrpcTimeoutMetadata, grpc_millis deadline); void Encode(TeMetadata, TeMetadata::ValueType value); + void Encode(UserAgentMetadata, const Slice& slice); private: struct FramePrefix { @@ -145,6 +146,10 @@ class HPackCompressor { const grpc_slice& value_slice); void EmitLitHdrWithStringKeyNotIdx(grpc_mdelem elem); + void EncodeAlwaysIndexed(uint32_t* index, const grpc_slice& key, + const grpc_slice& value, + uint32_t transport_length); + size_t CurrentFrameSize() const; void Add(grpc_slice slice); uint8_t* AddTiny(size_t len); @@ -269,7 +274,12 @@ class HPackCompressor { // seen and *may* be in the decompressor table HPackEncoderIndex elem_index_; HPackEncoderIndex key_index_; + // Index into table_ for the te:trailers metadata element uint32_t te_index_ = 0; + // Index into table_ for the user-agent metadata element + uint32_t user_agent_index_ = 0; + // The user-agent string referred to by user_agent_index_ + Slice user_agent_; }; } // namespace grpc_core diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index eb2f2f456cf..1f10092dcb4 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -656,24 +656,10 @@ class HPackParser::String { private: // Forward declare take functions... we'll need them in the public interface - UnmanagedMemorySlice Take(Extern); - ManagedMemorySlice Take(Intern); + Slice Take(Extern); + Slice Take(Intern); public: - // If a String is a Slice then unref - ~String() { - if (auto* p = absl::get_if(&value_)) { - grpc_slice_unref_internal(*p); - } - } - - // Take the value and leave this empty - // Use Intern/Extern to choose memory management - template - auto Take() -> decltype(this->Take(T())) { - return Take(T()); - } - String(const String&) = delete; String& operator=(const String&) = delete; String(String&& other) noexcept : value_(std::move(other.value_)) { @@ -685,6 +671,27 @@ class HPackParser::String { return *this; } + // Take the value and leave this empty + // Use Intern/Extern to choose memory management + template + auto Take() -> decltype(this->Take(T())) { + return Take(T()); + } + + // Return a reference to the value as a string view + absl::string_view string_view() const { + return Match( + value_, [](const Slice& slice) { return slice.as_string_view(); }, + [](absl::Span span) { + return absl::string_view(reinterpret_cast(span.data()), + span.size()); + }, + [](const std::vector& v) { + return absl::string_view(reinterpret_cast(v.data()), + v.size()); + }); + } + // Parse a non-binary string static absl::optional Parse(Input* input) { auto pfx = input->ParseStringPrefix(); @@ -758,18 +765,7 @@ class HPackParser::String { explicit String(std::vector v) : value_(std::move(v)) {} explicit String(absl::Span v) : value_(v) {} String(grpc_slice_refcount* r, const uint8_t* begin, const uint8_t* end) - : value_(MakeSlice(r, begin, end)) {} - - // Given a refcount and a byte range, make a slice - static grpc_slice MakeSlice(grpc_slice_refcount* r, const uint8_t* begin, - const uint8_t* end) { - grpc_slice out; - out.refcount = r; - r->Ref(); - out.data.refcounted.bytes = const_cast(begin); - out.data.refcounted.length = end - begin; - return out; - } + : value_(Slice::FromRefcountAndBytes(r, begin, end)) {} // Parse some huffman encoded bytes, using output(uint8_t b) to emit each // decoded byte. @@ -931,8 +927,7 @@ class HPackParser::String { GPR_UNREACHABLE_CODE(return out;); } - absl::variant, std::vector> - value_; + absl::variant, std::vector> value_; }; // Parser parses one key/value pair from a byte stream. @@ -1129,14 +1124,16 @@ class HPackParser::Parser { absl::optional ParseLiteralKey() { auto key = String::Parse(input_); if (!key.has_value()) return {}; - auto key_slice = key->Take(); - auto value = - ParseValueString(grpc_is_refcounted_slice_binary_header(key_slice)); + auto value = ParseValueString(absl::EndsWith(key->string_view(), "-bin")); if (GPR_UNLIKELY(!value.has_value())) { - grpc_slice_unref_internal(key_slice); return {}; } - return grpc_metadata_batch::Parse(key_slice, value->Take()); + auto key_string = key->string_view(); + auto value_slice = value->Take(); + const auto transport_size = key_string.size() + value_slice.size() + + hpack_constants::kEntryOverhead; + return grpc_metadata_batch::Parse(key->string_view(), + std::move(value_slice), transport_size); } // Parse an index encoded key and a string encoded value @@ -1244,7 +1241,7 @@ class HPackParser::Parser { const LogInfo log_info_; }; -UnmanagedMemorySlice HPackParser::String::Take(Extern) { +Slice HPackParser::String::Take(Extern) { auto s = Match( value_, [](const grpc_slice& slice) { @@ -1264,10 +1261,10 @@ UnmanagedMemorySlice HPackParser::String::Take(Extern) { v.size()); }); value_ = absl::Span(); - return s; + return Slice(s); } -ManagedMemorySlice HPackParser::String::Take(Intern) { +Slice HPackParser::String::Take(Intern) { auto s = Match( value_, [](const grpc_slice& slice) { @@ -1285,7 +1282,7 @@ ManagedMemorySlice HPackParser::String::Take(Intern) { v.size()); }); value_ = absl::Span(); - return s; + return Slice(s); } /* PUBLIC INTERFACE */ diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.cc b/src/core/ext/transport/cronet/transport/cronet_transport.cc index 4c719e5bf94..7cdeac54f1f 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.cc +++ b/src/core/ext/transport/cronet/transport/cronet_transport.cc @@ -417,7 +417,7 @@ static void convert_cronet_array_to_metadata( value = grpc_slice_intern( grpc_slice_from_static_string(header_array->headers[i].value)); } - mds->Append(header_array->headers[i].key, value); + mds->Append(header_array->headers[i].key, grpc_core::Slice(value)); } } @@ -714,10 +714,11 @@ class CronetMetadataEncoder { CronetMetadataEncoder& operator=(const CronetMetadataEncoder&) = delete; template - void Encode(T, V value) { + void Encode(T, const V& value) { auto value_slice = T::Encode(value); - auto key_slice = grpc_slice_from_static_string(T::key()); - auto mdelem = grpc_mdelem_from_slices(key_slice, value_slice); + auto key_slice = + grpc_core::ExternallyManagedSlice(T::key().data(), T::key().length()); + auto mdelem = grpc_mdelem_from_slices(key_slice, value_slice.TakeCSlice()); Encode(mdelem); GRPC_MDELEM_UNREF(mdelem); } diff --git a/src/core/ext/transport/inproc/inproc_transport.cc b/src/core/ext/transport/inproc/inproc_transport.cc index 9994987770c..e4ba6ca740a 100644 --- a/src/core/ext/transport/inproc/inproc_transport.cc +++ b/src/core/ext/transport/inproc/inproc_transport.cc @@ -316,6 +316,11 @@ class CopySink { dst_->Set(trait, value); } + template + void Encode(T trait, const grpc_core::Slice& value) { + dst_->Set(trait, value.AsOwned()); + } + private: grpc_metadata_batch* dst_; }; diff --git a/src/core/lib/event_engine/memory_allocator.cc b/src/core/lib/event_engine/memory_allocator.cc index ea2a3c261cc..8b6994e76d8 100644 --- a/src/core/lib/event_engine/memory_allocator.cc +++ b/src/core/lib/event_engine/memory_allocator.cc @@ -47,7 +47,7 @@ class SliceRefCount { private: grpc_slice_refcount base_; - grpc_core::RefCount refs_; + std::atomic refs_{1}; std::shared_ptr allocator_; size_t size_; }; diff --git a/src/core/lib/gprpp/table.h b/src/core/lib/gprpp/table.h index b91b0c0da49..01c2140c090 100644 --- a/src/core/lib/gprpp/table.h +++ b/src/core/lib/gprpp/table.h @@ -268,13 +268,25 @@ class Table { TypeIndex* set(Args&&... args) { auto* p = element_ptr(); if (set_present(true)) { - *p = TypeIndex(std::forward(args)...); + TypeIndex replacement(std::forward(args)...); + *p = std::move(replacement); } else { new (p) TypeIndex(std::forward(args)...); } return p; } + template + TypeIndex* set(TypeIndex&& value) { + auto* p = element_ptr(); + if (set_present(true)) { + *p = std::forward>(value); + } else { + new (p) TypeIndex(std::forward>(value)); + } + return p; + } + // Clear the value for type T, leaving it un-set. template void clear() { diff --git a/src/core/lib/slice/slice.cc b/src/core/lib/slice/slice.cc index 44af1f4f748..857db368adc 100644 --- a/src/core/lib/slice/slice.cc +++ b/src/core/lib/slice/slice.cc @@ -69,7 +69,7 @@ class NewSliceRefcount { private: grpc_slice_refcount base_; - RefCount refs_; + std::atomic refs_{1}; void (*user_destroy_)(void*); void* user_data_; }; @@ -130,7 +130,7 @@ class NewWithLenSliceRefcount { private: grpc_slice_refcount base_; - RefCount refs_; + std::atomic refs_{1}; void* user_data_; size_t user_length_; void (*user_destroy_)(void*, size_t); @@ -152,7 +152,7 @@ class MovedStringSliceRefCount { } grpc_slice_refcount base_; - RefCount refs_; + std::atomic refs_{1}; UniquePtr str_; }; @@ -172,7 +172,7 @@ class MovedCppStringSliceRefCount { } grpc_slice_refcount base_; - RefCount refs_; + std::atomic refs_{1}; std::string str_; }; @@ -281,7 +281,7 @@ class MallocRefCount { private: grpc_slice_refcount base_; - grpc_core::RefCount refs_; + std::atomic refs_{1}; }; } // namespace diff --git a/src/core/lib/slice/slice.h b/src/core/lib/slice/slice.h new file mode 100644 index 00000000000..73c4cce8abe --- /dev/null +++ b/src/core/lib/slice/slice.h @@ -0,0 +1,313 @@ +// Copyright 2021 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_H +#define GRPC_CORE_LIB_SLICE_SLICE_H + +#include + +#include "absl/strings/string_view.h" + +#include + +#include "src/core/lib/slice/slice_internal.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 +// mutability. +// +// The team: +// Slice - provides a wrapper around an unknown type of slice. +// Immutable (since we don't know who else might be referencing +// it), and potentially ref counted. +// StaticSlice - provides a wrapper around a static slice. Not refcounted, +// fast to copy. +// MutableSlice - provides a guarantee of unique ownership, meaning the +// underlying data can be mutated safely. + +namespace grpc_core { + +// Forward declarations +class Slice; +class StaticSlice; +class MutableSlice; + +namespace slice_detail { + +// Returns an empty slice. +static constexpr grpc_slice EmptySlice() { return {nullptr, {}}; } + +// BaseSlice holds the grpc_slice object, but does not apply refcounting policy. +// It does export immutable access into the slice, such that this can be shared +// by all storage policies. +class BaseSlice { + public: + BaseSlice(const BaseSlice&) = delete; + BaseSlice& operator=(const BaseSlice&) = delete; + BaseSlice(BaseSlice&& other) = delete; + BaseSlice& operator=(BaseSlice&& other) = delete; + + // Iterator access to the underlying bytes + const uint8_t* begin() const { return GRPC_SLICE_START_PTR(c_slice()); } + const uint8_t* end() const { return GRPC_SLICE_END_PTR(c_slice()); } + const uint8_t* cbegin() const { return GRPC_SLICE_START_PTR(c_slice()); } + const uint8_t* cend() const { return GRPC_SLICE_END_PTR(c_slice()); } + + // Retrieve a borrowed reference to the underlying grpc_slice. + const grpc_slice& c_slice() const { return slice_; } + + // Retrieve the underlying grpc_slice, and replace the one in this object with + // EmptySlice(). + grpc_slice TakeCSlice() { + grpc_slice out = slice_; + slice_ = EmptySlice(); + return out; + } + + // As other things... borrowed references. + absl::string_view as_string_view() const { + return absl::string_view(reinterpret_cast(data()), size()); + } + + // Array access + uint8_t operator[](size_t i) const { + return GRPC_SLICE_START_PTR(c_slice())[i]; + } + + // Access underlying data + const uint8_t* data() const { return GRPC_SLICE_START_PTR(c_slice()); } + + // Size of the slice + size_t size() const { return GRPC_SLICE_LENGTH(c_slice()); } + size_t length() const { return size(); } + bool empty() const { return size() == 0; } + + // For inlined slices - are these two slices equal? + // For non-inlined slices - do these two slices refer to the same block of + // memory? + bool is_equivalent(const BaseSlice& other) const { + return grpc_slice_is_equivalent(slice_, other.slice_); + } + + protected: + BaseSlice() : slice_(EmptySlice()) {} + explicit BaseSlice(const grpc_slice& slice) : slice_(slice) {} + ~BaseSlice() = default; + + void Swap(BaseSlice* other) { std::swap(slice_, other->slice_); } + void SetCSlice(const grpc_slice& slice) { slice_ = slice; } + + uint8_t* mutable_data() { return GRPC_SLICE_START_PTR(slice_); } + + private: + grpc_slice slice_; +}; + +inline bool operator==(const BaseSlice& a, const BaseSlice& b) { + return grpc_slice_eq(a.c_slice(), b.c_slice()) != 0; +} + +inline bool operator!=(const BaseSlice& a, const BaseSlice& b) { + return grpc_slice_eq(a.c_slice(), b.c_slice()) == 0; +} + +inline bool operator==(const BaseSlice& a, absl::string_view b) { + return a.as_string_view() == b; +} + +inline bool operator!=(const BaseSlice& a, absl::string_view b) { + return a.as_string_view() != b; +} + +inline bool operator==(absl::string_view a, const BaseSlice& b) { + return a == b.as_string_view(); +} + +inline bool operator!=(absl::string_view a, const BaseSlice& b) { + return a != b.as_string_view(); +} + +inline bool operator==(const BaseSlice& a, const grpc_slice& b) { + return grpc_slice_eq(a.c_slice(), b) != 0; +} + +inline bool operator!=(const BaseSlice& a, const grpc_slice& b) { + return grpc_slice_eq(a.c_slice(), b) == 0; +} + +inline bool operator==(const grpc_slice& a, const BaseSlice& b) { + return grpc_slice_eq(a, b.c_slice()) != 0; +} + +inline bool operator!=(const grpc_slice& a, const BaseSlice& b) { + return grpc_slice_eq(a, b.c_slice()) == 0; +} + +template +struct CopyConstructors { + static Out FromCopiedString(const char* s) { + return Out(grpc_slice_from_copied_string(s)); + } + static Out FromCopiedString(std::string s) { + return Out(grpc_slice_from_cpp_string(std::move(s))); + } +}; + +} // namespace slice_detail + +class StaticSlice : public slice_detail::BaseSlice { + public: + StaticSlice() = default; + explicit StaticSlice(const grpc_slice& slice) + : slice_detail::BaseSlice(slice) { + GPR_DEBUG_ASSERT( + slice.refcount->GetType() == grpc_slice_refcount::Type::STATIC || + slice.refcount->GetType() == grpc_slice_refcount::Type::NOP); + } + explicit StaticSlice(const StaticMetadataSlice& slice) + : slice_detail::BaseSlice(slice) {} + + static StaticSlice FromStaticString(const char* s) { + return StaticSlice(grpc_slice_from_static_string(s)); + } + + StaticSlice(const StaticSlice& other) + : slice_detail::BaseSlice(other.c_slice()) {} + StaticSlice& operator=(const StaticSlice& other) { + SetCSlice(other.c_slice()); + return *this; + } + StaticSlice(StaticSlice&& other) noexcept + : slice_detail::BaseSlice(other.TakeCSlice()) {} + StaticSlice& operator=(StaticSlice&& other) noexcept { + Swap(&other); + return *this; + } +}; + +class MutableSlice : public slice_detail::BaseSlice, + public slice_detail::CopyConstructors { + public: + MutableSlice() = default; + explicit MutableSlice(const grpc_slice& slice) + : slice_detail::BaseSlice(slice) { + GPR_DEBUG_ASSERT(slice.refcount == nullptr || + slice.refcount->IsRegularUnique()); + } + ~MutableSlice() { grpc_slice_unref_internal(c_slice()); } + + MutableSlice(const MutableSlice&) = delete; + MutableSlice& operator=(const MutableSlice&) = delete; + MutableSlice(MutableSlice&& other) noexcept + : slice_detail::BaseSlice(other.TakeCSlice()) {} + MutableSlice& operator=(MutableSlice&& other) noexcept { + Swap(&other); + return *this; + } + + // Iterator access to the underlying bytes + uint8_t* begin() { return mutable_data(); } + uint8_t* end() { return mutable_data() + size(); } + + // Array access + uint8_t& operator[](size_t i) { return mutable_data()[i]; } +}; + +class Slice : public slice_detail::BaseSlice, + public slice_detail::CopyConstructors { + public: + Slice() = default; + ~Slice() { grpc_slice_unref_internal(c_slice()); } + explicit Slice(const grpc_slice& slice) : slice_detail::BaseSlice(slice) {} + template + explicit Slice(absl::enable_if_t< + std::is_base_of::value, + SliceType>&& other) + : slice_detail::BaseSlice(other.TakeCSlice()) {} + + Slice(const Slice&) = delete; + Slice& operator=(const Slice&) = delete; + Slice(Slice&& other) noexcept : slice_detail::BaseSlice(other.TakeCSlice()) {} + Slice& operator=(Slice&& other) noexcept { + Swap(&other); + return *this; + } + + // A slice might refer to some memory that we keep a refcount to (this is + // owned), or some memory that's inlined into the slice (also owned), or some + // other block of memory that we know will be available for the lifetime of + // some operation in the common case (not owned). In the *less common* case + // that we need to keep that slice text for longer than our API's guarantee us + // access, we need to take a copy and turn this into something that we do own. + + // TakeOwned returns an owned slice regardless of current ownership, and + // leaves the current slice in a valid but externally unpredictable state - in + // doing so it can avoid adding a ref to the underlying slice. + Slice TakeOwned() { + if (c_slice().refcount == nullptr) { + return Slice(c_slice()); + } + if (c_slice().refcount->GetType() == grpc_slice_refcount::Type::NOP) { + return Slice(grpc_slice_copy(c_slice())); + } + return Slice(TakeCSlice()); + } + + // AsOwned returns an owned slice but does not mutate the current slice, + // meaning that it may add a reference to the underlying slice. + Slice AsOwned() const { + if (c_slice().refcount == nullptr) { + return Slice(c_slice()); + } + if (c_slice().refcount->GetType() == grpc_slice_refcount::Type::NOP) { + return Slice(grpc_slice_copy(c_slice())); + } + return Slice(grpc_slice_ref_internal(c_slice())); + } + + // TakeMutable returns a MutableSlice, and leaves the current slice in an + // indeterminate but valid state. + // A mutable slice requires only one reference to the bytes of the slice - + // this can be achieved either with inlined storage or with a single + // reference. + // If the current slice is refcounted and there are more than one references + // to that slice, then the slice is copied in order to achieve a mutable + // version. + MutableSlice TakeMutable() { + if (c_slice().refcount == nullptr) { + return MutableSlice(c_slice()); + } + if (c_slice().refcount->GetType() == grpc_slice_refcount::Type::REGULAR && + c_slice().refcount->IsRegularUnique()) { + return MutableSlice(TakeCSlice()); + } + return MutableSlice(grpc_slice_copy(c_slice())); + } + + Slice Ref() const { return Slice(grpc_slice_ref_internal(c_slice())); } + + static Slice FromRefcountAndBytes(grpc_slice_refcount* r, + const uint8_t* begin, const uint8_t* end) { + grpc_slice out; + out.refcount = r; + r->Ref(); + out.data.refcounted.bytes = const_cast(begin); + out.data.refcounted.length = end - begin; + return Slice(out); + } +}; + +} // namespace grpc_core + +#endif // GRPC_CORE_LIB_SLICE_SLICE_H diff --git a/src/core/lib/slice/slice_intern.cc b/src/core/lib/slice/slice_intern.cc index d3744570da8..0bed91a9e2c 100644 --- a/src/core/lib/slice/slice_intern.cc +++ b/src/core/lib/slice/slice_intern.cc @@ -235,7 +235,7 @@ static InternedSliceRefcount* MatchInternedSliceLocked(uint32_t 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 (s->refcnt.RefIfNonZero()) { + if (grpc_core::IncrementIfNonzero(&s->refcnt)) { return s; } } diff --git a/src/core/lib/slice/slice_refcount.h b/src/core/lib/slice/slice_refcount.h index 1a98f19c853..f16b251c8fd 100644 --- a/src/core/lib/slice/slice_refcount.h +++ b/src/core/lib/slice/slice_refcount.h @@ -17,6 +17,8 @@ #include +#include + #include "src/core/lib/slice/slice_refcount_base.h" #include "src/core/lib/slice/static_slice.h" @@ -28,6 +30,8 @@ namespace grpc_core { 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); @@ -48,7 +52,7 @@ struct InternedSliceRefcount { grpc_slice_refcount base; grpc_slice_refcount sub; const size_t length; - RefCount refcnt; + std::atomic refcnt{1}; const uint32_t hash; InternedSliceRefcount* bucket_next; }; diff --git a/src/core/lib/slice/slice_refcount_base.h b/src/core/lib/slice/slice_refcount_base.h index 62ad152e70b..9c1e34056b4 100644 --- a/src/core/lib/slice/slice_refcount_base.h +++ b/src/core/lib/slice/slice_refcount_base.h @@ -17,9 +17,10 @@ #include -#include +#include -#include "src/core/lib/gprpp/ref_counted.h" +#include +#include // grpc_slice_refcount : A reference count for grpc_slice. // @@ -103,11 +104,10 @@ struct grpc_slice_refcount { // Whether we are the refcount for a static // metadata slice, an interned slice, or any other kind of slice. // - // 2. RefCount* ref - // The pointer to the actual underlying grpc_core::RefCount. Rather than - // performing struct offset computations as in the original implementation to - // get to the refcount, which requires a virtual method, we devirtualize by - // using a nullable pointer to allow a single pair of Ref/Unref methods. + // 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. @@ -117,7 +117,7 @@ struct grpc_slice_refcount { // // 5. grpc_slice_refcount* sub // Argument used for interned slices. - grpc_slice_refcount(grpc_slice_refcount::Type type, grpc_core::RefCount* ref, + grpc_slice_refcount(grpc_slice_refcount::Type type, std::atomic* ref, DestroyerFn destroyer_fn, void* destroyer_arg, grpc_slice_refcount* sub) : ref_(ref), @@ -136,19 +136,27 @@ struct grpc_slice_refcount { uint32_t Hash(const grpc_slice& slice); void Ref() { if (ref_ == nullptr) return; - ref_->RefNonZero(); + ref_->fetch_add(1, std::memory_order_relaxed); } void Unref() { if (ref_ == nullptr) return; - if (ref_->Unref()) { + if (ref_->fetch_sub(1, std::memory_order_acq_rel) == 1) { dest_fn_(destroy_fn_arg_); } } + // Only for type REGULAR, 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_; } private: - grpc_core::RefCount* ref_ = nullptr; + std::atomic* ref_ = nullptr; const Type ref_type_ = Type::REGULAR; grpc_slice_refcount* sub_refcount_ = this; DestroyerFn dest_fn_ = nullptr; diff --git a/src/core/lib/slice/static_slice.cc b/src/core/lib/slice/static_slice.cc index 76bc08ed7bb..9c475aad25c 100644 --- a/src/core/lib/slice/static_slice.cc +++ b/src/core/lib/slice/static_slice.cc @@ -47,14 +47,14 @@ const uint8_t g_static_metadata_bytes[] = { 97, 108, 45, 101, 110, 99, 111, 100, 105, 110, 103, 45, 114, 101, 113, 117, 101, 115, 116, 103, 114, 112, 99, 45, 105, 110, 116, 101, 114, 110, 97, 108, 45, 115, 116, 114, 101, 97, 109, 45, 101, 110, 99, 111, 100, - 105, 110, 103, 45, 114, 101, 113, 117, 101, 115, 116, 117, 115, 101, 114, - 45, 97, 103, 101, 110, 116, 104, 111, 115, 116, 103, 114, 112, 99, 45, - 112, 114, 101, 118, 105, 111, 117, 115, 45, 114, 112, 99, 45, 97, 116, - 116, 101, 109, 112, 116, 115, 103, 114, 112, 99, 45, 114, 101, 116, 114, - 121, 45, 112, 117, 115, 104, 98, 97, 99, 107, 45, 109, 115, 120, 45, - 101, 110, 100, 112, 111, 105, 110, 116, 45, 108, 111, 97, 100, 45, 109, - 101, 116, 114, 105, 99, 115, 45, 98, 105, 110, 103, 114, 112, 99, 45, - 116, 105, 109, 101, 111, 117, 116, 49, 50, 51, 52, 103, 114, 112, 99, + 105, 110, 103, 45, 114, 101, 113, 117, 101, 115, 116, 104, 111, 115, 116, + 103, 114, 112, 99, 45, 112, 114, 101, 118, 105, 111, 117, 115, 45, 114, + 112, 99, 45, 97, 116, 116, 101, 109, 112, 116, 115, 103, 114, 112, 99, + 45, 114, 101, 116, 114, 121, 45, 112, 117, 115, 104, 98, 97, 99, 107, + 45, 109, 115, 120, 45, 101, 110, 100, 112, 111, 105, 110, 116, 45, 108, + 111, 97, 100, 45, 109, 101, 116, 114, 105, 99, 115, 45, 98, 105, 110, + 103, 114, 112, 99, 45, 116, 105, 109, 101, 111, 117, 116, 117, 115, 101, + 114, 45, 97, 103, 101, 110, 116, 49, 50, 51, 52, 103, 114, 112, 99, 46, 119, 97, 105, 116, 95, 102, 111, 114, 95, 114, 101, 97, 100, 121, 103, 114, 112, 99, 46, 116, 105, 109, 101, 111, 117, 116, 103, 114, 112, 99, 46, 109, 97, 120, 95, 114, 101, 113, 117, 101, 115, 116, 95, 109, @@ -231,18 +231,18 @@ const StaticMetadataSlice g_static_metadata_bytes + 199), StaticMetadataSlice(&g_static_metadata_slice_refcounts[17].base, 37, g_static_metadata_bytes + 229), - StaticMetadataSlice(&g_static_metadata_slice_refcounts[18].base, 10, + StaticMetadataSlice(&g_static_metadata_slice_refcounts[18].base, 4, g_static_metadata_bytes + 266), - StaticMetadataSlice(&g_static_metadata_slice_refcounts[19].base, 4, - g_static_metadata_bytes + 276), - StaticMetadataSlice(&g_static_metadata_slice_refcounts[20].base, 26, - g_static_metadata_bytes + 280), - StaticMetadataSlice(&g_static_metadata_slice_refcounts[21].base, 22, - g_static_metadata_bytes + 306), - StaticMetadataSlice(&g_static_metadata_slice_refcounts[22].base, 27, - g_static_metadata_bytes + 328), - StaticMetadataSlice(&g_static_metadata_slice_refcounts[23].base, 12, - g_static_metadata_bytes + 355), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[19].base, 26, + g_static_metadata_bytes + 270), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[20].base, 22, + g_static_metadata_bytes + 296), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[21].base, 27, + g_static_metadata_bytes + 318), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[22].base, 12, + g_static_metadata_bytes + 345), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[23].base, 10, + g_static_metadata_bytes + 357), StaticMetadataSlice(&g_static_metadata_slice_refcounts[24].base, 1, g_static_metadata_bytes + 367), StaticMetadataSlice(&g_static_metadata_slice_refcounts[25].base, 1, diff --git a/src/core/lib/slice/static_slice.h b/src/core/lib/slice/static_slice.h index 893b2efd2d8..a7d73f15669 100644 --- a/src/core/lib/slice/static_slice.h +++ b/src/core/lib/slice/static_slice.h @@ -37,7 +37,7 @@ static_assert( std::is_trivially_destructible::value, - "grpc_core::StaticMetadataSlice must be trivially destructible."); + "StaticMetadataSlice must be trivially destructible."); #define GRPC_STATIC_MDSTR_COUNT 110 /* ":path" */ #define GRPC_MDSTR_PATH (::grpc_core::g_static_metadata_slice_table[0]) @@ -84,21 +84,21 @@ static_assert( /* "grpc-internal-stream-encoding-request" */ #define GRPC_MDSTR_GRPC_INTERNAL_STREAM_ENCODING_REQUEST \ (::grpc_core::g_static_metadata_slice_table[17]) -/* "user-agent" */ -#define GRPC_MDSTR_USER_AGENT (::grpc_core::g_static_metadata_slice_table[18]) /* "host" */ -#define GRPC_MDSTR_HOST (::grpc_core::g_static_metadata_slice_table[19]) +#define GRPC_MDSTR_HOST (::grpc_core::g_static_metadata_slice_table[18]) /* "grpc-previous-rpc-attempts" */ #define GRPC_MDSTR_GRPC_PREVIOUS_RPC_ATTEMPTS \ - (::grpc_core::g_static_metadata_slice_table[20]) + (::grpc_core::g_static_metadata_slice_table[19]) /* "grpc-retry-pushback-ms" */ #define GRPC_MDSTR_GRPC_RETRY_PUSHBACK_MS \ - (::grpc_core::g_static_metadata_slice_table[21]) + (::grpc_core::g_static_metadata_slice_table[20]) /* "x-endpoint-load-metrics-bin" */ #define GRPC_MDSTR_X_ENDPOINT_LOAD_METRICS_BIN \ - (::grpc_core::g_static_metadata_slice_table[22]) + (::grpc_core::g_static_metadata_slice_table[21]) /* "grpc-timeout" */ -#define GRPC_MDSTR_GRPC_TIMEOUT (::grpc_core::g_static_metadata_slice_table[23]) +#define GRPC_MDSTR_GRPC_TIMEOUT (::grpc_core::g_static_metadata_slice_table[22]) +/* "user-agent" */ +#define GRPC_MDSTR_USER_AGENT (::grpc_core::g_static_metadata_slice_table[23]) /* "1" */ #define GRPC_MDSTR_1 (::grpc_core::g_static_metadata_slice_table[24]) /* "2" */ @@ -324,8 +324,9 @@ extern const uint8_t g_static_metadata_bytes[]; ((slice).refcount != NULL && \ (slice).refcount->GetType() == grpc_slice_refcount::Type::STATIC) -#define GRPC_STATIC_METADATA_INDEX(static_slice) \ - (reinterpret_cast((static_slice).refcount) \ +#define GRPC_STATIC_METADATA_INDEX(static_slice) \ + (reinterpret_cast<::grpc_core::StaticSliceRefcount*>( \ + (static_slice).refcount) \ ->index) #endif /* GRPC_CORE_LIB_SLICE_STATIC_SLICE_H */ diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 632f6ed77d0..c9af114548c 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -988,27 +988,50 @@ static grpc_stream_compression_algorithm decode_stream_compression( return algorithm; } +namespace { +class PublishToAppEncoder { + public: + explicit PublishToAppEncoder(grpc_metadata_array* dest) : dest_(dest) {} + + void Encode(grpc_mdelem md) { Append(GRPC_MDKEY(md), GRPC_MDVALUE(md)); } + + void Encode(grpc_core::GrpcTimeoutMetadata, grpc_millis) {} + void Encode(grpc_core::TeMetadata, grpc_core::TeMetadata::ValueType) {} + + template + void Encode(Which, const grpc_core::Slice& value) { + const auto key = Which::key(); + Append(grpc_core::ExternallyManagedSlice(key.data(), key.length()), + value.c_slice()); + } + + private: + void Append(grpc_slice key, grpc_slice value) { + auto* mdusr = &dest_->metadata[dest_->count++]; + mdusr->key = key; + mdusr->value = value; + } + + grpc_metadata_array* const dest_; +}; +} // namespace + static void publish_app_metadata(grpc_call* call, grpc_metadata_batch* b, int is_trailing) { - if (b->non_deadline_count() == 0) return; + if (b->count() == 0) return; if (!call->is_client && is_trailing) return; if (is_trailing && call->buffered_metadata[1] == nullptr) return; GPR_TIMER_SCOPE("publish_app_metadata", 0); grpc_metadata_array* dest; - grpc_metadata* mdusr; dest = call->buffered_metadata[is_trailing]; - if (dest->count + b->non_deadline_count() > dest->capacity) { - dest->capacity = std::max(dest->capacity + b->non_deadline_count(), - dest->capacity * 3 / 2); + if (dest->count + b->count() > dest->capacity) { + dest->capacity = + std::max(dest->capacity + b->count(), dest->capacity * 3 / 2); dest->metadata = static_cast( gpr_realloc(dest->metadata, sizeof(grpc_metadata) * dest->capacity)); } - b->ForEach([&](grpc_mdelem md) { - mdusr = &dest->metadata[dest->count++]; - /* we pass back borrowed slices that are valid whilst the call is valid */ - mdusr->key = GRPC_MDKEY(md); - mdusr->value = GRPC_MDVALUE(md); - }); + PublishToAppEncoder encoder(dest); + b->Encode(&encoder); } static void recv_initial_filter(grpc_call* call, grpc_metadata_batch* b) { diff --git a/src/core/lib/transport/metadata_batch.cc b/src/core/lib/transport/metadata_batch.cc index c651faa72e0..f74c3ecf430 100644 --- a/src/core/lib/transport/metadata_batch.cc +++ b/src/core/lib/transport/metadata_batch.cc @@ -71,6 +71,11 @@ class CopySink { dst_->Set(trait, value); } + template + void Encode(T trait, const grpc_core::Slice& value) { + dst_->Set(trait, std::move(value.AsOwned())); + } + private: grpc_metadata_batch* dst_; }; diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h index 2a90e823ed1..d2b8de6b03c 100644 --- a/src/core/lib/transport/metadata_batch.h +++ b/src/core/lib/transport/metadata_batch.h @@ -34,6 +34,7 @@ #include "src/core/lib/gprpp/chunked_vector.h" #include "src/core/lib/gprpp/table.h" #include "src/core/lib/iomgr/exec_ctx.h" +#include "src/core/lib/slice/slice.h" #include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/metadata.h" #include "src/core/lib/transport/parsed_metadata.h" @@ -82,13 +83,12 @@ namespace grpc_core { struct GrpcTimeoutMetadata { using ValueType = grpc_millis; using MementoType = grpc_millis; - static const char* key() { return "grpc-timeout"; } - static MementoType ParseMemento(const grpc_slice& value) { + static absl::string_view key() { return "grpc-timeout"; } + static MementoType ParseMemento(Slice value) { grpc_millis timeout; - if (GPR_UNLIKELY(!grpc_http2_decode_timeout(value, &timeout))) { + if (GPR_UNLIKELY(!grpc_http2_decode_timeout(value.c_slice(), &timeout))) { timeout = GRPC_MILLIS_INF_FUTURE; } - grpc_slice_unref_internal(value); return timeout; } static ValueType MementoToValue(MementoType timeout) { @@ -97,10 +97,10 @@ struct GrpcTimeoutMetadata { } return ExecCtx::Get()->Now() + timeout; } - static grpc_slice Encode(ValueType x) { + static Slice Encode(ValueType x) { char timeout[GRPC_HTTP2_TIMEOUT_ENCODE_MIN_BUFSIZE]; grpc_http2_encode_timeout(x, timeout); - return grpc_slice_from_copied_string(timeout); + return Slice::FromCopiedString(timeout); } static MementoType DisplayValue(MementoType x) { return x; } }; @@ -115,19 +115,18 @@ struct TeMetadata { kInvalid, }; using MementoType = ValueType; - static const char* key() { return "te"; } - static MementoType ParseMemento(const grpc_slice& value) { + static absl::string_view key() { return "te"; } + static MementoType ParseMemento(Slice value) { auto out = kInvalid; - if (grpc_slice_eq(value, GRPC_MDSTR_TRAILERS)) { + if (value == "trailers") { out = kTrailers; } - grpc_slice_unref_internal(value); return out; } static ValueType MementoToValue(MementoType te) { return te; } - static grpc_slice Encode(ValueType x) { + static StaticSlice Encode(ValueType x) { GPR_ASSERT(x == kTrailers); - return GRPC_MDSTR_TRAILERS; + return StaticSlice(GRPC_MDSTR_TRAILERS); } static const char* DisplayValue(MementoType te) { switch (te) { @@ -139,6 +138,19 @@ struct TeMetadata { } }; +// user-agent metadata trait. +struct UserAgentMetadata { + using ValueType = Slice; + using MementoType = Slice; + static absl::string_view key() { return "user-agent"; } + static MementoType ParseMemento(Slice value) { return value.TakeOwned(); } + static ValueType MementoToValue(MementoType value) { return value; } + static Slice Encode(const ValueType& x) { return x.Ref(); } + static absl::string_view DisplayValue(const MementoType& value) { + return value.as_string_view(); + } +}; + namespace metadata_detail { // Inner implementation of MetadataMap::Parse() @@ -151,25 +163,24 @@ struct ParseHelper; template struct ParseHelper { template - static ParsedMetadata Parse(absl::string_view key, - const grpc_slice& value, + static ParsedMetadata Parse(absl::string_view key, Slice value, + uint32_t transport_size, NotFound not_found) { if (key == Trait::key()) { return ParsedMetadata( - Trait(), Trait::ParseMemento(value), - ParsedMetadata::TransportSize(key.size(), - GRPC_SLICE_LENGTH(value))); + Trait(), Trait::ParseMemento(value.TakeOwned()), transport_size); } - return ParseHelper::Parse(key, value, not_found); + return ParseHelper::Parse(key, std::move(value), + transport_size, not_found); } }; template struct ParseHelper { template - static ParsedMetadata Parse(absl::string_view, const grpc_slice&, - NotFound not_found) { - return not_found(); + static ParsedMetadata Parse(absl::string_view, Slice value, + uint32_t, NotFound not_found) { + return not_found(std::move(value)); } }; @@ -183,14 +194,14 @@ struct AppendHelper; template struct AppendHelper { template - static void Append(Container* container, absl::string_view key, - const grpc_slice& value, NotFound not_found) { + static void Append(Container* container, absl::string_view key, Slice value, + NotFound not_found) { if (key == Trait::key()) { - container->Set(Trait(), - Trait::MementoToValue(Trait::ParseMemento(value))); + container->Set(Trait(), Trait::MementoToValue( + Trait::ParseMemento(value.TakeOwned()))); return; } - AppendHelper::Append(container, key, value, + AppendHelper::Append(container, key, std::move(value), not_found); } }; @@ -198,9 +209,9 @@ struct AppendHelper { template struct AppendHelper { template - static void Append(Container*, absl::string_view, const grpc_slice&, + static void Append(Container*, absl::string_view, Slice value, NotFound not_found) { - not_found(); + not_found(std::move(value)); } }; @@ -228,15 +239,15 @@ struct AppendHelper { // // The type that's stored in compression/decompression tables // using MementoType = ...; // // The string key for this metadata type (for transports that require it) -// static constexpr char* key() { return "grpc-xyz"; } +// static constexpr absl::string_view key() { return "grpc-xyz"; } // // Parse a memento from a slice // // Takes ownership of value -// static MementoType ParseMemento(const grpc_slice& value) { ... } +// static MementoType ParseMemento(Slice value) { ... } // // Convert a memento to a value // static ValueType MementoToValue(MementoType memento) { ... } // // Convert a value to its canonical text wire format (the format that // // ParseMemento will accept!) -// static grpc_slice Encode(ValueType value); +// static Slice Encode(const ValueType& value); // // Convert a value to something that can be passed to StrCat and displayed // // for debugging // static SomeStrCatableType DisplayValue(MementoType value) { ... } @@ -350,19 +361,17 @@ class MetadataMap { // that result. // TODO(ctiller): key should probably be an absl::string_view. // Once we don't care about interning anymore, make that change! - template - static ParsedMetadata Parse(const KeySlice& key, - const ValueSlice& value) { + static ParsedMetadata Parse(absl::string_view key, Slice value, + uint32_t transport_size) { bool parsed = true; auto out = metadata_detail::ParseHelper::Parse( - StringViewFromSlice(key), value, [&] { + key, std::move(value), transport_size, [&](Slice value) { parsed = false; - return ParsedMetadata( - grpc_mdelem_from_slices(key, value)); + return ParsedMetadata(grpc_mdelem_from_slices( + grpc_slice_intern( + grpc_slice_from_static_buffer(key.data(), key.size())), + value.TakeCSlice())); }); - if (parsed) { - grpc_slice_unref_internal(key); - } return out; } @@ -373,14 +382,14 @@ class MetadataMap { } // Append a key/value pair - takes ownership of value - void Append(absl::string_view key, const grpc_slice& value) { + void Append(absl::string_view key, Slice value) { metadata_detail::AppendHelper::Append( - this, key, value, [&] { + this, key, std::move(value), [this, key](Slice value) { GPR_ASSERT(GRPC_ERROR_NONE == Append(grpc_mdelem_from_slices( grpc_slice_intern(grpc_slice_from_static_buffer( key.data(), key.length())), - value))); + value.TakeCSlice()))); }); } @@ -494,10 +503,15 @@ class MetadataMap { struct Value { Value() = default; explicit Value(const typename Which::ValueType& value) : value(value) {} - Value(const Value&) = default; - Value& operator=(const Value&) = default; + explicit Value(typename Which::ValueType&& value) + : value(std::forward(value)) {} + Value(const Value&) = delete; + Value& operator=(const Value&) = delete; Value(Value&&) noexcept = default; - Value& operator=(Value&&) noexcept = default; + Value& operator=(Value&& other) noexcept { + value = std::move(other.value); + return *this; + } GPR_NO_UNIQUE_ADDRESS typename Which::ValueType value; }; // Callable for the ForEach in Encode() -- for each value, call the @@ -896,7 +910,7 @@ bool MetadataMap::ReplaceIfExists(grpc_slice key, grpc_slice value) { using grpc_metadata_batch = grpc_core::MetadataMap; + grpc_core::TeMetadata, grpc_core::UserAgentMetadata>; inline void grpc_metadata_batch_clear(grpc_metadata_batch* batch) { batch->Clear(); diff --git a/src/core/lib/transport/parsed_metadata.h b/src/core/lib/transport/parsed_metadata.h index 305c369ffd0..7c8cdc70cbd 100644 --- a/src/core/lib/transport/parsed_metadata.h +++ b/src/core/lib/transport/parsed_metadata.h @@ -24,6 +24,7 @@ #include "absl/strings/match.h" #include "src/core/lib/iomgr/error.h" +#include "src/core/lib/slice/slice.h" #include "src/core/lib/surface/validate_metadata.h" #include "src/core/lib/transport/metadata.h" @@ -37,7 +38,7 @@ template struct HasSimpleMemento { static constexpr bool value = std::is_trivial::value && - sizeof(typename Which::MementoType) <= sizeof(intptr_t); + sizeof(typename Which::MementoType) <= sizeof(uint64_t); }; } // namespace metadata_detail @@ -60,27 +61,37 @@ class ParsedMetadata { value, uint32_t transport_size) : vtable_(ParsedMetadata::template TrivialTraitVTable()), - value_(static_cast(value)), - transport_size_(transport_size) {} + transport_size_(transport_size) { + value_.trivial = static_cast(value); + } template ParsedMetadata( Which, - absl::enable_if_t::value, - typename Which::MementoType> + absl::enable_if_t< + !metadata_detail::HasSimpleMemento::value && + !std::is_convertible::value, + typename Which::MementoType> value, uint32_t transport_size) : vtable_(ParsedMetadata::template NonTrivialTraitVTable()), - value_( - reinterpret_cast(new typename Which::MementoType(value))), - transport_size_(transport_size) {} + transport_size_(transport_size) { + value_.pointer = new typename Which::MementoType(std::move(value)); + } + template + ParsedMetadata(Which, Slice value, uint32_t transport_size) + : vtable_(ParsedMetadata::template SliceTraitVTable()), + transport_size_(transport_size) { + value_.slice = value.TakeCSlice(); + } // Takes ownership of elem explicit ParsedMetadata(grpc_mdelem elem) : vtable_(grpc_is_binary_header_internal(GRPC_MDKEY(elem)) ? MdelemVtable() : MdelemVtable()), - value_(static_cast(elem.payload)), - transport_size_(GRPC_MDELEM_LENGTH(elem)) {} - ParsedMetadata() : vtable_(EmptyVTable()), value_(0), transport_size_(0) {} + transport_size_(GRPC_MDELEM_LENGTH(elem)) { + value_.mdelem = elem; + } + ParsedMetadata() : vtable_(EmptyVTable()), transport_size_(0) {} ~ParsedMetadata() { vtable_->destroy(value_); } // Non copyable, but movable. @@ -111,8 +122,8 @@ class ParsedMetadata { // HTTP2 defined storage size of this metadatum. uint32_t transport_size() const { return transport_size_; } // Create a new parsed metadata with the same key but a different value. - ParsedMetadata WithNewValue(const grpc_slice& value) const { - return vtable_->with_new_value(value_, value); + ParsedMetadata WithNewValue(Slice value) const { + return vtable_->with_new_value(value_, std::move(value)); } std::string DebugString() const { return vtable_->debug_string(value_); } @@ -123,14 +134,21 @@ class ParsedMetadata { } private: + union Buffer { + uint64_t trivial; + void* pointer; + grpc_slice slice; + grpc_mdelem mdelem; + }; + struct VTable { const bool is_binary_header; - void (*const destroy)(intptr_t value); - grpc_error_handle (*const set)(intptr_t value, + void (*const destroy)(const Buffer& value); + grpc_error_handle (*const set)(const Buffer& value, MetadataContainer* container); - ParsedMetadata (*const with_new_value)(intptr_t value, - const grpc_slice& new_value); - std::string (*debug_string)(intptr_t value); + ParsedMetadata (*const with_new_value)(const Buffer& value, + Slice new_value); + std::string (*debug_string)(const Buffer& value); }; static const VTable* EmptyVTable(); @@ -138,11 +156,13 @@ class ParsedMetadata { static const VTable* TrivialTraitVTable(); template static const VTable* NonTrivialTraitVTable(); + template + static const VTable* SliceTraitVTable(); template static const VTable* MdelemVtable(); const VTable* vtable_; - intptr_t value_; + Buffer value_; uint32_t transport_size_; }; @@ -152,13 +172,13 @@ ParsedMetadata::EmptyVTable() { static const VTable vtable = { false, // destroy - [](intptr_t) {}, + [](const Buffer&) {}, // set - [](intptr_t, MetadataContainer*) { return GRPC_ERROR_NONE; }, + [](const Buffer&, MetadataContainer*) { return GRPC_ERROR_NONE; }, // with_new_value - [](intptr_t, const grpc_slice&) { return ParsedMetadata(); }, + [](const Buffer&, Slice) { return ParsedMetadata(); }, // debug_string - [](intptr_t) -> std::string { return "empty"; }}; + [](const Buffer&) -> std::string { return "empty"; }}; return &vtable; } @@ -169,25 +189,26 @@ ParsedMetadata::TrivialTraitVTable() { static const VTable vtable = { absl::EndsWith(Which::key(), "-bin"), // destroy - [](intptr_t) {}, + [](const Buffer&) {}, // set - [](intptr_t value, MetadataContainer* map) { - map->Set(Which(), Which::MementoToValue( - static_cast(value))); + [](const Buffer& value, MetadataContainer* map) { + map->Set(Which(), + Which::MementoToValue( + static_cast(value.trivial))); return GRPC_ERROR_NONE; }, // with_new_value - [](intptr_t, const grpc_slice& value) { - return ParsedMetadata( - Which(), Which::ParseMemento(value), - TransportSize(strlen(Which::key()), GRPC_SLICE_LENGTH(value))); + [](const Buffer&, Slice value) { + const auto length = value.length(); + return ParsedMetadata(Which(), Which::ParseMemento(std::move(value)), + TransportSize(Which::key().length(), length)); }, // debug_string - [](intptr_t value) { + [](const Buffer& value) { return absl::StrCat( Which::key(), ": ", Which::DisplayValue( - static_cast(value))); + static_cast(value.trivial))); }}; return &vtable; } @@ -199,29 +220,57 @@ ParsedMetadata::NonTrivialTraitVTable() { static const VTable vtable = { absl::EndsWith(Which::key(), "-bin"), // destroy - [](intptr_t value) { - delete reinterpret_cast(value); + [](const Buffer& value) { + delete static_cast(value.pointer); }, // set - [](intptr_t value, MetadataContainer* map) { - auto* p = reinterpret_cast(value); + [](const Buffer& value, MetadataContainer* map) { + auto* p = static_cast(value.pointer); map->Set(Which(), Which::MementoToValue(*p)); return GRPC_ERROR_NONE; }, // with_new_value - [](intptr_t, const grpc_slice& value) { - return ParsedMetadata( - Which(), Which::ParseMemento(value), - TransportSize(strlen(Which::key()), GRPC_SLICE_LENGTH(value))); + [](const Buffer&, Slice value) { + const auto length = value.length(); + return ParsedMetadata(Which(), Which::ParseMemento(std::move(value)), + TransportSize(Which::key().length(), length)); }, // debug_string - [](intptr_t value) { - auto* p = reinterpret_cast(value); + [](const Buffer& value) { + auto* p = static_cast(value.pointer); return absl::StrCat(Which::key(), ": ", Which::DisplayValue(*p)); }}; return &vtable; } +template +template +const typename ParsedMetadata::VTable* +ParsedMetadata::SliceTraitVTable() { + static const VTable vtable = { + absl::EndsWith(Which::key(), "-bin"), + // destroy + [](const Buffer& value) { grpc_slice_unref_internal(value.slice); }, + // set + [](const Buffer& value, MetadataContainer* map) { + map->Set(Which(), Slice(grpc_slice_ref_internal(value.slice))); + return GRPC_ERROR_NONE; + }, + // with_new_value + [](const Buffer&, Slice value) { + const auto length = value.length(); + return ParsedMetadata(Which(), Which::ParseMemento(std::move(value)), + TransportSize(Which::key().length(), length)); + }, + // debug_string + [](const Buffer& value) { + return absl::StrCat( + Which::key(), ": ", + Which::DisplayValue(Slice(grpc_slice_ref_internal(value.slice)))); + }}; + return &vtable; +} + template template const typename ParsedMetadata::VTable* @@ -229,10 +278,10 @@ ParsedMetadata::MdelemVtable() { static const VTable vtable = { kIsBinaryHeader, // destroy - [](intptr_t value) { GRPC_MDELEM_UNREF(grpc_mdelem{uintptr_t(value)}); }, + [](const Buffer& value) { GRPC_MDELEM_UNREF(value.mdelem); }, // set - [](intptr_t value, MetadataContainer* map) { - auto md = GRPC_MDELEM_REF(grpc_mdelem{uintptr_t(value)}); + [](const Buffer& value, MetadataContainer* map) { + auto md = GRPC_MDELEM_REF(value.mdelem); auto err = map->Append(md); // If an error occurs, md is not consumed and we need to. // This is an awful API, but that's why we're replacing it. @@ -242,18 +291,16 @@ ParsedMetadata::MdelemVtable() { return err; }, // with_new_value - [](intptr_t value, const grpc_slice& value_slice) { - grpc_mdelem elem{uintptr_t(value)}; + [](const Buffer& value, Slice value_slice) { return ParsedMetadata(grpc_mdelem_from_slices( static_cast( - grpc_slice_ref_internal(GRPC_MDKEY(elem))), - value_slice)); + grpc_slice_ref_internal(GRPC_MDKEY(value.mdelem))), + value_slice.TakeCSlice())); }, // debug_string - [](intptr_t value) { - grpc_mdelem elem{uintptr_t(value)}; - return absl::StrCat(StringViewFromSlice(GRPC_MDKEY(elem)), ": ", - StringViewFromSlice(GRPC_MDVALUE(elem))); + [](const Buffer& value) { + return absl::StrCat(StringViewFromSlice(GRPC_MDKEY(value.mdelem)), ": ", + StringViewFromSlice(GRPC_MDVALUE(value.mdelem))); }}; return &vtable; } diff --git a/src/core/lib/transport/static_metadata.cc b/src/core/lib/transport/static_metadata.cc index c6c745d9d2f..b9a4ba5aaab 100644 --- a/src/core/lib/transport/static_metadata.cc +++ b/src/core/lib/transport/static_metadata.cc @@ -255,8 +255,8 @@ StaticMetadata g_static_mdelem_table[GRPC_STATIC_MDELEM_COUNT] = { g_static_metadata_bytes + 371), 36), StaticMetadata( - StaticMetadataSlice(&g_static_metadata_slice_refcounts[19].base, 4, - g_static_metadata_bytes + 276), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[18].base, 4, + g_static_metadata_bytes + 266), StaticMetadataSlice(&g_static_metadata_slice_refcounts[28].base, 0, g_static_metadata_bytes + 371), 37), @@ -375,8 +375,8 @@ StaticMetadata g_static_mdelem_table[GRPC_STATIC_MDELEM_COUNT] = { g_static_metadata_bytes + 371), 56), StaticMetadata( - StaticMetadataSlice(&g_static_metadata_slice_refcounts[18].base, 10, - g_static_metadata_bytes + 266), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[23].base, 10, + g_static_metadata_bytes + 357), StaticMetadataSlice(&g_static_metadata_slice_refcounts[28].base, 0, g_static_metadata_bytes + 371), 57), @@ -979,11 +979,11 @@ uintptr_t grpc_static_mdelem_user_data[GRPC_STATIC_MDELEM_COUNT] = { static const int8_t elems_r[] = { 18, 11, -8, 0, 3, -75, -51, 0, 7, -4, 0, 0, 0, 12, -1, -2, - 0, 0, 7, 6, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 7, 0, 0, 0, 0, -2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -50, 0, -55, -75, -76, -77, 0, - 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 28, 19, 18, 17, 16, 17, - 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, + 30, 29, 28, 27, 26, 25, 48, 22, 21, 20, 19, 19, 18, 17, 16, 17, + 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 5, 4, 3, 2, 1, 0, -1, 0, 0, 0, 0, 0, 0, 0, -11, 0}; static uint32_t elems_phash(uint32_t i) { i -= 46; @@ -1000,18 +1000,18 @@ static uint32_t elems_phash(uint32_t i) { static const uint16_t elem_keys[] = { 270, 271, 272, 273, 274, 275, 276, 1029, 1030, 1568, 1678, 154, 155, 488, 489, 760, 919, 920, 46, 47, 1458, 1580, - 1690, 684, 685, 2008, 2118, 6628, 6738, 6848, 6958, 7068, 7178, - 7288, 7398, 7508, 7618, 7728, 7838, 7948, 1708, 8168, 8278, 8388, - 8498, 6518, 6298, 8608, 8058, 8718, 8828, 8938, 9048, 9158, 9268, - 9378, 9488, 9598, 9708, 9818, 9928, 10038, 10148, 10258, 10368, 10478, + 1690, 684, 685, 2008, 2558, 6628, 6738, 6848, 6958, 7068, 7178, + 7288, 7398, 7508, 7728, 7838, 7948, 8058, 1708, 8168, 8278, 8388, + 8498, 6518, 6298, 8608, 8718, 8828, 8938, 9048, 9158, 9268, 9378, + 9488, 9598, 9708, 9818, 9928, 7618, 10038, 10148, 10258, 10368, 10478, 10588, 10698, 543, 1091, 10808, 214, 10918, 11578, 1096, 1097, 1098, 1099, 981, 0, 0, 0, 1641, 1751, 0, 0, 0, 0, 358, 1757, 0, 0, 0, 0, 1532}; static const uint8_t elem_idxs[] = { 7, 8, 9, 10, 11, 12, 13, 75, 77, 25, 70, 1, 2, 5, 6, 61, - 66, 65, 3, 4, 30, 72, 82, 62, 63, 57, 37, 17, 18, 19, 20, 21, - 22, 23, 24, 26, 27, 28, 29, 31, 15, 33, 34, 35, 36, 16, 14, 38, - 32, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, + 66, 65, 3, 4, 30, 72, 82, 62, 63, 37, 57, 17, 18, 19, 20, 21, + 22, 23, 24, 26, 28, 29, 31, 32, 15, 33, 34, 35, 36, 16, 14, 38, + 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 27, 51, 52, 53, 54, 55, 56, 58, 68, 74, 59, 69, 60, 73, 76, 78, 79, 80, 64, 255, 255, 255, 71, 81, 255, 255, 255, 255, 0, 83, 255, 255, 255, 255, 67}; diff --git a/src/core/lib/transport/static_metadata.h b/src/core/lib/transport/static_metadata.h index 8b5783e76df..272d16a606c 100644 --- a/src/core/lib/transport/static_metadata.h +++ b/src/core/lib/transport/static_metadata.h @@ -277,7 +277,6 @@ typedef enum { GRPC_BATCH_ACCEPT_ENCODING, GRPC_BATCH_GRPC_INTERNAL_ENCODING_REQUEST, GRPC_BATCH_GRPC_INTERNAL_STREAM_ENCODING_REQUEST, - GRPC_BATCH_USER_AGENT, GRPC_BATCH_HOST, GRPC_BATCH_GRPC_PREVIOUS_RPC_ATTEMPTS, GRPC_BATCH_GRPC_RETRY_PUSHBACK_MS, @@ -306,7 +305,6 @@ typedef union { struct grpc_linked_mdelem* accept_encoding; struct grpc_linked_mdelem* grpc_internal_encoding_request; struct grpc_linked_mdelem* grpc_internal_stream_encoding_request; - struct grpc_linked_mdelem* user_agent; struct grpc_linked_mdelem* host; struct grpc_linked_mdelem* grpc_previous_rpc_attempts; struct grpc_linked_mdelem* grpc_retry_pushback_ms; diff --git a/src/core/lib/transport/transport.cc b/src/core/lib/transport/transport.cc index 5555b62eb1e..c4f4fb1675f 100644 --- a/src/core/lib/transport/transport.cc +++ b/src/core/lib/transport/transport.cc @@ -61,25 +61,6 @@ void slice_stream_destroy(void* arg) { grpc_stream_destroy(static_cast(arg)); } -#define STREAM_REF_FROM_SLICE_REF(p) \ - ((grpc_stream_refcount*)(((uint8_t*)(p)) - \ - offsetof(grpc_stream_refcount, slice_refcount))) - -grpc_slice grpc_slice_from_stream_owned_buffer(grpc_stream_refcount* refcount, - void* buffer, size_t length) { -#ifndef NDEBUG - grpc_stream_ref(STREAM_REF_FROM_SLICE_REF(&refcount->slice_refcount), - "slice"); -#else - grpc_stream_ref(STREAM_REF_FROM_SLICE_REF(&refcount->slice_refcount)); -#endif - grpc_slice res; - res.refcount = &refcount->slice_refcount; - res.data.refcounted.bytes = static_cast(buffer); - res.data.refcounted.length = length; - return res; -} - #ifndef NDEBUG void grpc_stream_ref_init(grpc_stream_refcount* refcount, int /*initial_refs*/, grpc_iomgr_cb_func cb, void* cb_arg, @@ -94,9 +75,6 @@ void grpc_stream_ref_init(grpc_stream_refcount* refcount, int /*initial_refs*/, new (&refcount->refs) grpc_core::RefCount( 1, GRPC_TRACE_FLAG_ENABLED(grpc_trace_stream_refcount) ? "stream_refcount" : nullptr); - new (&refcount->slice_refcount) grpc_slice_refcount( - grpc_slice_refcount::Type::REGULAR, &refcount->refs, slice_stream_destroy, - refcount, &refcount->slice_refcount); } static void move64(uint64_t* from, uint64_t* to) { diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h index 5d223ee5f78..72440cda5ca 100644 --- a/src/core/lib/transport/transport.h +++ b/src/core/lib/transport/transport.h @@ -59,7 +59,6 @@ typedef struct grpc_stream_refcount { #ifndef NDEBUG const char* object_type; #endif - grpc_slice_refcount slice_refcount; } grpc_stream_refcount; #ifndef NDEBUG diff --git a/src/core/lib/transport/transport_op_string.cc b/src/core/lib/transport/transport_op_string.cc index 230d43e4e49..16c01203882 100644 --- a/src/core/lib/transport/transport_op_string.cc +++ b/src/core/lib/transport/transport_op_string.cc @@ -62,7 +62,7 @@ class MetadataListEncoder { } template - void Encode(Which, typename Which::ValueType value) { + void Encode(Which, const typename Which::ValueType& value) { MaybeAddComma(); out_->push_back( absl::StrCat(Which::key(), "=", Which::DisplayValue(value))); diff --git a/test/core/end2end/fuzzers/hpack.dictionary b/test/core/end2end/fuzzers/hpack.dictionary index 0bad6e63bf8..e9f60d1d996 100644 --- a/test/core/end2end/fuzzers/hpack.dictionary +++ b/test/core/end2end/fuzzers/hpack.dictionary @@ -17,12 +17,12 @@ "\x0Faccept-encoding" "\x1Egrpc-internal-encoding-request" "%grpc-internal-stream-encoding-request" -"\x0Auser-agent" "\x04host" "\x1Agrpc-previous-rpc-attempts" "\x16grpc-retry-pushback-ms" "\x1Bx-endpoint-load-metrics-bin" "\x0Cgrpc-timeout" +"\x0Auser-agent" "\x011" "\x012" "\x013" diff --git a/test/core/slice/BUILD b/test/core/slice/BUILD index 854418a2bf1..192443a4a29 100644 --- a/test/core/slice/BUILD +++ b/test/core/slice/BUILD @@ -82,11 +82,15 @@ grpc_cc_test( grpc_cc_test( name = "slice_test", srcs = ["slice_test.cc"], + external_deps = [ + "gtest", + ], language = "C++", uses_polling = False, deps = [ "//:gpr", "//:slice", + "//test/core/util:build", "//test/core/util:grpc_suppressions", ], ) diff --git a/test/core/slice/slice_test.cc b/test/core/slice/slice_test.cc index 3da56f16eb8..1f5ea505054 100644 --- a/test/core/slice/slice_test.cc +++ b/test/core/slice/slice_test.cc @@ -18,9 +18,15 @@ #include +#include "src/core/lib/slice/slice.h" + #include #include +#include + +#include + #include #include #include @@ -28,27 +34,24 @@ #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/slice/slice_internal.h" +#include "test/core/util/build.h" -#define LOG_TEST_NAME(x) gpr_log(GPR_INFO, "%s", x); - -static void test_slice_malloc_returns_something_sensible(void) { +TEST(GrpcSliceTest, MallocReturnsSomethingSensible) { /* Calls grpc_slice_create for various lengths and verifies the internals for consistency. */ size_t length; size_t i; grpc_slice slice; - LOG_TEST_NAME("test_slice_malloc_returns_something_sensible"); - for (length = 0; length <= 1024; length++) { slice = grpc_slice_malloc(length); /* If there is a length, slice.data must be non-NULL. If length is zero we don't care. */ if (length > GRPC_SLICE_INLINED_SIZE) { - GPR_ASSERT(slice.data.refcounted.bytes); + EXPECT_NE(slice.data.refcounted.bytes, nullptr); } /* Returned slice length must be what was requested. */ - GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == length); + EXPECT_EQ(GRPC_SLICE_LENGTH(slice), length); /* We must be able to write to every byte of the data */ for (i = 0; i < length; i++) { GRPC_SLICE_START_PTR(slice)[i] = static_cast(i); @@ -60,20 +63,20 @@ static void test_slice_malloc_returns_something_sensible(void) { static void do_nothing(void* /*ignored*/) {} -static void test_slice_new_returns_something_sensible(void) { +TEST(GrpcSliceTest, SliceNewReturnsSomethingSensible) { uint8_t x; grpc_slice slice = grpc_slice_new(&x, 1, do_nothing); - GPR_ASSERT(slice.refcount); - GPR_ASSERT(slice.data.refcounted.bytes == &x); - GPR_ASSERT(slice.data.refcounted.length == 1); + EXPECT_NE(slice.refcount, nullptr); + EXPECT_EQ(slice.data.refcounted.bytes, &x); + EXPECT_EQ(slice.data.refcounted.length, 1); grpc_slice_unref_internal(slice); } /* destroy function that sets a mark to indicate it was called. */ static void set_mark(void* p) { *(static_cast(p)) = 1; } -static void test_slice_new_with_user_data(void) { +TEST(GrpcSliceTest, SliceNewWithUserData) { int marker = 0; uint8_t buf[2]; grpc_slice slice; @@ -81,33 +84,34 @@ static void test_slice_new_with_user_data(void) { buf[0] = 0; buf[1] = 1; slice = grpc_slice_new_with_user_data(buf, 2, set_mark, &marker); - GPR_ASSERT(marker == 0); - GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == 2); - GPR_ASSERT(GRPC_SLICE_START_PTR(slice)[0] == 0); - GPR_ASSERT(GRPC_SLICE_START_PTR(slice)[1] == 1); + EXPECT_EQ(marker, 0); + EXPECT_EQ(GRPC_SLICE_LENGTH(slice), 2); + EXPECT_EQ(GRPC_SLICE_START_PTR(slice)[0], 0); + EXPECT_EQ(GRPC_SLICE_START_PTR(slice)[1], 1); /* unref should cause destroy function to run. */ grpc_slice_unref_internal(slice); - GPR_ASSERT(marker == 1); + EXPECT_EQ(marker, 1); } static int do_nothing_with_len_1_calls = 0; static void do_nothing_with_len_1(void* /*ignored*/, size_t len) { - GPR_ASSERT(len == 1); + EXPECT_EQ(len, 1); do_nothing_with_len_1_calls++; } -static void test_slice_new_with_len_returns_something_sensible(void) { +TEST(GrpcSliceTest, SliceNewWithLenReturnsSomethingSensible) { uint8_t x; int num_refs = 5; /* To test adding/removing an arbitrary number of refs */ int i; grpc_slice slice = grpc_slice_new_with_len(&x, 1, do_nothing_with_len_1); - GPR_ASSERT(slice.refcount); /* ref count is initialized to 1 at this point */ - GPR_ASSERT(slice.data.refcounted.bytes == &x); - GPR_ASSERT(slice.data.refcounted.length == 1); - GPR_ASSERT(do_nothing_with_len_1_calls == 0); + EXPECT_NE(slice.refcount, + nullptr); /* ref count is initialized to 1 at this point */ + EXPECT_EQ(slice.data.refcounted.bytes, &x); + EXPECT_EQ(slice.data.refcounted.length, 1); + EXPECT_EQ(do_nothing_with_len_1_calls, 0); /* Add an arbitrary number of refs to the slice and remoe the refs. This is to make sure that that the destroy callback (i.e do_nothing_with_len_1()) is @@ -118,21 +122,22 @@ static void test_slice_new_with_len_returns_something_sensible(void) { for (i = 0; i < num_refs; i++) { grpc_slice_unref_internal(slice); } - GPR_ASSERT(do_nothing_with_len_1_calls == 0); /* Shouldn't be called yet */ + EXPECT_EQ(do_nothing_with_len_1_calls, 0); /* Shouldn't be called yet */ /* last unref */ grpc_slice_unref_internal(slice); - GPR_ASSERT(do_nothing_with_len_1_calls == 1); + EXPECT_EQ(do_nothing_with_len_1_calls, 1); } -static void test_slice_sub_works(unsigned length) { +class GrpcSliceSizedTest : public ::testing::TestWithParam {}; + +TEST_P(GrpcSliceSizedTest, SliceSubWorks) { + const auto length = GetParam(); + grpc_slice slice; grpc_slice sub; unsigned i, j, k; - LOG_TEST_NAME("test_slice_sub_works"); - gpr_log(GPR_INFO, "length=%d", length); - /* Create a slice in which each byte is equal to the distance from it to the beginning of the slice. */ slice = grpc_slice_malloc(length); @@ -145,9 +150,9 @@ static void test_slice_sub_works(unsigned length) { for (i = 0; i < length; i++) { for (j = i; j < length; j++) { sub = grpc_slice_sub(slice, i, j); - GPR_ASSERT(GRPC_SLICE_LENGTH(sub) == j - i); + EXPECT_EQ(GRPC_SLICE_LENGTH(sub), j - i); for (k = 0; k < j - i; k++) { - GPR_ASSERT(GRPC_SLICE_START_PTR(sub)[k] == (uint8_t)(i + k)); + EXPECT_EQ(GRPC_SLICE_START_PTR(sub)[k], (uint8_t)(i + k)); } grpc_slice_unref_internal(sub); } @@ -157,20 +162,21 @@ static void test_slice_sub_works(unsigned length) { static void check_head_tail(grpc_slice slice, grpc_slice head, grpc_slice tail) { - GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == - GRPC_SLICE_LENGTH(head) + GRPC_SLICE_LENGTH(tail)); - GPR_ASSERT(0 == memcmp(GRPC_SLICE_START_PTR(slice), - GRPC_SLICE_START_PTR(head), GRPC_SLICE_LENGTH(head))); - GPR_ASSERT(0 == memcmp(GRPC_SLICE_START_PTR(slice) + GRPC_SLICE_LENGTH(head), - GRPC_SLICE_START_PTR(tail), GRPC_SLICE_LENGTH(tail))); + EXPECT_EQ(GRPC_SLICE_LENGTH(slice), + GRPC_SLICE_LENGTH(head) + GRPC_SLICE_LENGTH(tail)); + EXPECT_EQ(0, memcmp(GRPC_SLICE_START_PTR(slice), GRPC_SLICE_START_PTR(head), + GRPC_SLICE_LENGTH(head))); + EXPECT_EQ(0, memcmp(GRPC_SLICE_START_PTR(slice) + GRPC_SLICE_LENGTH(head), + GRPC_SLICE_START_PTR(tail), GRPC_SLICE_LENGTH(tail))); } -static void test_slice_split_head_works(size_t length) { +TEST_P(GrpcSliceSizedTest, SliceSplitHeadWorks) { + const auto length = GetParam(); + grpc_slice slice; grpc_slice head, tail; size_t i; - LOG_TEST_NAME("test_slice_split_head_works"); gpr_log(GPR_INFO, "length=%" PRIuPTR, length); /* Create a slice in which each byte is equal to the distance from it to the @@ -193,12 +199,13 @@ static void test_slice_split_head_works(size_t length) { grpc_slice_unref_internal(slice); } -static void test_slice_split_tail_works(size_t length) { +TEST_P(GrpcSliceSizedTest, SliceSplitTailWorks) { + const auto length = GetParam(); + grpc_slice slice; grpc_slice head, tail; size_t i; - LOG_TEST_NAME("test_slice_split_tail_works"); gpr_log(GPR_INFO, "length=%" PRIuPTR, length); /* Create a slice in which each byte is equal to the distance from it to the @@ -221,30 +228,37 @@ static void test_slice_split_tail_works(size_t length) { grpc_slice_unref_internal(slice); } -static void test_slice_from_copied_string_works(void) { +INSTANTIATE_TEST_SUITE_P(GrpcSliceSizedTest, GrpcSliceSizedTest, + ::testing::ValuesIn([] { + std::vector out; + for (size_t i = 0; i < 128; i++) { + out.push_back(i); + } + return out; + }()), + [](const testing::TestParamInfo& info) { + return std::to_string(info.param); + }); + +TEST(GrpcSliceTest, SliceFromCopiedString) { static const char* text = "HELLO WORLD!"; grpc_slice slice; - LOG_TEST_NAME("test_slice_from_copied_string_works"); - slice = grpc_slice_from_copied_string(text); - GPR_ASSERT(strlen(text) == GRPC_SLICE_LENGTH(slice)); - GPR_ASSERT( - 0 == memcmp(text, GRPC_SLICE_START_PTR(slice), GRPC_SLICE_LENGTH(slice))); + EXPECT_EQ(strlen(text), GRPC_SLICE_LENGTH(slice)); + EXPECT_EQ( + 0, memcmp(text, GRPC_SLICE_START_PTR(slice), GRPC_SLICE_LENGTH(slice))); grpc_slice_unref_internal(slice); } -static void test_moved_string_slice(void) { - LOG_TEST_NAME("test_moved_string_slice"); - +TEST(GrpcSliceTest, MovedStringSlice) { // Small string should be inlined. constexpr char kSmallStr[] = "hello12345"; char* small_ptr = strdup(kSmallStr); grpc_slice small = grpc_slice_from_moved_string(grpc_core::UniquePtr(small_ptr)); - GPR_ASSERT(GRPC_SLICE_LENGTH(small) == strlen(kSmallStr)); - GPR_ASSERT(GRPC_SLICE_START_PTR(small) != - reinterpret_cast(small_ptr)); + EXPECT_EQ(GRPC_SLICE_LENGTH(small), strlen(kSmallStr)); + EXPECT_NE(GRPC_SLICE_START_PTR(small), reinterpret_cast(small_ptr)); grpc_slice_unref_internal(small); // Large string should be move the reference. @@ -252,9 +266,8 @@ static void test_moved_string_slice(void) { char* large_ptr = strdup(kSLargeStr); grpc_slice large = grpc_slice_from_moved_string(grpc_core::UniquePtr(large_ptr)); - GPR_ASSERT(GRPC_SLICE_LENGTH(large) == strlen(kSLargeStr)); - GPR_ASSERT(GRPC_SLICE_START_PTR(large) == - reinterpret_cast(large_ptr)); + EXPECT_EQ(GRPC_SLICE_LENGTH(large), strlen(kSLargeStr)); + EXPECT_EQ(GRPC_SLICE_START_PTR(large), reinterpret_cast(large_ptr)); grpc_slice_unref_internal(large); // Moved buffer must respect the provided length not the actual length of the @@ -262,32 +275,167 @@ static void test_moved_string_slice(void) { large_ptr = strdup(kSLargeStr); small = grpc_slice_from_moved_buffer(grpc_core::UniquePtr(large_ptr), strlen(kSmallStr)); - GPR_ASSERT(GRPC_SLICE_LENGTH(small) == strlen(kSmallStr)); - GPR_ASSERT(GRPC_SLICE_START_PTR(small) != - reinterpret_cast(large_ptr)); + EXPECT_EQ(GRPC_SLICE_LENGTH(small), strlen(kSmallStr)); + EXPECT_NE(GRPC_SLICE_START_PTR(small), reinterpret_cast(large_ptr)); grpc_slice_unref_internal(small); } -void test_string_view_from_slice() { +TEST(GrpcSliceTest, StringViewFromSlice) { constexpr char kStr[] = "foo"; absl::string_view sv( grpc_core::StringViewFromSlice(grpc_slice_from_static_string(kStr))); - GPR_ASSERT(std::string(sv) == kStr); + EXPECT_EQ(sv, kStr); } -int main(int, char**) { - unsigned length; - test_slice_malloc_returns_something_sensible(); - test_slice_new_returns_something_sensible(); - test_slice_new_with_user_data(); - test_slice_new_with_len_returns_something_sensible(); - for (length = 0; length < 128; length++) { - test_slice_sub_works(length); - test_slice_split_head_works(length); - test_slice_split_tail_works(length); +namespace grpc_core { +namespace { + +TEST(SliceTest, FromSmallCopiedString) { + Slice slice = Slice::FromCopiedString("hello"); + EXPECT_EQ(slice[0], 'h'); + EXPECT_EQ(slice[1], 'e'); + EXPECT_EQ(slice[2], 'l'); + EXPECT_EQ(slice[3], 'l'); + EXPECT_EQ(slice[4], 'o'); + EXPECT_EQ(slice.size(), 5); + EXPECT_EQ(slice.length(), 5); + EXPECT_EQ(slice.as_string_view(), "hello"); + EXPECT_EQ(0, memcmp(slice.data(), "hello", 5)); +} + +class SliceSizedTest : public ::testing::TestWithParam {}; + +std::string RandomString(size_t length) { + std::string str; + std::random_device r; + for (size_t i = 0; i < length; ++i) { + str.push_back(char(r())); + } + return str; +} + +TEST_P(SliceSizedTest, FromCopiedString) { + const std::string str = RandomString(GetParam()); + Slice slice = Slice::FromCopiedString(str); + + EXPECT_EQ(slice.size(), str.size()); + EXPECT_EQ(slice.length(), str.size()); + EXPECT_EQ(slice.as_string_view(), str); + EXPECT_EQ(0, memcmp(slice.data(), str.data(), str.size())); + for (size_t i = 0; i < str.size(); ++i) { + EXPECT_EQ(slice[i], uint8_t(str[i])); } - test_slice_from_copied_string_works(); - test_moved_string_slice(); - test_string_view_from_slice(); - return 0; + + EXPECT_TRUE(slice.is_equivalent(slice.Ref())); + EXPECT_TRUE(slice.is_equivalent(slice.AsOwned())); + EXPECT_TRUE(slice.is_equivalent(slice.Ref().TakeOwned())); +} + +INSTANTIATE_TEST_SUITE_P(SliceSizedTest, SliceSizedTest, + ::testing::ValuesIn([] { + std::vector out; + size_t i = 1; + size_t j = 1; + while (i < 1024 * 1024) { + out.push_back(j); + size_t n = i + j; + i = j; + j = n; + } + return out; + }()), + [](const testing::TestParamInfo& info) { + return std::to_string(info.param); + }); + +size_t SumSlice(const Slice& slice) { + size_t x = 0; + for (size_t i = 0; i < slice.size(); ++i) { + x += slice[i]; + } + return x; +} + +TEST(SliceTest, ExternalAsOwned) { + auto external_string = absl::make_unique(RandomString(1024)); + Slice slice(ExternallyManagedSlice(external_string->data(), + external_string->length())); + const auto initial_sum = SumSlice(slice); + Slice owned = slice.AsOwned(); + EXPECT_EQ(initial_sum, SumSlice(owned)); + external_string.reset(); + // In ASAN (where we can be sure that it'll crash), go ahead and read the + // bytes we just deleted. + if (BuiltUnderAsan()) { + ASSERT_DEATH({ SumSlice(slice); }, ""); + } + EXPECT_EQ(initial_sum, SumSlice(owned)); +} + +TEST(SliceTest, ExternalTakeOwned) { + std::unique_ptr external_string( + new std::string(RandomString(1024))); + SumSlice(Slice(ExternallyManagedSlice(external_string->data(), + external_string->length())) + .TakeOwned()); +} + +TEST(SliceTest, StaticSlice) { + static const char* hello = "hello"; + StaticSlice slice = StaticSlice::FromStaticString(hello); + EXPECT_EQ(slice[0], 'h'); + EXPECT_EQ(slice[1], 'e'); + EXPECT_EQ(slice[2], 'l'); + EXPECT_EQ(slice[3], 'l'); + EXPECT_EQ(slice[4], 'o'); + EXPECT_EQ(slice.size(), 5); + EXPECT_EQ(slice.length(), 5); + EXPECT_EQ(slice.as_string_view(), "hello"); + EXPECT_EQ(0, memcmp(slice.data(), "hello", 5)); + EXPECT_EQ(reinterpret_cast(hello), slice.data()); +} + +TEST(SliceTest, SliceEquality) { + auto a = Slice::FromCopiedString( + "hello world 123456789123456789123456789123456789123456789"); + auto b = Slice::FromCopiedString( + "hello world 123456789123456789123456789123456789123456789"); + auto c = Slice::FromCopiedString( + "this is not the same as the other two strings!!!!!!!!!!!!"); + EXPECT_FALSE(a.is_equivalent(b)); + EXPECT_FALSE(b.is_equivalent(a)); + EXPECT_EQ(a, b); + EXPECT_NE(a, c); + EXPECT_NE(b, c); + EXPECT_EQ(a, "hello world 123456789123456789123456789123456789123456789"); + EXPECT_NE(a, "pfoooey"); + EXPECT_EQ(c, "this is not the same as the other two strings!!!!!!!!!!!!"); + EXPECT_EQ("hello world 123456789123456789123456789123456789123456789", a); + EXPECT_NE("pfoooey", a); + EXPECT_EQ("this is not the same as the other two strings!!!!!!!!!!!!", c); +} + +TEST(SliceTest, LetsGetMutable) { + auto slice = MutableSlice::FromCopiedString("hello"); + EXPECT_EQ(slice[0], 'h'); + EXPECT_EQ(slice[1], 'e'); + EXPECT_EQ(slice[2], 'l'); + EXPECT_EQ(slice[3], 'l'); + EXPECT_EQ(slice[4], 'o'); + EXPECT_EQ(slice.size(), 5); + EXPECT_EQ(slice.length(), 5); + EXPECT_EQ(slice.as_string_view(), "hello"); + EXPECT_EQ(0, memcmp(slice.data(), "hello", 5)); + slice[2] = 'm'; + EXPECT_EQ(slice.as_string_view(), "hemlo"); + for (auto& c : slice) c++; + EXPECT_EQ(slice.as_string_view(), "ifnmp"); +} + +} // namespace +} // namespace grpc_core + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); } diff --git a/test/core/transport/BUILD b/test/core/transport/BUILD index ce79187e2bc..6d54c7c28be 100644 --- a/test/core/transport/BUILD +++ b/test/core/transport/BUILD @@ -156,18 +156,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "stream_owned_slice_test", - srcs = ["stream_owned_slice_test.cc"], - language = "C++", - uses_polling = False, - deps = [ - "//:gpr", - "//:grpc", - "//test/core/util:grpc_test_util", - ], -) - grpc_cc_test( name = "timeout_encoding_test", srcs = ["timeout_encoding_test.cc"], diff --git a/test/core/transport/chttp2/hpack_parser_test.cc b/test/core/transport/chttp2/hpack_parser_test.cc index 7327c6db2af..30b89df24ef 100644 --- a/test/core/transport/chttp2/hpack_parser_test.cc +++ b/test/core/transport/chttp2/hpack_parser_test.cc @@ -117,7 +117,7 @@ class ParseTest : public ::testing::TestWithParam { } template - void Encode(T, V) { + void Encode(T, const V&) { abort(); // not implemented } diff --git a/test/core/transport/parsed_metadata_test.cc b/test/core/transport/parsed_metadata_test.cc index d0a8c34d5a4..de1195fa285 100644 --- a/test/core/transport/parsed_metadata_test.cc +++ b/test/core/transport/parsed_metadata_test.cc @@ -27,27 +27,25 @@ namespace testing { struct CharTrait { using MementoType = char; - static const char* key() { return "key"; } + static absl::string_view key() { return "key"; } static char test_memento() { return 'a'; } static char test_value() { return 'a'; } static size_t test_memento_transport_size() { return 34; } static char MementoToValue(char memento) { return memento; } - static char ParseMemento(const grpc_slice& slice) { - return *GRPC_SLICE_START_PTR(slice); - } + static char ParseMemento(Slice slice) { return slice[0]; } static std::string DisplayValue(char value) { return std::string(1, value); } }; struct Int32Trait { using MementoType = int32_t; - static const char* key() { return "key2"; } + static absl::string_view key() { return "key2"; } static int32_t test_memento() { return -1; } static int32_t test_value() { return -1; } static size_t test_memento_transport_size() { return 478; } static int32_t MementoToValue(int32_t memento) { return memento; } - static int32_t ParseMemento(const grpc_slice& slice) { + static int32_t ParseMemento(Slice slice) { int32_t out; - GPR_ASSERT(absl::SimpleAtoi(StringViewFromSlice(slice), &out)); + GPR_ASSERT(absl::SimpleAtoi(slice.as_string_view(), &out)); return out; } static std::string DisplayValue(int32_t value) { @@ -57,14 +55,14 @@ struct Int32Trait { struct Int64Trait { using MementoType = int64_t; - static const char* key() { return "key3"; } + static absl::string_view key() { return "key3"; } static int64_t test_memento() { return 83481847284179298; } static int64_t test_value() { return -83481847284179298; } static size_t test_memento_transport_size() { return 87; } static int64_t MementoToValue(int64_t memento) { return -memento; } - static int64_t ParseMemento(const grpc_slice& slice) { + static int64_t ParseMemento(Slice slice) { int64_t out; - GPR_ASSERT(absl::SimpleAtoi(StringViewFromSlice(slice), &out)); + GPR_ASSERT(absl::SimpleAtoi(slice.as_string_view(), &out)); return out; } static std::string DisplayValue(int64_t value) { @@ -74,14 +72,14 @@ struct Int64Trait { struct IntptrTrait { using MementoType = intptr_t; - static const char* key() { return "key4"; } + static absl::string_view key() { return "key4"; } static intptr_t test_memento() { return 8374298; } static intptr_t test_value() { return test_memento() / 2; } static size_t test_memento_transport_size() { return 800; } static intptr_t MementoToValue(intptr_t memento) { return memento / 2; } - static intptr_t ParseMemento(const grpc_slice& slice) { + static intptr_t ParseMemento(Slice slice) { intptr_t out; - GPR_ASSERT(absl::SimpleAtoi(StringViewFromSlice(slice), &out)); + GPR_ASSERT(absl::SimpleAtoi(slice.as_string_view(), &out)); return out; } static std::string DisplayValue(intptr_t value) { @@ -91,15 +89,15 @@ struct IntptrTrait { struct StringTrait { using MementoType = std::string; - static const char* key() { return "key5-bin"; } + static absl::string_view key() { return "key5-bin"; } static std::string test_memento() { return "hello"; } static std::string test_value() { return "hi hello"; } static size_t test_memento_transport_size() { return 599; } static std::string MementoToValue(std::string memento) { return "hi " + memento; } - static std::string ParseMemento(const grpc_slice& slice) { - auto view = StringViewFromSlice(slice); + static std::string ParseMemento(Slice slice) { + auto view = slice.as_string_view(); return std::string(view.begin(), view.end()); } static std::string DisplayValue(const std::string& value) { return value; } diff --git a/test/core/transport/stream_owned_slice_test.cc b/test/core/transport/stream_owned_slice_test.cc deleted file mode 100644 index e7b76042f0c..00000000000 --- a/test/core/transport/stream_owned_slice_test.cc +++ /dev/null @@ -1,42 +0,0 @@ -/* - * - * Copyright 2017 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 "src/core/lib/transport/transport.h" -#include "test/core/util/test_config.h" - -static void do_nothing(void* /*arg*/, grpc_error_handle /*error*/) {} - -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(argc, argv); - grpc_init(); - - uint8_t buffer[] = "abc123"; - grpc_stream_refcount r; - GRPC_STREAM_REF_INIT(&r, 1, do_nothing, nullptr, "test"); - grpc_slice slice = - grpc_slice_from_stream_owned_buffer(&r, buffer, sizeof(buffer)); - GPR_ASSERT(GRPC_SLICE_START_PTR(slice) == buffer); - GPR_ASSERT(GRPC_SLICE_LENGTH(slice) == sizeof(buffer)); - grpc_slice_unref(slice); - - grpc_shutdown(); - return 0; -} diff --git a/test/core/util/BUILD b/test/core/util/BUILD index e1fb6ffb9dd..7202058f439 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -81,6 +81,7 @@ grpc_cc_library( ], language = "C++", deps = [ + ":build", ":grpc_suppressions", ":stack_tracer", "//:gpr", @@ -206,3 +207,9 @@ grpc_cc_library( "//:grpc", ], ) + +grpc_cc_library( + name = "build", + srcs = ["build.cc"], + hdrs = ["build.h"], +) diff --git a/test/core/util/build.cc b/test/core/util/build.cc new file mode 100644 index 00000000000..04d39e9574d --- /dev/null +++ b/test/core/util/build.cc @@ -0,0 +1,77 @@ +// Copyright 2021 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. + +bool BuiltUnderValgrind() { +#ifdef RUNNING_ON_VALGRIND + return true; +#else + return false; +#endif +} + +bool BuiltUnderTsan() { +#if defined(__has_feature) +#if __has_feature(thread_sanitizer) + return true; +#else + return false; +#endif +#else +#ifdef THREAD_SANITIZER + return true; +#else + return false; +#endif +#endif +} + +bool BuiltUnderAsan() { +#if defined(__has_feature) +#if __has_feature(address_sanitizer) + return true; +#else + return false; +#endif +#else +#ifdef ADDRESS_SANITIZER + return true; +#else + return false; +#endif +#endif +} + +bool BuiltUnderMsan() { +#if defined(__has_feature) +#if __has_feature(memory_sanitizer) + return true; +#else + return false; +#endif +#else +#ifdef MEMORY_SANITIZER + return true; +#else + return false; +#endif +#endif +} + +bool BuiltUnderUbsan() { +#ifdef GRPC_UBSAN + return true; +#else + return false; +#endif +} diff --git a/test/core/util/build.h b/test/core/util/build.h new file mode 100644 index 00000000000..9cdab0ced78 --- /dev/null +++ b/test/core/util/build.h @@ -0,0 +1,33 @@ +// Copyright 2021 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_TEST_CORE_UTIL_BUILD_H +#define GRPC_TEST_CORE_UTIL_BUILD_H + +// Returns whether this is built using our Valgrind config +bool BuiltUnderValgrind(); + +// Returns whether this is built under ThreadSanitizer +bool BuiltUnderTsan(); + +// Returns whether this is built under AddressSanitizer +bool BuiltUnderAsan(); + +// Returns whether this is built under MemorySanitizer +bool BuiltUnderMsan(); + +// Returns whether this is built under UndefinedBehaviorSanitizer +bool BuiltUnderUbsan(); + +#endif diff --git a/test/core/util/test_config.cc b/test/core/util/test_config.cc index 09952c0b349..1ae22f34019 100644 --- a/test/core/util/test_config.cc +++ b/test/core/util/test_config.cc @@ -37,6 +37,7 @@ #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/examine_stack.h" #include "src/core/lib/surface/init.h" +#include "test/core/util/build.h" #include "test/core/util/stack_tracer.h" int64_t g_fixture_slowdown_factor = 1; @@ -52,70 +53,6 @@ static unsigned seed(void) { return static_cast(getpid()); } static unsigned seed(void) { return (unsigned)_getpid(); } #endif -bool BuiltUnderValgrind() { -#ifdef RUNNING_ON_VALGRIND - return true; -#else - return false; -#endif -} - -bool BuiltUnderTsan() { -#if defined(__has_feature) -#if __has_feature(thread_sanitizer) - return true; -#else - return false; -#endif -#else -#ifdef THREAD_SANITIZER - return true; -#else - return false; -#endif -#endif -} - -bool BuiltUnderAsan() { -#if defined(__has_feature) -#if __has_feature(address_sanitizer) - return true; -#else - return false; -#endif -#else -#ifdef ADDRESS_SANITIZER - return true; -#else - return false; -#endif -#endif -} - -bool BuiltUnderMsan() { -#if defined(__has_feature) -#if __has_feature(memory_sanitizer) - return true; -#else - return false; -#endif -#else -#ifdef MEMORY_SANITIZER - return true; -#else - return false; -#endif -#endif -} - -bool BuiltUnderUbsan() { -#ifdef GRPC_UBSAN - return true; -#else - return false; -#endif -} - int64_t grpc_test_sanitizer_slowdown_factor() { int64_t sanitizer_multiplier = 1; if (BuiltUnderValgrind()) { diff --git a/test/core/util/test_config.h b/test/core/util/test_config.h index 6ac43de2666..35da4d0f8e8 100644 --- a/test/core/util/test_config.h +++ b/test/core/util/test_config.h @@ -21,6 +21,8 @@ #include +#include "test/core/util/build.h" + extern int64_t g_fixture_slowdown_factor; extern int64_t g_poller_slowdown_factor; @@ -37,18 +39,6 @@ gpr_timespec grpc_timeout_milliseconds_to_deadline(int64_t time_ms); #define GRPC_TEST_PICK_PORT #endif -// Returns whether this is built under ThreadSanitizer -bool BuiltUnderTsan(); - -// Returns whether this is built under AddressSanitizer -bool BuiltUnderAsan(); - -// Returns whether this is built under MemorySanitizer -bool BuiltUnderMsan(); - -// Returns whether this is built under UndefinedBehaviorSanitizer -bool BuiltUnderUbsan(); - // Prefer TestEnvironment below. void grpc_test_init(int argc, char** argv); diff --git a/tools/codegen/core/gen_static_metadata.py b/tools/codegen/core/gen_static_metadata.py index a1d50f06fd9..c0b098990c9 100755 --- a/tools/codegen/core/gen_static_metadata.py +++ b/tools/codegen/core/gen_static_metadata.py @@ -177,7 +177,6 @@ METADATA_BATCH_CALLOUTS = [ 'accept-encoding', 'grpc-internal-encoding-request', 'grpc-internal-stream-encoding-request', - 'user-agent', 'host', 'grpc-previous-rpc-attempts', 'grpc-retry-pushback-ms', @@ -430,13 +429,13 @@ for i, elem in enumerate(all_strs): def slice_def_for_ctx(i): return ( - 'grpc_core::StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' + 'StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' ) % (i, len(all_strs[i]), id2strofs[i]) def slice_def(i): return ( - 'grpc_core::StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' + 'StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' ) % (i, len(all_strs[i]), id2strofs[i]) @@ -452,7 +451,7 @@ for elem in METADATA_BATCH_CALLOUTS: static_slice_dest_assert = ( 'static_assert(std::is_trivially_destructible' + '::value, ' - '"grpc_core::StaticMetadataSlice must be trivially destructible.");') + '"StaticMetadataSlice must be trivially destructible.");') print(static_slice_dest_assert, file=STR_H) print('#define GRPC_STATIC_MDSTR_COUNT %d' % len(all_strs), file=STR_H) for i, elem in enumerate(all_strs): @@ -475,7 +474,7 @@ extern const uint8_t g_static_metadata_bytes[]; } ''', file=STR_H) -print('grpc_slice_refcount grpc_core::StaticSliceRefcount::kStaticSubRefcount;', +print('grpc_slice_refcount StaticSliceRefcount::kStaticSubRefcount;', file=STR_C) print(''' StaticSliceRefcount @@ -539,7 +538,7 @@ print('', file=STR_H) print('', file=STR_C) print('#define GRPC_STATIC_METADATA_INDEX(static_slice) \\', file=STR_H) print( - '(reinterpret_cast((static_slice).refcount)->index)', + '(reinterpret_cast<::grpc_core::StaticSliceRefcount*>((static_slice).refcount)->index)', file=STR_H) print('', file=STR_H) diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index a732113f407..80044680bc1 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2171,6 +2171,7 @@ src/core/lib/slice/b64.h \ src/core/lib/slice/percent_encoding.cc \ src/core/lib/slice/percent_encoding.h \ 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 \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index c704210eaae..008872b004d 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1970,6 +1970,7 @@ src/core/lib/slice/b64.h \ src/core/lib/slice/percent_encoding.cc \ src/core/lib/slice/percent_encoding.h \ 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 \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 4ac6a3f5e3b..d0ac6b7d89f 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2511,30 +2511,6 @@ ], "uses_polling": true }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "stream_owned_slice_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false, @@ -2695,30 +2671,6 @@ ], "uses_polling": true }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "test_core_slice_slice_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false, @@ -6885,6 +6837,30 @@ ], "uses_polling": false }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "test_core_slice_slice_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false,