diff --git a/BUILD b/BUILD index ed081cd1a17..17d763d72ee 100644 --- a/BUILD +++ b/BUILD @@ -1541,6 +1541,7 @@ grpc_cc_library( ], deps = [ "gpr_base", + "ref_counted", ], ) @@ -1551,7 +1552,6 @@ 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 ad44908acd0..d21fed5cbef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -696,6 +696,7 @@ 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) @@ -709,6 +710,7 @@ 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) @@ -937,7 +939,6 @@ 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) @@ -2207,7 +2208,6 @@ 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,7 +2276,6 @@ 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 @@ -7042,6 +7041,33 @@ 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) @@ -7238,6 +7264,38 @@ 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) @@ -15670,47 +15728,6 @@ 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) @@ -16777,7 +16794,6 @@ 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 9bffdfc3e4a..14f759345a8 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -914,7 +914,6 @@ 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 @@ -1584,7 +1583,6 @@ 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 @@ -1607,7 +1605,6 @@ 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 @@ -1639,7 +1636,6 @@ 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 @@ -1661,7 +1657,6 @@ 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 @@ -1943,7 +1938,6 @@ 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 @@ -3738,7 +3732,6 @@ 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 @@ -4056,7 +4049,6 @@ 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 @@ -4150,6 +4142,15 @@ 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 @@ -4222,6 +4223,30 @@ 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 @@ -5700,7 +5725,6 @@ 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 @@ -6783,7 +6807,6 @@ 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 @@ -7507,7 +7530,6 @@ 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 @@ -7964,34 +7986,6 @@ 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 @@ -8542,7 +8536,6 @@ 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 @@ -8567,7 +8560,6 @@ 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 7901c6ebe4a..cd196de9df2 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -773,7 +773,6 @@ 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', @@ -1485,7 +1484,6 @@ 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 4f32d80154b..1c4f9b6888c 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1276,7 +1276,6 @@ 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', @@ -2009,7 +2008,6 @@ 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', @@ -2272,8 +2270,6 @@ 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 2be20dec14a..43c4e6be18b 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1196,7 +1196,6 @@ 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 07e2df3d6da..a4194c1cc12 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -1084,7 +1084,6 @@ 'grpc', ], 'sources': [ - 'test/core/util/build.cc', 'test/core/util/cmdline.cc', 'test/core/util/fuzzer_util.cc', 'test/core/util/grpc_profiler.cc', @@ -1118,7 +1117,6 @@ '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 61b10179b93..31ab5a475eb 100644 --- a/package.xml +++ b/package.xml @@ -1176,7 +1176,6 @@ - diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 30707e06ab7..4b79a3b813c 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -2428,9 +2428,16 @@ class ClientChannel::LoadBalancedCall::Metadata std::vector> TestOnlyCopyToVector() override { - Encoder encoder; - batch_->Encode(&encoder); - return encoder.Take(); + 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; } absl::optional Lookup(absl::string_view key, @@ -2439,33 +2446,6 @@ 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 37fb8f1fe06..0804f907093 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -79,6 +79,7 @@ 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; @@ -103,7 +104,7 @@ struct call_data { struct channel_data { grpc_mdelem static_scheme; - grpc_core::Slice user_agent; + grpc_mdelem user_agent; size_t max_payload_size_for_get; }; } // namespace @@ -444,6 +445,9 @@ 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. */ @@ -462,8 +466,11 @@ 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; - batch->payload->send_initial_metadata.send_initial_metadata->Set( - grpc_core::UserAgentMetadata(), channeld->user_agent.Ref()); + 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; } done: @@ -527,8 +534,8 @@ static size_t max_payload_size_from_args(const grpc_channel_args* args) { return kMaxPayloadSizeForGet; } -static grpc_core::Slice user_agent_from_args(const grpc_channel_args* args, - const char* transport_name) { +static grpc_core::ManagedMemorySlice 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++) { @@ -558,27 +565,29 @@ static grpc_core::Slice user_agent_from_args(const grpc_channel_args* args, } std::string user_agent_string = absl::StrJoin(user_agent_fields, " "); - return grpc_core::Slice::FromCopiedString(user_agent_string.c_str()); + return grpc_core::ManagedMemorySlice(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_core::Slice(user_agent_from_args( - args->channel_args, args->optional_transport->vtable->name)); + chand->user_agent = grpc_mdelem_from_slices( + GRPC_MDSTR_USER_AGENT, + 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) { - static_cast(elem->channel_data)->~channel_data(); + channel_data* chand = static_cast(elem->channel_data); + GRPC_MDELEM_UNREF(chand->user_agent); } 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 abf6d190077..0a2108d0051 100644 --- a/src/core/ext/filters/http/server/http_server_filter.cc +++ b/src/core/ext/filters/http/server/http_server_filter.cc @@ -329,8 +329,9 @@ 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->Remove(grpc_core::UserAgentMetadata()); + if (!chand->surface_user_agent && + b->legacy_index()->named.user_agent != nullptr) { + b->Remove(GRPC_BATCH_USER_AGENT); } 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 1d0f94edfa2..68d393f11ea 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_core::Slice::FromCopiedString(p.second)); + mb->Append(p.first, grpc_slice_from_cpp_string(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 d70a30cc683..fe0910adba4 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.cc @@ -476,20 +476,12 @@ void HPackCompressor::Framer::EncodeDynamic(grpc_mdelem elem) { void HPackCompressor::Framer::Encode(TeMetadata, TeMetadata::ValueType value) { GPR_ASSERT(value == TeMetadata::ValueType::kTrailers); - 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)); + if (compressor_->table_.ConvertableToDynamicIndex(compressor_->te_index_)) { + EmitIndexed(compressor_->table_.DynamicIndex(compressor_->te_index_)); } else { - *index = compressor_->table_.AllocateIndex(transport_length); - EmitLitHdrWithNonBinaryStringKeyIncIdx(key, value); + compressor_->te_index_ = compressor_->table_.AllocateIndex( + 2 /* te */ + 8 /* trailers */ + hpack_constants::kEntryOverhead); + EmitLitHdrWithNonBinaryStringKeyIncIdx(GRPC_MDSTR_TE, GRPC_MDSTR_TRAILERS); } } @@ -504,16 +496,6 @@ 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 0702a391b53..172578d2854 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_encoder.h +++ b/src/core/ext/transport/chttp2/transport/hpack_encoder.h @@ -119,7 +119,6 @@ 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 { @@ -146,10 +145,6 @@ 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); @@ -274,12 +269,7 @@ 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 1f10092dcb4..eb2f2f456cf 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -656,19 +656,15 @@ class HPackParser::String { private: // Forward declare take functions... we'll need them in the public interface - Slice Take(Extern); - Slice Take(Intern); + UnmanagedMemorySlice Take(Extern); + ManagedMemorySlice Take(Intern); public: - String(const String&) = delete; - String& operator=(const String&) = delete; - String(String&& other) noexcept : value_(std::move(other.value_)) { - other.value_ = absl::Span(); - } - String& operator=(String&& other) noexcept { - value_ = std::move(other.value_); - other.value_ = absl::Span(); - return *this; + // 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 @@ -678,18 +674,15 @@ class HPackParser::String { 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()); - }); + String(const String&) = delete; + String& operator=(const String&) = delete; + String(String&& other) noexcept : value_(std::move(other.value_)) { + other.value_ = absl::Span(); + } + String& operator=(String&& other) noexcept { + value_ = std::move(other.value_); + other.value_ = absl::Span(); + return *this; } // Parse a non-binary string @@ -765,7 +758,18 @@ 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_(Slice::FromRefcountAndBytes(r, begin, 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; + } // Parse some huffman encoded bytes, using output(uint8_t b) to emit each // decoded byte. @@ -927,7 +931,8 @@ 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. @@ -1124,16 +1129,14 @@ class HPackParser::Parser { absl::optional ParseLiteralKey() { auto key = String::Parse(input_); if (!key.has_value()) return {}; - auto value = ParseValueString(absl::EndsWith(key->string_view(), "-bin")); + auto key_slice = key->Take(); + auto value = + ParseValueString(grpc_is_refcounted_slice_binary_header(key_slice)); if (GPR_UNLIKELY(!value.has_value())) { + grpc_slice_unref_internal(key_slice); return {}; } - 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); + return grpc_metadata_batch::Parse(key_slice, value->Take()); } // Parse an index encoded key and a string encoded value @@ -1241,7 +1244,7 @@ class HPackParser::Parser { const LogInfo log_info_; }; -Slice HPackParser::String::Take(Extern) { +UnmanagedMemorySlice HPackParser::String::Take(Extern) { auto s = Match( value_, [](const grpc_slice& slice) { @@ -1261,10 +1264,10 @@ Slice HPackParser::String::Take(Extern) { v.size()); }); value_ = absl::Span(); - return Slice(s); + return s; } -Slice HPackParser::String::Take(Intern) { +ManagedMemorySlice HPackParser::String::Take(Intern) { auto s = Match( value_, [](const grpc_slice& slice) { @@ -1282,7 +1285,7 @@ Slice HPackParser::String::Take(Intern) { v.size()); }); value_ = absl::Span(); - return Slice(s); + return 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 7cdeac54f1f..4c719e5bf94 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, grpc_core::Slice(value)); + mds->Append(header_array->headers[i].key, value); } } @@ -714,11 +714,10 @@ class CronetMetadataEncoder { CronetMetadataEncoder& operator=(const CronetMetadataEncoder&) = delete; template - void Encode(T, const V& value) { + void Encode(T, V value) { auto value_slice = T::Encode(value); - auto key_slice = - grpc_core::ExternallyManagedSlice(T::key().data(), T::key().length()); - auto mdelem = grpc_mdelem_from_slices(key_slice, value_slice.TakeCSlice()); + auto key_slice = grpc_slice_from_static_string(T::key()); + auto mdelem = grpc_mdelem_from_slices(key_slice, value_slice); 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 e4ba6ca740a..9994987770c 100644 --- a/src/core/ext/transport/inproc/inproc_transport.cc +++ b/src/core/ext/transport/inproc/inproc_transport.cc @@ -316,11 +316,6 @@ 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 8b6994e76d8..ea2a3c261cc 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_; - std::atomic refs_{1}; + grpc_core::RefCount refs_; std::shared_ptr allocator_; size_t size_; }; diff --git a/src/core/lib/gprpp/table.h b/src/core/lib/gprpp/table.h index 01c2140c090..b91b0c0da49 100644 --- a/src/core/lib/gprpp/table.h +++ b/src/core/lib/gprpp/table.h @@ -268,25 +268,13 @@ class Table { TypeIndex* set(Args&&... args) { auto* p = element_ptr(); if (set_present(true)) { - TypeIndex replacement(std::forward(args)...); - *p = std::move(replacement); + *p = TypeIndex(std::forward(args)...); } 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 857db368adc..44af1f4f748 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_; - std::atomic refs_{1}; + RefCount refs_; void (*user_destroy_)(void*); void* user_data_; }; @@ -130,7 +130,7 @@ class NewWithLenSliceRefcount { private: grpc_slice_refcount base_; - std::atomic refs_{1}; + RefCount refs_; void* user_data_; size_t user_length_; void (*user_destroy_)(void*, size_t); @@ -152,7 +152,7 @@ class MovedStringSliceRefCount { } grpc_slice_refcount base_; - std::atomic refs_{1}; + RefCount refs_; UniquePtr str_; }; @@ -172,7 +172,7 @@ class MovedCppStringSliceRefCount { } grpc_slice_refcount base_; - std::atomic refs_{1}; + RefCount refs_; std::string str_; }; @@ -281,7 +281,7 @@ class MallocRefCount { private: grpc_slice_refcount base_; - std::atomic refs_{1}; + grpc_core::RefCount refs_; }; } // namespace diff --git a/src/core/lib/slice/slice.h b/src/core/lib/slice/slice.h deleted file mode 100644 index 73c4cce8abe..00000000000 --- a/src/core/lib/slice/slice.h +++ /dev/null @@ -1,313 +0,0 @@ -// 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 0bed91a9e2c..d3744570da8 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 (grpc_core::IncrementIfNonzero(&s->refcnt)) { + if (s->refcnt.RefIfNonZero()) { return s; } } diff --git a/src/core/lib/slice/slice_refcount.h b/src/core/lib/slice/slice_refcount.h index f16b251c8fd..1a98f19c853 100644 --- a/src/core/lib/slice/slice_refcount.h +++ b/src/core/lib/slice/slice_refcount.h @@ -17,8 +17,6 @@ #include -#include - #include "src/core/lib/slice/slice_refcount_base.h" #include "src/core/lib/slice/static_slice.h" @@ -30,8 +28,6 @@ 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); @@ -52,7 +48,7 @@ struct InternedSliceRefcount { grpc_slice_refcount base; grpc_slice_refcount sub; const size_t length; - std::atomic refcnt{1}; + RefCount refcnt; 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 9c1e34056b4..62ad152e70b 100644 --- a/src/core/lib/slice/slice_refcount_base.h +++ b/src/core/lib/slice/slice_refcount_base.h @@ -17,10 +17,9 @@ #include -#include - #include -#include + +#include "src/core/lib/gprpp/ref_counted.h" // grpc_slice_refcount : A reference count for grpc_slice. // @@ -104,10 +103,11 @@ struct grpc_slice_refcount { // Whether we are the refcount for a static // metadata slice, an interned slice, or any other kind of slice. // - // 2. std::atomic* ref - // The pointer to the actual underlying grpc_core::RefCount. - // TODO(ctiller): remove the pointer indirection and just put the refcount on - // this object once we remove interning. + // 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. // // 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, std::atomic* ref, + grpc_slice_refcount(grpc_slice_refcount::Type type, grpc_core::RefCount* ref, DestroyerFn destroyer_fn, void* destroyer_arg, grpc_slice_refcount* sub) : ref_(ref), @@ -136,27 +136,19 @@ struct grpc_slice_refcount { uint32_t Hash(const grpc_slice& slice); void Ref() { if (ref_ == nullptr) return; - ref_->fetch_add(1, std::memory_order_relaxed); + ref_->RefNonZero(); } void Unref() { if (ref_ == nullptr) return; - if (ref_->fetch_sub(1, std::memory_order_acq_rel) == 1) { + if (ref_->Unref()) { 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: - std::atomic* ref_ = nullptr; + grpc_core::RefCount* 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 9c475aad25c..76bc08ed7bb 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, 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, + 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, 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, 4, + StaticMetadataSlice(&g_static_metadata_slice_refcounts[18].base, 10, g_static_metadata_bytes + 266), - 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[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[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 a7d73f15669..893b2efd2d8 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, - "StaticMetadataSlice must be trivially destructible."); + "grpc_core::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[18]) +#define GRPC_MDSTR_HOST (::grpc_core::g_static_metadata_slice_table[19]) /* "grpc-previous-rpc-attempts" */ #define GRPC_MDSTR_GRPC_PREVIOUS_RPC_ATTEMPTS \ - (::grpc_core::g_static_metadata_slice_table[19]) + (::grpc_core::g_static_metadata_slice_table[20]) /* "grpc-retry-pushback-ms" */ #define GRPC_MDSTR_GRPC_RETRY_PUSHBACK_MS \ - (::grpc_core::g_static_metadata_slice_table[20]) + (::grpc_core::g_static_metadata_slice_table[21]) /* "x-endpoint-load-metrics-bin" */ #define GRPC_MDSTR_X_ENDPOINT_LOAD_METRICS_BIN \ - (::grpc_core::g_static_metadata_slice_table[21]) + (::grpc_core::g_static_metadata_slice_table[22]) /* "grpc-timeout" */ -#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]) +#define GRPC_MDSTR_GRPC_TIMEOUT (::grpc_core::g_static_metadata_slice_table[23]) /* "1" */ #define GRPC_MDSTR_1 (::grpc_core::g_static_metadata_slice_table[24]) /* "2" */ @@ -324,9 +324,8 @@ 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<::grpc_core::StaticSliceRefcount*>( \ - (static_slice).refcount) \ +#define GRPC_STATIC_METADATA_INDEX(static_slice) \ + (reinterpret_cast((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 c9af114548c..632f6ed77d0 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -988,50 +988,27 @@ 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->count() == 0) return; + if (b->non_deadline_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->count() > dest->capacity) { - dest->capacity = - std::max(dest->capacity + b->count(), dest->capacity * 3 / 2); + if (dest->count + b->non_deadline_count() > dest->capacity) { + dest->capacity = std::max(dest->capacity + b->non_deadline_count(), + dest->capacity * 3 / 2); dest->metadata = static_cast( gpr_realloc(dest->metadata, sizeof(grpc_metadata) * dest->capacity)); } - PublishToAppEncoder encoder(dest); - b->Encode(&encoder); + 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); + }); } 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 f74c3ecf430..c651faa72e0 100644 --- a/src/core/lib/transport/metadata_batch.cc +++ b/src/core/lib/transport/metadata_batch.cc @@ -71,11 +71,6 @@ 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 d2b8de6b03c..2a90e823ed1 100644 --- a/src/core/lib/transport/metadata_batch.h +++ b/src/core/lib/transport/metadata_batch.h @@ -34,7 +34,6 @@ #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" @@ -83,12 +82,13 @@ namespace grpc_core { struct GrpcTimeoutMetadata { using ValueType = grpc_millis; using MementoType = grpc_millis; - static absl::string_view key() { return "grpc-timeout"; } - static MementoType ParseMemento(Slice value) { + static const char* key() { return "grpc-timeout"; } + static MementoType ParseMemento(const grpc_slice& value) { grpc_millis timeout; - if (GPR_UNLIKELY(!grpc_http2_decode_timeout(value.c_slice(), &timeout))) { + if (GPR_UNLIKELY(!grpc_http2_decode_timeout(value, &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 Slice Encode(ValueType x) { + static grpc_slice Encode(ValueType x) { char timeout[GRPC_HTTP2_TIMEOUT_ENCODE_MIN_BUFSIZE]; grpc_http2_encode_timeout(x, timeout); - return Slice::FromCopiedString(timeout); + return grpc_slice_from_copied_string(timeout); } static MementoType DisplayValue(MementoType x) { return x; } }; @@ -115,18 +115,19 @@ struct TeMetadata { kInvalid, }; using MementoType = ValueType; - static absl::string_view key() { return "te"; } - static MementoType ParseMemento(Slice value) { + static const char* key() { return "te"; } + static MementoType ParseMemento(const grpc_slice& value) { auto out = kInvalid; - if (value == "trailers") { + if (grpc_slice_eq(value, GRPC_MDSTR_TRAILERS)) { out = kTrailers; } + grpc_slice_unref_internal(value); return out; } static ValueType MementoToValue(MementoType te) { return te; } - static StaticSlice Encode(ValueType x) { + static grpc_slice Encode(ValueType x) { GPR_ASSERT(x == kTrailers); - return StaticSlice(GRPC_MDSTR_TRAILERS); + return GRPC_MDSTR_TRAILERS; } static const char* DisplayValue(MementoType te) { switch (te) { @@ -138,19 +139,6 @@ 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() @@ -163,24 +151,25 @@ struct ParseHelper; template struct ParseHelper { template - static ParsedMetadata Parse(absl::string_view key, Slice value, - uint32_t transport_size, + static ParsedMetadata Parse(absl::string_view key, + const grpc_slice& value, NotFound not_found) { if (key == Trait::key()) { return ParsedMetadata( - Trait(), Trait::ParseMemento(value.TakeOwned()), transport_size); + Trait(), Trait::ParseMemento(value), + ParsedMetadata::TransportSize(key.size(), + GRPC_SLICE_LENGTH(value))); } - return ParseHelper::Parse(key, std::move(value), - transport_size, not_found); + return ParseHelper::Parse(key, value, not_found); } }; template struct ParseHelper { template - static ParsedMetadata Parse(absl::string_view, Slice value, - uint32_t, NotFound not_found) { - return not_found(std::move(value)); + static ParsedMetadata Parse(absl::string_view, const grpc_slice&, + NotFound not_found) { + return not_found(); } }; @@ -194,14 +183,14 @@ struct AppendHelper; template struct AppendHelper { template - static void Append(Container* container, absl::string_view key, Slice value, - NotFound not_found) { + static void Append(Container* container, absl::string_view key, + const grpc_slice& value, NotFound not_found) { if (key == Trait::key()) { - container->Set(Trait(), Trait::MementoToValue( - Trait::ParseMemento(value.TakeOwned()))); + container->Set(Trait(), + Trait::MementoToValue(Trait::ParseMemento(value))); return; } - AppendHelper::Append(container, key, std::move(value), + AppendHelper::Append(container, key, value, not_found); } }; @@ -209,9 +198,9 @@ struct AppendHelper { template struct AppendHelper { template - static void Append(Container*, absl::string_view, Slice value, + static void Append(Container*, absl::string_view, const grpc_slice&, NotFound not_found) { - not_found(std::move(value)); + not_found(); } }; @@ -239,15 +228,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 absl::string_view key() { return "grpc-xyz"; } +// static constexpr char* key() { return "grpc-xyz"; } // // Parse a memento from a slice // // Takes ownership of value -// static MementoType ParseMemento(Slice value) { ... } +// static MementoType ParseMemento(const grpc_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 Slice Encode(const ValueType& value); +// static grpc_slice Encode(ValueType value); // // Convert a value to something that can be passed to StrCat and displayed // // for debugging // static SomeStrCatableType DisplayValue(MementoType value) { ... } @@ -361,17 +350,19 @@ 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! - static ParsedMetadata Parse(absl::string_view key, Slice value, - uint32_t transport_size) { + template + static ParsedMetadata Parse(const KeySlice& key, + const ValueSlice& value) { bool parsed = true; auto out = metadata_detail::ParseHelper::Parse( - key, std::move(value), transport_size, [&](Slice value) { + StringViewFromSlice(key), value, [&] { parsed = false; - return ParsedMetadata(grpc_mdelem_from_slices( - grpc_slice_intern( - grpc_slice_from_static_buffer(key.data(), key.size())), - value.TakeCSlice())); + return ParsedMetadata( + grpc_mdelem_from_slices(key, value)); }); + if (parsed) { + grpc_slice_unref_internal(key); + } return out; } @@ -382,14 +373,14 @@ class MetadataMap { } // Append a key/value pair - takes ownership of value - void Append(absl::string_view key, Slice value) { + void Append(absl::string_view key, const grpc_slice& value) { metadata_detail::AppendHelper::Append( - this, key, std::move(value), [this, key](Slice value) { + this, key, value, [&] { GPR_ASSERT(GRPC_ERROR_NONE == Append(grpc_mdelem_from_slices( grpc_slice_intern(grpc_slice_from_static_buffer( key.data(), key.length())), - value.TakeCSlice()))); + value))); }); } @@ -503,15 +494,10 @@ class MetadataMap { struct Value { Value() = default; explicit Value(const typename Which::ValueType& value) : value(value) {} - explicit Value(typename Which::ValueType&& value) - : value(std::forward(value)) {} - Value(const Value&) = delete; - Value& operator=(const Value&) = delete; + Value(const Value&) = default; + Value& operator=(const Value&) = default; Value(Value&&) noexcept = default; - Value& operator=(Value&& other) noexcept { - value = std::move(other.value); - return *this; - } + Value& operator=(Value&&) noexcept = default; GPR_NO_UNIQUE_ADDRESS typename Which::ValueType value; }; // Callable for the ForEach in Encode() -- for each value, call the @@ -910,7 +896,7 @@ bool MetadataMap::ReplaceIfExists(grpc_slice key, grpc_slice value) { using grpc_metadata_batch = grpc_core::MetadataMap; + grpc_core::TeMetadata>; 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 7c8cdc70cbd..305c369ffd0 100644 --- a/src/core/lib/transport/parsed_metadata.h +++ b/src/core/lib/transport/parsed_metadata.h @@ -24,7 +24,6 @@ #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" @@ -38,7 +37,7 @@ template struct HasSimpleMemento { static constexpr bool value = std::is_trivial::value && - sizeof(typename Which::MementoType) <= sizeof(uint64_t); + sizeof(typename Which::MementoType) <= sizeof(intptr_t); }; } // namespace metadata_detail @@ -61,37 +60,27 @@ class ParsedMetadata { value, uint32_t transport_size) : vtable_(ParsedMetadata::template TrivialTraitVTable()), - transport_size_(transport_size) { - value_.trivial = static_cast(value); - } + value_(static_cast(value)), + transport_size_(transport_size) {} template ParsedMetadata( Which, - absl::enable_if_t< - !metadata_detail::HasSimpleMemento::value && - !std::is_convertible::value, - typename Which::MementoType> + absl::enable_if_t::value, + typename Which::MementoType> value, uint32_t transport_size) : vtable_(ParsedMetadata::template NonTrivialTraitVTable()), - 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(); - } + value_( + reinterpret_cast(new typename Which::MementoType(value))), + transport_size_(transport_size) {} // Takes ownership of elem explicit ParsedMetadata(grpc_mdelem elem) : vtable_(grpc_is_binary_header_internal(GRPC_MDKEY(elem)) ? MdelemVtable() : MdelemVtable()), - transport_size_(GRPC_MDELEM_LENGTH(elem)) { - value_.mdelem = elem; - } - ParsedMetadata() : vtable_(EmptyVTable()), transport_size_(0) {} + value_(static_cast(elem.payload)), + transport_size_(GRPC_MDELEM_LENGTH(elem)) {} + ParsedMetadata() : vtable_(EmptyVTable()), value_(0), transport_size_(0) {} ~ParsedMetadata() { vtable_->destroy(value_); } // Non copyable, but movable. @@ -122,8 +111,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(Slice value) const { - return vtable_->with_new_value(value_, std::move(value)); + ParsedMetadata WithNewValue(const grpc_slice& value) const { + return vtable_->with_new_value(value_, value); } std::string DebugString() const { return vtable_->debug_string(value_); } @@ -134,21 +123,14 @@ 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)(const Buffer& value); - grpc_error_handle (*const set)(const Buffer& value, + void (*const destroy)(intptr_t value); + grpc_error_handle (*const set)(intptr_t value, MetadataContainer* container); - ParsedMetadata (*const with_new_value)(const Buffer& value, - Slice new_value); - std::string (*debug_string)(const Buffer& value); + ParsedMetadata (*const with_new_value)(intptr_t value, + const grpc_slice& new_value); + std::string (*debug_string)(intptr_t value); }; static const VTable* EmptyVTable(); @@ -156,13 +138,11 @@ class ParsedMetadata { static const VTable* TrivialTraitVTable(); template static const VTable* NonTrivialTraitVTable(); - template - static const VTable* SliceTraitVTable(); template static const VTable* MdelemVtable(); const VTable* vtable_; - Buffer value_; + intptr_t value_; uint32_t transport_size_; }; @@ -172,13 +152,13 @@ ParsedMetadata::EmptyVTable() { static const VTable vtable = { false, // destroy - [](const Buffer&) {}, + [](intptr_t) {}, // set - [](const Buffer&, MetadataContainer*) { return GRPC_ERROR_NONE; }, + [](intptr_t, MetadataContainer*) { return GRPC_ERROR_NONE; }, // with_new_value - [](const Buffer&, Slice) { return ParsedMetadata(); }, + [](intptr_t, const grpc_slice&) { return ParsedMetadata(); }, // debug_string - [](const Buffer&) -> std::string { return "empty"; }}; + [](intptr_t) -> std::string { return "empty"; }}; return &vtable; } @@ -189,26 +169,25 @@ ParsedMetadata::TrivialTraitVTable() { static const VTable vtable = { absl::EndsWith(Which::key(), "-bin"), // destroy - [](const Buffer&) {}, + [](intptr_t) {}, // set - [](const Buffer& value, MetadataContainer* map) { - map->Set(Which(), - Which::MementoToValue( - static_cast(value.trivial))); + [](intptr_t value, MetadataContainer* map) { + map->Set(Which(), Which::MementoToValue( + static_cast(value))); 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)); + [](intptr_t, const grpc_slice& value) { + return ParsedMetadata( + Which(), Which::ParseMemento(value), + TransportSize(strlen(Which::key()), GRPC_SLICE_LENGTH(value))); }, // debug_string - [](const Buffer& value) { + [](intptr_t value) { return absl::StrCat( Which::key(), ": ", Which::DisplayValue( - static_cast(value.trivial))); + static_cast(value))); }}; return &vtable; } @@ -220,57 +199,29 @@ ParsedMetadata::NonTrivialTraitVTable() { static const VTable vtable = { absl::EndsWith(Which::key(), "-bin"), // destroy - [](const Buffer& value) { - delete static_cast(value.pointer); + [](intptr_t value) { + delete reinterpret_cast(value); }, // set - [](const Buffer& value, MetadataContainer* map) { - auto* p = static_cast(value.pointer); + [](intptr_t value, MetadataContainer* map) { + auto* p = reinterpret_cast(value); map->Set(Which(), Which::MementoToValue(*p)); 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)); + [](intptr_t, const grpc_slice& value) { + return ParsedMetadata( + Which(), Which::ParseMemento(value), + TransportSize(strlen(Which::key()), GRPC_SLICE_LENGTH(value))); }, // debug_string - [](const Buffer& value) { - auto* p = static_cast(value.pointer); + [](intptr_t value) { + auto* p = reinterpret_cast(value); 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* @@ -278,10 +229,10 @@ ParsedMetadata::MdelemVtable() { static const VTable vtable = { kIsBinaryHeader, // destroy - [](const Buffer& value) { GRPC_MDELEM_UNREF(value.mdelem); }, + [](intptr_t value) { GRPC_MDELEM_UNREF(grpc_mdelem{uintptr_t(value)}); }, // set - [](const Buffer& value, MetadataContainer* map) { - auto md = GRPC_MDELEM_REF(value.mdelem); + [](intptr_t value, MetadataContainer* map) { + auto md = GRPC_MDELEM_REF(grpc_mdelem{uintptr_t(value)}); 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. @@ -291,16 +242,18 @@ ParsedMetadata::MdelemVtable() { return err; }, // with_new_value - [](const Buffer& value, Slice value_slice) { + [](intptr_t value, const grpc_slice& value_slice) { + grpc_mdelem elem{uintptr_t(value)}; return ParsedMetadata(grpc_mdelem_from_slices( static_cast( - grpc_slice_ref_internal(GRPC_MDKEY(value.mdelem))), - value_slice.TakeCSlice())); + grpc_slice_ref_internal(GRPC_MDKEY(elem))), + value_slice)); }, // debug_string - [](const Buffer& value) { - return absl::StrCat(StringViewFromSlice(GRPC_MDKEY(value.mdelem)), ": ", - StringViewFromSlice(GRPC_MDVALUE(value.mdelem))); + [](intptr_t value) { + grpc_mdelem elem{uintptr_t(value)}; + return absl::StrCat(StringViewFromSlice(GRPC_MDKEY(elem)), ": ", + StringViewFromSlice(GRPC_MDVALUE(elem))); }}; return &vtable; } diff --git a/src/core/lib/transport/static_metadata.cc b/src/core/lib/transport/static_metadata.cc index b9a4ba5aaab..c6c745d9d2f 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[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[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[23].base, 10, - g_static_metadata_bytes + 357), + StaticMetadataSlice(&g_static_metadata_slice_refcounts[18].base, 10, + g_static_metadata_bytes + 266), 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, 0, 0, 0, 0, -2, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 7, 6, 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, 0, 0, 0, 0, -50, 0, -55, -75, -76, -77, 0, - 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, + 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, 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, 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, + 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, 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, 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, + 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, 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 272d16a606c..8b5783e76df 100644 --- a/src/core/lib/transport/static_metadata.h +++ b/src/core/lib/transport/static_metadata.h @@ -277,6 +277,7 @@ 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, @@ -305,6 +306,7 @@ 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 c4f4fb1675f..5555b62eb1e 100644 --- a/src/core/lib/transport/transport.cc +++ b/src/core/lib/transport/transport.cc @@ -61,6 +61,25 @@ 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, @@ -75,6 +94,9 @@ 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 72440cda5ca..5d223ee5f78 100644 --- a/src/core/lib/transport/transport.h +++ b/src/core/lib/transport/transport.h @@ -59,6 +59,7 @@ 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 16c01203882..230d43e4e49 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, const typename Which::ValueType& value) { + void Encode(Which, 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 e9f60d1d996..0bad6e63bf8 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 192443a4a29..854418a2bf1 100644 --- a/test/core/slice/BUILD +++ b/test/core/slice/BUILD @@ -82,15 +82,11 @@ 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 1f5ea505054..3da56f16eb8 100644 --- a/test/core/slice/slice_test.cc +++ b/test/core/slice/slice_test.cc @@ -18,15 +18,9 @@ #include -#include "src/core/lib/slice/slice.h" - #include #include -#include - -#include - #include #include #include @@ -34,24 +28,27 @@ #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/slice/slice_internal.h" -#include "test/core/util/build.h" -TEST(GrpcSliceTest, MallocReturnsSomethingSensible) { +#define LOG_TEST_NAME(x) gpr_log(GPR_INFO, "%s", x); + +static void test_slice_malloc_returns_something_sensible(void) { /* 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) { - EXPECT_NE(slice.data.refcounted.bytes, nullptr); + GPR_ASSERT(slice.data.refcounted.bytes); } /* Returned slice length must be what was requested. */ - EXPECT_EQ(GRPC_SLICE_LENGTH(slice), length); + GPR_ASSERT(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); @@ -63,20 +60,20 @@ TEST(GrpcSliceTest, MallocReturnsSomethingSensible) { static void do_nothing(void* /*ignored*/) {} -TEST(GrpcSliceTest, SliceNewReturnsSomethingSensible) { +static void test_slice_new_returns_something_sensible(void) { uint8_t x; grpc_slice slice = grpc_slice_new(&x, 1, do_nothing); - EXPECT_NE(slice.refcount, nullptr); - EXPECT_EQ(slice.data.refcounted.bytes, &x); - EXPECT_EQ(slice.data.refcounted.length, 1); + GPR_ASSERT(slice.refcount); + GPR_ASSERT(slice.data.refcounted.bytes == &x); + GPR_ASSERT(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; } -TEST(GrpcSliceTest, SliceNewWithUserData) { +static void test_slice_new_with_user_data(void) { int marker = 0; uint8_t buf[2]; grpc_slice slice; @@ -84,34 +81,33 @@ TEST(GrpcSliceTest, SliceNewWithUserData) { buf[0] = 0; buf[1] = 1; slice = grpc_slice_new_with_user_data(buf, 2, set_mark, &marker); - 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); + 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); /* unref should cause destroy function to run. */ grpc_slice_unref_internal(slice); - EXPECT_EQ(marker, 1); + GPR_ASSERT(marker == 1); } static int do_nothing_with_len_1_calls = 0; static void do_nothing_with_len_1(void* /*ignored*/, size_t len) { - EXPECT_EQ(len, 1); + GPR_ASSERT(len == 1); do_nothing_with_len_1_calls++; } -TEST(GrpcSliceTest, SliceNewWithLenReturnsSomethingSensible) { +static void test_slice_new_with_len_returns_something_sensible(void) { 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); - 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); + 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); /* 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 @@ -122,22 +118,21 @@ TEST(GrpcSliceTest, SliceNewWithLenReturnsSomethingSensible) { for (i = 0; i < num_refs; i++) { grpc_slice_unref_internal(slice); } - EXPECT_EQ(do_nothing_with_len_1_calls, 0); /* Shouldn't be called yet */ + GPR_ASSERT(do_nothing_with_len_1_calls == 0); /* Shouldn't be called yet */ /* last unref */ grpc_slice_unref_internal(slice); - EXPECT_EQ(do_nothing_with_len_1_calls, 1); + GPR_ASSERT(do_nothing_with_len_1_calls == 1); } -class GrpcSliceSizedTest : public ::testing::TestWithParam {}; - -TEST_P(GrpcSliceSizedTest, SliceSubWorks) { - const auto length = GetParam(); - +static void test_slice_sub_works(unsigned length) { 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); @@ -150,9 +145,9 @@ TEST_P(GrpcSliceSizedTest, SliceSubWorks) { for (i = 0; i < length; i++) { for (j = i; j < length; j++) { sub = grpc_slice_sub(slice, i, j); - EXPECT_EQ(GRPC_SLICE_LENGTH(sub), j - i); + GPR_ASSERT(GRPC_SLICE_LENGTH(sub) == j - i); for (k = 0; k < j - i; k++) { - EXPECT_EQ(GRPC_SLICE_START_PTR(sub)[k], (uint8_t)(i + k)); + GPR_ASSERT(GRPC_SLICE_START_PTR(sub)[k] == (uint8_t)(i + k)); } grpc_slice_unref_internal(sub); } @@ -162,21 +157,20 @@ TEST_P(GrpcSliceSizedTest, SliceSubWorks) { static void check_head_tail(grpc_slice slice, grpc_slice head, grpc_slice 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))); + 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))); } -TEST_P(GrpcSliceSizedTest, SliceSplitHeadWorks) { - const auto length = GetParam(); - +static void test_slice_split_head_works(size_t length) { 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 @@ -199,13 +193,12 @@ TEST_P(GrpcSliceSizedTest, SliceSplitHeadWorks) { grpc_slice_unref_internal(slice); } -TEST_P(GrpcSliceSizedTest, SliceSplitTailWorks) { - const auto length = GetParam(); - +static void test_slice_split_tail_works(size_t length) { 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 @@ -228,37 +221,30 @@ TEST_P(GrpcSliceSizedTest, SliceSplitTailWorks) { grpc_slice_unref_internal(slice); } -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 void test_slice_from_copied_string_works(void) { 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); - EXPECT_EQ(strlen(text), GRPC_SLICE_LENGTH(slice)); - EXPECT_EQ( - 0, memcmp(text, GRPC_SLICE_START_PTR(slice), GRPC_SLICE_LENGTH(slice))); + GPR_ASSERT(strlen(text) == GRPC_SLICE_LENGTH(slice)); + GPR_ASSERT( + 0 == memcmp(text, GRPC_SLICE_START_PTR(slice), GRPC_SLICE_LENGTH(slice))); grpc_slice_unref_internal(slice); } -TEST(GrpcSliceTest, MovedStringSlice) { +static void test_moved_string_slice(void) { + LOG_TEST_NAME("test_moved_string_slice"); + // 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)); - EXPECT_EQ(GRPC_SLICE_LENGTH(small), strlen(kSmallStr)); - EXPECT_NE(GRPC_SLICE_START_PTR(small), reinterpret_cast(small_ptr)); + GPR_ASSERT(GRPC_SLICE_LENGTH(small) == strlen(kSmallStr)); + GPR_ASSERT(GRPC_SLICE_START_PTR(small) != + reinterpret_cast(small_ptr)); grpc_slice_unref_internal(small); // Large string should be move the reference. @@ -266,8 +252,9 @@ TEST(GrpcSliceTest, MovedStringSlice) { char* large_ptr = strdup(kSLargeStr); grpc_slice large = grpc_slice_from_moved_string(grpc_core::UniquePtr(large_ptr)); - EXPECT_EQ(GRPC_SLICE_LENGTH(large), strlen(kSLargeStr)); - EXPECT_EQ(GRPC_SLICE_START_PTR(large), reinterpret_cast(large_ptr)); + GPR_ASSERT(GRPC_SLICE_LENGTH(large) == strlen(kSLargeStr)); + GPR_ASSERT(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 @@ -275,167 +262,32 @@ TEST(GrpcSliceTest, MovedStringSlice) { large_ptr = strdup(kSLargeStr); small = grpc_slice_from_moved_buffer(grpc_core::UniquePtr(large_ptr), strlen(kSmallStr)); - EXPECT_EQ(GRPC_SLICE_LENGTH(small), strlen(kSmallStr)); - EXPECT_NE(GRPC_SLICE_START_PTR(small), reinterpret_cast(large_ptr)); + GPR_ASSERT(GRPC_SLICE_LENGTH(small) == strlen(kSmallStr)); + GPR_ASSERT(GRPC_SLICE_START_PTR(small) != + reinterpret_cast(large_ptr)); grpc_slice_unref_internal(small); } -TEST(GrpcSliceTest, StringViewFromSlice) { +void test_string_view_from_slice() { constexpr char kStr[] = "foo"; absl::string_view sv( grpc_core::StringViewFromSlice(grpc_slice_from_static_string(kStr))); - EXPECT_EQ(sv, kStr); + GPR_ASSERT(std::string(sv) == kStr); } -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])); +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); } - - 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(); + test_slice_from_copied_string_works(); + test_moved_string_slice(); + test_string_view_from_slice(); + return 0; } diff --git a/test/core/transport/BUILD b/test/core/transport/BUILD index 6d54c7c28be..ce79187e2bc 100644 --- a/test/core/transport/BUILD +++ b/test/core/transport/BUILD @@ -156,6 +156,18 @@ 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 30b89df24ef..7327c6db2af 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, const V&) { + void Encode(T, V) { abort(); // not implemented } diff --git a/test/core/transport/parsed_metadata_test.cc b/test/core/transport/parsed_metadata_test.cc index de1195fa285..d0a8c34d5a4 100644 --- a/test/core/transport/parsed_metadata_test.cc +++ b/test/core/transport/parsed_metadata_test.cc @@ -27,25 +27,27 @@ namespace testing { struct CharTrait { using MementoType = char; - static absl::string_view key() { return "key"; } + static const char* 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(Slice slice) { return slice[0]; } + static char ParseMemento(const grpc_slice& slice) { + return *GRPC_SLICE_START_PTR(slice); + } static std::string DisplayValue(char value) { return std::string(1, value); } }; struct Int32Trait { using MementoType = int32_t; - static absl::string_view key() { return "key2"; } + static const char* 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(Slice slice) { + static int32_t ParseMemento(const grpc_slice& slice) { int32_t out; - GPR_ASSERT(absl::SimpleAtoi(slice.as_string_view(), &out)); + GPR_ASSERT(absl::SimpleAtoi(StringViewFromSlice(slice), &out)); return out; } static std::string DisplayValue(int32_t value) { @@ -55,14 +57,14 @@ struct Int32Trait { struct Int64Trait { using MementoType = int64_t; - static absl::string_view key() { return "key3"; } + static const char* 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(Slice slice) { + static int64_t ParseMemento(const grpc_slice& slice) { int64_t out; - GPR_ASSERT(absl::SimpleAtoi(slice.as_string_view(), &out)); + GPR_ASSERT(absl::SimpleAtoi(StringViewFromSlice(slice), &out)); return out; } static std::string DisplayValue(int64_t value) { @@ -72,14 +74,14 @@ struct Int64Trait { struct IntptrTrait { using MementoType = intptr_t; - static absl::string_view key() { return "key4"; } + static const char* 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(Slice slice) { + static intptr_t ParseMemento(const grpc_slice& slice) { intptr_t out; - GPR_ASSERT(absl::SimpleAtoi(slice.as_string_view(), &out)); + GPR_ASSERT(absl::SimpleAtoi(StringViewFromSlice(slice), &out)); return out; } static std::string DisplayValue(intptr_t value) { @@ -89,15 +91,15 @@ struct IntptrTrait { struct StringTrait { using MementoType = std::string; - static absl::string_view key() { return "key5-bin"; } + static const char* 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(Slice slice) { - auto view = slice.as_string_view(); + static std::string ParseMemento(const grpc_slice& slice) { + auto view = StringViewFromSlice(slice); 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 new file mode 100644 index 00000000000..e7b76042f0c --- /dev/null +++ b/test/core/transport/stream_owned_slice_test.cc @@ -0,0 +1,42 @@ +/* + * + * 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 7202058f439..e1fb6ffb9dd 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -81,7 +81,6 @@ grpc_cc_library( ], language = "C++", deps = [ - ":build", ":grpc_suppressions", ":stack_tracer", "//:gpr", @@ -207,9 +206,3 @@ 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 deleted file mode 100644 index 04d39e9574d..00000000000 --- a/test/core/util/build.cc +++ /dev/null @@ -1,77 +0,0 @@ -// 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 deleted file mode 100644 index 9cdab0ced78..00000000000 --- a/test/core/util/build.h +++ /dev/null @@ -1,33 +0,0 @@ -// 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 1ae22f34019..09952c0b349 100644 --- a/test/core/util/test_config.cc +++ b/test/core/util/test_config.cc @@ -37,7 +37,6 @@ #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; @@ -53,6 +52,70 @@ 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 35da4d0f8e8..6ac43de2666 100644 --- a/test/core/util/test_config.h +++ b/test/core/util/test_config.h @@ -21,8 +21,6 @@ #include -#include "test/core/util/build.h" - extern int64_t g_fixture_slowdown_factor; extern int64_t g_poller_slowdown_factor; @@ -39,6 +37,18 @@ 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 c0b098990c9..a1d50f06fd9 100755 --- a/tools/codegen/core/gen_static_metadata.py +++ b/tools/codegen/core/gen_static_metadata.py @@ -177,6 +177,7 @@ 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', @@ -429,13 +430,13 @@ for i, elem in enumerate(all_strs): def slice_def_for_ctx(i): return ( - 'StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' + 'grpc_core::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 ( - 'StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' + 'grpc_core::StaticMetadataSlice(&g_static_metadata_slice_refcounts[%d].base, %d, g_static_metadata_bytes+%d)' ) % (i, len(all_strs[i]), id2strofs[i]) @@ -451,7 +452,7 @@ for elem in METADATA_BATCH_CALLOUTS: static_slice_dest_assert = ( 'static_assert(std::is_trivially_destructible' + '::value, ' - '"StaticMetadataSlice must be trivially destructible.");') + '"grpc_core::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): @@ -474,7 +475,7 @@ extern const uint8_t g_static_metadata_bytes[]; } ''', file=STR_H) -print('grpc_slice_refcount StaticSliceRefcount::kStaticSubRefcount;', +print('grpc_slice_refcount grpc_core::StaticSliceRefcount::kStaticSubRefcount;', file=STR_C) print(''' StaticSliceRefcount @@ -538,7 +539,7 @@ print('', file=STR_H) print('', file=STR_C) print('#define GRPC_STATIC_METADATA_INDEX(static_slice) \\', file=STR_H) print( - '(reinterpret_cast<::grpc_core::StaticSliceRefcount*>((static_slice).refcount)->index)', + '(reinterpret_cast((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 80044680bc1..a732113f407 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2171,7 +2171,6 @@ 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 008872b004d..c704210eaae 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1970,7 +1970,6 @@ 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 d0ac6b7d89f..4ac6a3f5e3b 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2511,6 +2511,30 @@ ], "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, @@ -2671,6 +2695,30 @@ ], "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, @@ -6837,30 +6885,6 @@ ], "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,