From 4423bfd0c68d6244c5fd08bc7dee57750568f2ca Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 30 May 2024 11:31:49 -0700 Subject: [PATCH 1/2] [dual-ref-counted] Add tuning knobs per RefCounted (#36754) Imbue `DualRefCounted` with the same customization arguments as `RefCounted`. Closes #36754 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36754 from ctiller:duality 24e7755287e5f42001e9314d8588bba2aca5e9c5 PiperOrigin-RevId: 638724845 --- src/core/BUILD | 1 + src/core/lib/gprpp/dual_ref_counted.h | 18 +++++++---- test/core/gprpp/dual_ref_counted_test.cc | 38 ++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/core/BUILD b/src/core/BUILD index 480e3f9431b..77b6dccd7a5 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -1236,6 +1236,7 @@ grpc_cc_library( public_hdrs = ["lib/gprpp/dual_ref_counted.h"], deps = [ "down_cast", + "ref_counted", "//:debug_location", "//:gpr", "//:orphanable", diff --git a/src/core/lib/gprpp/dual_ref_counted.h b/src/core/lib/gprpp/dual_ref_counted.h index 5641b0970d4..38365db381b 100644 --- a/src/core/lib/gprpp/dual_ref_counted.h +++ b/src/core/lib/gprpp/dual_ref_counted.h @@ -28,6 +28,7 @@ #include "src/core/lib/gprpp/debug_location.h" #include "src/core/lib/gprpp/down_cast.h" #include "src/core/lib/gprpp/orphanable.h" +#include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" namespace grpc_core { @@ -46,15 +47,16 @@ namespace grpc_core { // // This will be used by CRTP (curiously-recurring template pattern), e.g.: // class MyClass : public RefCounted { ... }; -template -class DualRefCounted { +// +// Impl & UnrefBehavior are as per RefCounted. +template +class DualRefCounted : public Impl { public: // Not copyable nor movable. DualRefCounted(const DualRefCounted&) = delete; DualRefCounted& operator=(const DualRefCounted&) = delete; - virtual ~DualRefCounted() = default; - GRPC_MUST_USE_RESULT RefCountedPtr Ref() { IncrementRefCount(); return RefCountedPtr(static_cast(this)); @@ -215,7 +217,7 @@ class DualRefCounted { CHECK_GT(weak_refs, 0u); #endif if (GPR_UNLIKELY(prev_ref_pair == MakeRefPair(0, 1))) { - delete static_cast(this); + unref_behavior_(static_cast(this)); } } void WeakUnref(const DebugLocation& location, const char* reason) { @@ -242,7 +244,7 @@ class DualRefCounted { (void)reason; #endif if (GPR_UNLIKELY(prev_ref_pair == MakeRefPair(0, 1))) { - delete static_cast(this); + unref_behavior_(static_cast(this)); } } @@ -266,6 +268,9 @@ class DualRefCounted { // Ref count has dropped to zero, so the object is now orphaned. virtual void Orphaned() = 0; + // Note: Depending on the Impl used, this dtor can be implicitly virtual. + ~DualRefCounted() = default; + private: // Allow RefCountedPtr<> to access IncrementRefCount(). template @@ -358,6 +363,7 @@ class DualRefCounted { const char* trace_; #endif std::atomic refs_{0}; + GPR_NO_UNIQUE_ADDRESS UnrefBehavior unref_behavior_; }; } // namespace grpc_core diff --git a/test/core/gprpp/dual_ref_counted_test.cc b/test/core/gprpp/dual_ref_counted_test.cc index f34a3b8949b..1f7cf80e41d 100644 --- a/test/core/gprpp/dual_ref_counted_test.cc +++ b/test/core/gprpp/dual_ref_counted_test.cc @@ -21,6 +21,8 @@ #include "absl/log/check.h" #include "gtest/gtest.h" +#include "src/core/lib/gprpp/manual_constructor.h" +#include "src/core/lib/gprpp/ref_counted.h" #include "test/core/test_util/test_config.h" namespace grpc_core { @@ -115,6 +117,42 @@ TEST(DualRefCountedWithTracing, Basic) { foo->Unref(DEBUG_LOCATION, "original_ref"); } +class FooWithNoDelete final + : public DualRefCounted { + public: + FooWithNoDelete(bool* orphaned_called, bool* destructor_called) + : DualRefCounted( + "FooWithNoDelete"), + orphaned_called_(orphaned_called), + destructor_called_(destructor_called) {} + ~FooWithNoDelete() { *destructor_called_ = true; } + + void Orphaned() override { *orphaned_called_ = true; } + + private: + bool* const orphaned_called_; + bool* const destructor_called_; +}; + +TEST(DualRefCountedWithNoDelete, Basic) { + ManualConstructor foo; + bool destructor_called = false; + bool orphaned_called = false; + foo.Init(&orphaned_called, &destructor_called); + EXPECT_FALSE(orphaned_called); + EXPECT_FALSE(destructor_called); + foo->WeakRef().release(); + EXPECT_FALSE(orphaned_called); + EXPECT_FALSE(destructor_called); + foo->Unref(); + EXPECT_TRUE(orphaned_called); + EXPECT_FALSE(destructor_called); + foo->WeakUnref(); + EXPECT_TRUE(orphaned_called); + EXPECT_TRUE(destructor_called); +} + } // namespace } // namespace testing } // namespace grpc_core From 53c42e9daeb2aba26d4abf159ae5b94574c2d877 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 30 May 2024 13:40:01 -0700 Subject: [PATCH 2/2] [arena] Make arena refcounted (#36758) Make `Arena` be a refcounted object. Solves a bunch of issues: our stack right now needs a very complicated dance between transport and surface to destroy a call, but with this scheme we can just hold a ref to what we need in each place and everything works out. Removes some `ifdef`'d out code that had been sitting dormant for a year or two also -- I'd left it in as a hedge against it being maybe a bad idea, but it looks like it's not needed. Closes #36758 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36758 from ctiller:arena-counting d1b672fe30cd9c3ad4c20a05dbdc1969cb2804ef PiperOrigin-RevId: 638767768 --- CMakeLists.txt | 3 + build_autogenerated.yaml | 7 + src/core/BUILD | 1 + src/core/client_channel/client_channel.cc | 11 +- src/core/client_channel/client_channel.h | 2 - .../subchannel_stream_client.cc | 9 +- .../client_channel/subchannel_stream_client.h | 4 +- .../client/chaotic_good_connector.cc | 2 +- .../client/chaotic_good_connector.h | 5 +- .../chaotic_good/client_transport.cc | 7 +- .../server/chaotic_good_server.cc | 6 +- .../chaotic_good/server/chaotic_good_server.h | 11 +- .../chaotic_good/server_transport.cc | 7 +- .../ext/transport/inproc/inproc_transport.cc | 6 +- src/core/lib/promise/activity.h | 25 +- src/core/lib/resource_quota/arena.cc | 109 +++----- src/core/lib/resource_quota/arena.h | 253 +++--------------- src/core/lib/surface/call.cc | 47 ++-- src/core/lib/surface/channel.cc | 20 +- src/core/lib/surface/channel.h | 11 +- src/core/lib/transport/call_arena_allocator.h | 16 +- src/core/lib/transport/call_spine.cc | 12 +- src/core/lib/transport/call_spine.h | 46 ++-- src/core/lib/transport/interception_chain.cc | 2 +- test/core/call/yodel/yodel_test.h | 5 +- test/core/channel/call_finalization_test.cc | 4 +- .../connected_subchannel_test.cc | 11 +- .../load_balanced_call_destination_test.cc | 6 +- test/core/filters/filter_test.cc | 3 +- test/core/filters/filter_test.h | 9 +- test/core/gprpp/chunked_vector_fuzzer.cc | 2 +- test/core/gprpp/chunked_vector_test.cc | 30 +-- test/core/promise/arena_promise_test.cc | 26 +- test/core/promise/for_each_test.cc | 20 +- test/core/promise/interceptor_list_test.cc | 4 +- test/core/promise/map_pipe_test.cc | 14 +- test/core/promise/pipe_test.cc | 64 ++--- test/core/resource_quota/arena_test.cc | 78 ++---- test/core/security/credentials_test.cc | 4 +- test/core/security/oauth2_utils.cc | 6 +- test/core/surface/channel_init_test.cc | 2 +- test/core/telemetry/call_tracer_test.cc | 21 +- .../transport/binder/binder_transport_test.cc | 23 +- test/core/transport/call_filters_test.cc | 60 ++--- .../client_transport_error_test.cc | 3 +- .../transport/chaotic_good/frame_fuzzer.cc | 4 +- .../core/transport/chaotic_good/frame_test.cc | 2 +- .../transport/chaotic_good/transport_test.h | 7 +- .../transport/chttp2/hpack_encoder_test.cc | 17 -- .../chttp2/hpack_parser_fuzzer_test.cc | 2 +- .../chttp2/hpack_parser_input_size_fuzzer.cc | 4 +- .../transport/chttp2/hpack_parser_test.cc | 4 - .../transport/chttp2/hpack_sync_fuzzer.cc | 4 - .../core/transport/interception_chain_test.cc | 3 +- test/core/transport/metadata_map_test.cc | 28 +- test/cpp/microbenchmarks/bm_arena.cc | 42 +-- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 32 --- 57 files changed, 342 insertions(+), 824 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 28872783538..c30ffda6257 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8535,8 +8535,11 @@ add_executable(call_filters_test src/core/lib/promise/activity.cc src/core/lib/promise/trace.cc src/core/lib/resource_quota/arena.cc + src/core/lib/resource_quota/connection_quota.cc src/core/lib/resource_quota/memory_quota.cc src/core/lib/resource_quota/periodic_update.cc + src/core/lib/resource_quota/resource_quota.cc + src/core/lib/resource_quota/thread_quota.cc src/core/lib/resource_quota/trace.cc src/core/lib/slice/percent_encoding.cc src/core/lib/slice/slice.cc diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index b2d77583ecf..b17bdd226ea 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -6393,6 +6393,7 @@ targets: - src/core/lib/gprpp/atomic_utils.h - src/core/lib/gprpp/bitset.h - src/core/lib/gprpp/chunked_vector.h + - src/core/lib/gprpp/cpp_impl_of.h - src/core/lib/gprpp/down_cast.h - src/core/lib/gprpp/dual_ref_counted.h - src/core/lib/gprpp/dump_args.h @@ -6432,8 +6433,11 @@ targets: - src/core/lib/promise/status_flag.h - src/core/lib/promise/trace.h - src/core/lib/resource_quota/arena.h + - src/core/lib/resource_quota/connection_quota.h - src/core/lib/resource_quota/memory_quota.h - src/core/lib/resource_quota/periodic_update.h + - 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/percent_encoding.h - src/core/lib/slice/slice.h @@ -6499,8 +6503,11 @@ targets: - src/core/lib/promise/activity.cc - src/core/lib/promise/trace.cc - src/core/lib/resource_quota/arena.cc + - src/core/lib/resource_quota/connection_quota.cc - src/core/lib/resource_quota/memory_quota.cc - src/core/lib/resource_quota/periodic_update.cc + - src/core/lib/resource_quota/resource_quota.cc + - src/core/lib/resource_quota/thread_quota.cc - src/core/lib/resource_quota/trace.cc - src/core/lib/slice/percent_encoding.cc - src/core/lib/slice/slice.cc diff --git a/src/core/BUILD b/src/core/BUILD index 77b6dccd7a5..99ce258ea65 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -1489,6 +1489,7 @@ grpc_cc_library( "context", "event_engine_memory_allocator", "memory_quota", + "resource_quota", "//:gpr", ], ) diff --git a/src/core/client_channel/client_channel.cc b/src/core/client_channel/client_channel.cc index 77de1ab121d..5395efce7b9 100644 --- a/src/core/client_channel/client_channel.cc +++ b/src/core/client_channel/client_channel.cc @@ -653,11 +653,6 @@ ClientChannel::ClientChannel( default_authority_( GetDefaultAuthorityFromChannelArgs(channel_args_, this->target())), channelz_node_(channel_args_.GetObject()), - call_arena_allocator_(MakeRefCounted( - channel_args_.GetObject() - ->memory_quota() - ->CreateMemoryAllocator("client_channel"), - 1024)), idle_timeout_(GetClientIdleTimeout(channel_args_)), resolver_data_for_calls_(ResolverDataForCalls{}), picker_(nullptr), @@ -825,9 +820,9 @@ CallInitiator ClientChannel::CreateCall( // Exit IDLE if needed. CheckConnectivityState(/*try_to_connect=*/true); // Create an initiator/unstarted-handler pair. - auto call = MakeCallPair( - std::move(client_initial_metadata), event_engine_.get(), - call_arena_allocator_->MakeArena(), call_arena_allocator_, nullptr); + auto call = + MakeCallPair(std::move(client_initial_metadata), event_engine_.get(), + call_arena_allocator()->MakeArena(), nullptr); // Spawn a promise to wait for the resolver result. // This will eventually start the call. call.initiator.SpawnGuardedUntilCallCompletes( diff --git a/src/core/client_channel/client_channel.h b/src/core/client_channel/client_channel.h index 77d26f34e95..abe53bbf689 100644 --- a/src/core/client_channel/client_channel.h +++ b/src/core/client_channel/client_channel.h @@ -179,8 +179,6 @@ class ClientChannel : public Channel { ClientChannelFactory* const client_channel_factory_; const std::string default_authority_; channelz::ChannelNode* const channelz_node_; - // TODO(ctiller): unify with Channel - const RefCountedPtr call_arena_allocator_; GlobalStatsPluginRegistry::StatsPluginGroup stats_plugin_group_; // diff --git a/src/core/client_channel/subchannel_stream_client.cc b/src/core/client_channel/subchannel_stream_client.cc index 2ee082ef1fa..ab3118d4442 100644 --- a/src/core/client_channel/subchannel_stream_client.cc +++ b/src/core/client_channel/subchannel_stream_client.cc @@ -59,12 +59,13 @@ SubchannelStreamClient::SubchannelStreamClient( connected_subchannel_(std::move(connected_subchannel)), interested_parties_(interested_parties), tracer_(tracer), - call_allocator_( + call_allocator_(MakeRefCounted( connected_subchannel_->args() .GetObject() ->memory_quota() ->CreateMemoryAllocator( - (tracer != nullptr) ? tracer : "SubchannelStreamClient")), + (tracer != nullptr) ? tracer : "SubchannelStreamClient"), + 1024)), event_handler_(std::move(event_handler)), retry_backoff_( BackOff::Options() @@ -171,9 +172,7 @@ SubchannelStreamClient::CallState::CallState( grpc_pollset_set* interested_parties) : subchannel_stream_client_(std::move(health_check_client)), pollent_(grpc_polling_entity_create_from_pollset_set(interested_parties)), - arena_(Arena::Create(subchannel_stream_client_->connected_subchannel_ - ->GetInitialCallSizeEstimate(), - &subchannel_stream_client_->call_allocator_)), + arena_(subchannel_stream_client_->call_allocator_->MakeArena()), payload_(context_) {} SubchannelStreamClient::CallState::~CallState() { diff --git a/src/core/client_channel/subchannel_stream_client.h b/src/core/client_channel/subchannel_stream_client.h index 38d25c755d3..d84f4b3cc1f 100644 --- a/src/core/client_channel/subchannel_stream_client.h +++ b/src/core/client_channel/subchannel_stream_client.h @@ -146,7 +146,7 @@ class SubchannelStreamClient final RefCountedPtr subchannel_stream_client_; grpc_polling_entity pollent_; - ScopedArenaPtr arena_; + RefCountedPtr arena_; CallCombiner call_combiner_; grpc_call_context_element context_[GRPC_CONTEXT_COUNT] = {}; @@ -201,7 +201,7 @@ class SubchannelStreamClient final RefCountedPtr connected_subchannel_; grpc_pollset_set* interested_parties_; // Do not own. const char* tracer_; - MemoryAllocator call_allocator_; + RefCountedPtr call_allocator_; Mutex mu_; std::unique_ptr event_handler_ ABSL_GUARDED_BY(mu_); diff --git a/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.cc b/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.cc index d22d3bbb02c..a5cf2893951 100644 --- a/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.cc +++ b/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.cc @@ -333,7 +333,7 @@ void ChaoticGoodConnector::OnHandshakeDone(void* arg, grpc_error_handle error) { status); } }, - self->arena_.get(), self->event_engine_.get()); + self->arena_, self->event_engine_.get()); MutexLock lock(&self->mu_); if (!self->is_shutdown_) { self->connect_activity_ = std::move(activity); diff --git a/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.h b/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.h index 90b4abff267..b8db7a52502 100644 --- a/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.h +++ b/src/core/ext/transport/chaotic_good/client/chaotic_good_connector.h @@ -79,10 +79,7 @@ class ChaoticGoodConnector : public SubchannelConnector { RefCountedPtr self); static void OnHandshakeDone(void* arg, grpc_error_handle error); - grpc_event_engine::experimental::MemoryAllocator memory_allocator_ = - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "connect_activity"); - ScopedArenaPtr arena_ = MakeScopedArena(1024, &memory_allocator_); + RefCountedPtr arena_ = SimpleArenaAllocator()->MakeArena(); Mutex mu_; Args args_; Result* result_ ABSL_GUARDED_BY(mu_); diff --git a/src/core/ext/transport/chaotic_good/client_transport.cc b/src/core/ext/transport/chaotic_good/client_transport.cc index cc1fd8b9a18..511d04f6ed1 100644 --- a/src/core/ext/transport/chaotic_good/client_transport.cc +++ b/src/core/ext/transport/chaotic_good/client_transport.cc @@ -146,10 +146,9 @@ auto ChaoticGoodClientTransport::TransportReadLoop( frame_limits); } else { // Stream not found, skip the frame. - auto arena = MakeScopedArena(1024, &allocator_); - deserialize_status = - transport->DeserializeFrame(frame_header, std::move(buffers), - arena.get(), frame, frame_limits); + deserialize_status = transport->DeserializeFrame( + frame_header, std::move(buffers), + SimpleArenaAllocator()->MakeArena().get(), frame, frame_limits); } return If( deserialize_status.ok() && call_handler.has_value(), diff --git a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc index 8af652a292c..524d9e68349 100644 --- a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc +++ b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.cc @@ -149,8 +149,7 @@ absl::Status ChaoticGoodServerListener::StartListening() { ChaoticGoodServerListener::ActiveConnection::ActiveConnection( RefCountedPtr listener, std::unique_ptr endpoint) - : memory_allocator_(listener->memory_allocator_), - listener_(std::move(listener)) { + : listener_(std::move(listener)) { handshaking_state_ = MakeRefCounted(Ref()); handshaking_state_->Start(std::move(endpoint)); } @@ -208,8 +207,7 @@ void ChaoticGoodServerListener::ActiveConnection::Done( ChaoticGoodServerListener::ActiveConnection::HandshakingState::HandshakingState( RefCountedPtr connection) - : memory_allocator_(connection->memory_allocator_), - connection_(std::move(connection)), + : connection_(std::move(connection)), handshake_mgr_(MakeRefCounted()) {} void ChaoticGoodServerListener::ActiveConnection::HandshakingState::Start( diff --git a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h index 0479016c15b..8511790c03b 100644 --- a/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h +++ b/src/core/ext/transport/chaotic_good/server/chaotic_good_server.h @@ -106,8 +106,6 @@ class ChaoticGoodServerListener final : public Server::ListenerInterface { static void OnHandshakeDone(void* arg, grpc_error_handle error); Timestamp GetConnectionDeadline(); - const std::shared_ptr - memory_allocator_; const RefCountedPtr connection_; const RefCountedPtr handshake_mgr_; }; @@ -115,9 +113,7 @@ class ChaoticGoodServerListener final : public Server::ListenerInterface { private: void Done(absl::optional error = absl::nullopt); void NewConnectionID(); - const std::shared_ptr - memory_allocator_; - ScopedArenaPtr arena_ = MakeScopedArena(1024, memory_allocator_.get()); + RefCountedPtr arena_ = SimpleArenaAllocator()->MakeArena(); const RefCountedPtr listener_; RefCountedPtr handshaking_state_; Mutex mu_; @@ -161,11 +157,6 @@ class ChaoticGoodServerListener final : public Server::ListenerInterface { absl::AnyInvocable connection_id_generator_ ABSL_GUARDED_BY(mu_); grpc_closure* on_destroy_done_ ABSL_GUARDED_BY(mu_) = nullptr; - std::shared_ptr - memory_allocator_ = - std::make_shared( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "server_connection")); }; } // namespace chaotic_good diff --git a/src/core/ext/transport/chaotic_good/server_transport.cc b/src/core/ext/transport/chaotic_good/server_transport.cc index 98fbf4e915e..bc4fbed3334 100644 --- a/src/core/ext/transport/chaotic_good/server_transport.cc +++ b/src/core/ext/transport/chaotic_good/server_transport.cc @@ -235,15 +235,14 @@ auto ChaoticGoodServerTransport::DeserializeAndPushFragmentToNewCall( FrameHeader frame_header, BufferPair buffers, ChaoticGoodTransport& transport) { ClientFragmentFrame fragment_frame; - ScopedArenaPtr arena(call_arena_allocator_->MakeArena()); + RefCountedPtr arena(call_arena_allocator_->MakeArena()); absl::Status status = transport.DeserializeFrame( frame_header, std::move(buffers), arena.get(), fragment_frame, FrameLimits{1024 * 1024 * 1024, aligned_bytes_ - 1}); absl::optional call_initiator; if (status.ok()) { - auto call = - MakeCallPair(std::move(fragment_frame.headers), event_engine_.get(), - arena.release(), call_arena_allocator_, nullptr); + auto call = MakeCallPair(std::move(fragment_frame.headers), + event_engine_.get(), std::move(arena), nullptr); call_initiator.emplace(std::move(call.initiator)); auto add_result = NewStream(frame_header.stream_id, *call_initiator); if (add_result.ok()) { diff --git a/src/core/ext/transport/inproc/inproc_transport.cc b/src/core/ext/transport/inproc/inproc_transport.cc index 7913e631fe8..c5ea01739f4 100644 --- a/src/core/ext/transport/inproc/inproc_transport.cc +++ b/src/core/ext/transport/inproc/inproc_transport.cc @@ -105,9 +105,9 @@ class InprocServerTransport final : public ServerTransport { case ConnectionState::kReady: break; } - auto* arena = call_arena_allocator_->MakeArena(); - auto server_call = MakeCallPair(std::move(md), event_engine_.get(), arena, - call_arena_allocator_, nullptr); + auto server_call = + MakeCallPair(std::move(md), event_engine_.get(), + call_arena_allocator_->MakeArena(), nullptr); unstarted_call_handler_->StartCall(std::move(server_call.handler)); return std::move(server_call.initiator); } diff --git a/src/core/lib/promise/activity.h b/src/core/lib/promise/activity.h index 1cbbc01e268..715de5317f2 100644 --- a/src/core/lib/promise/activity.h +++ b/src/core/lib/promise/activity.h @@ -289,6 +289,19 @@ class ContextHolder> { std::unique_ptr value_; }; +template +class ContextHolder> { + public: + using ContextType = Context; + + explicit ContextHolder(RefCountedPtr value) + : value_(std::move(value)) {} + Context* GetContext() { return value_.get(); } + + private: + RefCountedPtr value_; +}; + template <> class Context { public: @@ -296,19 +309,23 @@ class Context { }; template -using ContextTypeFromHeld = typename ContextHolder::ContextType; +using ContextTypeFromHeld = typename ContextHolder< + typename std::remove_reference::type>::ContextType; template -class ActivityContexts : public ContextHolder... { +class ActivityContexts + : public ContextHolder::type>... { public: explicit ActivityContexts(Contexts&&... contexts) - : ContextHolder(std::forward(contexts))... {} + : ContextHolder::type>( + std::forward(contexts))... {} class ScopedContext : public Context>... { public: explicit ScopedContext(ActivityContexts* contexts) : Context>( - static_cast*>(contexts) + static_cast::type>*>(contexts) ->GetContext())... { // Silence `unused-but-set-parameter` in case of Contexts = {} (void)contexts; diff --git a/src/core/lib/resource_quota/arena.cc b/src/core/lib/resource_quota/arena.cc index a2969abe241..2f9e3c851e5 100644 --- a/src/core/lib/resource_quota/arena.cc +++ b/src/core/lib/resource_quota/arena.cc @@ -24,6 +24,7 @@ #include #include +#include "src/core/lib/resource_quota/resource_quota.h" #include "src/core/util/alloc.h" namespace { @@ -46,6 +47,10 @@ void* ArenaStorage(size_t initial_size) { namespace grpc_core { Arena::~Arena() { + DestroyManagedNewObjects(); + arena_factory_->FinalizeArena(this); + arena_factory_->allocator().Release( + total_allocated_.load(std::memory_order_relaxed)); Zone* z = last_zone_; while (z) { Zone* prev_z = z->prev; @@ -58,19 +63,19 @@ Arena::~Arena() { #endif } -Arena* Arena::Create(size_t initial_size, MemoryAllocator* memory_allocator) { - return new (ArenaStorage(initial_size)) - Arena(initial_size, 0, memory_allocator); +RefCountedPtr Arena::Create(size_t initial_size, + RefCountedPtr arena_factory) { + return RefCountedPtr(new (ArenaStorage(initial_size)) Arena( + initial_size, 0, std::move(arena_factory))); } -std::pair Arena::CreateWithAlloc( - size_t initial_size, size_t alloc_size, MemoryAllocator* memory_allocator) { - static constexpr size_t base_size = - GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(Arena)); - auto* new_arena = new (ArenaStorage(initial_size)) - Arena(initial_size, alloc_size, memory_allocator); - void* first_alloc = reinterpret_cast(new_arena) + base_size; - return std::make_pair(new_arena, first_alloc); +Arena::Arena(size_t initial_size, size_t initial_alloc, + RefCountedPtr arena_factory) + : total_used_(GPR_ROUND_UP_TO_ALIGNMENT_SIZE(initial_alloc)), + initial_zone_size_(initial_size), + arena_factory_(std::move(arena_factory)) { + arena_factory_->allocator().Reserve( + GPR_ROUND_UP_TO_ALIGNMENT_SIZE(initial_alloc)); } void Arena::DestroyManagedNewObjects() { @@ -86,11 +91,9 @@ void Arena::DestroyManagedNewObjects() { } } -void Arena::Destroy() { - DestroyManagedNewObjects(); - memory_allocator_->Release(total_allocated_.load(std::memory_order_relaxed)); +void Arena::Destroy() const { this->~Arena(); - gpr_free_aligned(this); + gpr_free_aligned(const_cast(this)); } void* Arena::AllocZone(size_t size) { @@ -102,7 +105,7 @@ void* Arena::AllocZone(size_t size) { static constexpr size_t zone_base_size = GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(Zone)); size_t alloc_size = zone_base_size + size; - memory_allocator_->Reserve(alloc_size); + arena_factory_->allocator().Reserve(alloc_size); total_allocated_.fetch_add(alloc_size, std::memory_order_relaxed); Zone* z = new (gpr_malloc_aligned(alloc_size, GPR_MAX_ALIGNMENT)) Zone(); auto* prev = last_zone_.load(std::memory_order_relaxed); @@ -120,63 +123,27 @@ void Arena::ManagedNewObject::Link(std::atomic* head) { } } -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC -void* Arena::AllocPooled(size_t obj_size, size_t alloc_size, - std::atomic* head) { - // ABA mitigation: - // AllocPooled may be called by multiple threads, and to remove a node from - // the free list we need to manipulate the next pointer, which may be done - // differently by each thread in a naive implementation. - // The literature contains various ways of dealing with this. Here we expect - // to be mostly single threaded - Arena's are owned by calls and calls don't - // do a lot of concurrent work with the pooled allocator. The place that they - // do is allocating metadata batches for decoding HPACK headers in chttp2. - // So we adopt an approach that is simple and fast for the single threaded - // case, and that is also correct in the multi threaded case. - - // First, take ownership of the entire free list. At this point we know that - // no other thread can see free nodes and will be forced to allocate. - // We think we're mostly single threaded and so that's ok. - FreePoolNode* p = head->exchange(nullptr, std::memory_order_acquire); - // If there are no nodes in the free list, then go ahead and allocate from the - // arena. - if (p == nullptr) { - void* r = Alloc(alloc_size); - TracePoolAlloc(obj_size, r); - return r; - } - // We had a non-empty free list... but we own the *entire* free list. - // We only want one node, so if there are extras we'd better give them back. - if (p->next != nullptr) { - // We perform an exchange to do so, but if there were concurrent frees with - // this allocation then there'll be a free list that needs to be merged with - // ours. - FreePoolNode* extra = head->exchange(p->next, std::memory_order_acq_rel); - // If there was a free list concurrently created, we merge it into the - // overall free list here by simply freeing each node in turn. This is O(n), - // but only O(n) in the number of nodes that were freed concurrently, and - // again: we think real world use cases are going to see this as mostly - // single threaded. - while (extra != nullptr) { - FreePoolNode* next = extra->next; - FreePooled(extra, head); - extra = next; +RefCountedPtr SimpleArenaAllocator(size_t initial_size) { + class Allocator : public ArenaFactory { + public: + explicit Allocator(size_t initial_size) + : ArenaFactory( + ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( + "simple-arena-allocator")), + initial_size_(initial_size) {} + + RefCountedPtr MakeArena() override { + return Arena::Create(initial_size_, Ref()); } - } - TracePoolAlloc(obj_size, p); - return p; -} -void Arena::FreePooled(void* p, std::atomic* head) { - // May spuriously trace a free of an already freed object - see AllocPooled - // ABA mitigation. - TracePoolFree(p); - FreePoolNode* node = static_cast(p); - node->next = head->load(std::memory_order_acquire); - while (!head->compare_exchange_weak( - node->next, node, std::memory_order_acq_rel, std::memory_order_relaxed)) { - } + void FinalizeArena(Arena*) override { + // No-op. + } + + private: + size_t initial_size_; + }; + return MakeRefCounted(initial_size); } -#endif } // namespace grpc_core diff --git a/src/core/lib/resource_quota/arena.h b/src/core/lib/resource_quota/arena.h index 813004087b7..61fc917badf 100644 --- a/src/core/lib/resource_quota/arena.h +++ b/src/core/lib/resource_quota/arena.h @@ -40,76 +40,12 @@ #include "src/core/lib/resource_quota/memory_quota.h" #include "src/core/util/alloc.h" -#define GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC -// #define GRPC_ARENA_TRACE_POOLED_ALLOCATIONS - namespace grpc_core { -namespace arena_detail { - -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC -struct PoolAndSize { - size_t alloc_size; - size_t pool_index; -}; - -template -struct PoolIndexForSize; - -template -struct PoolIndexForSize< - absl::enable_if_t, kIndex, - kObjectSize, kSmallestRemainingBucket, kBucketSizes...> { - static constexpr size_t kPool = kIndex; - static constexpr size_t kSize = kSmallestRemainingBucket; -}; - -template -struct PoolIndexForSize< - absl::enable_if_t<(kObjectSize > kSmallestRemainingBucket)>, kIndex, - kObjectSize, kSmallestRemainingBucket, kBucketSizes...> - : public PoolIndexForSize { -}; - -template -constexpr size_t PoolFromObjectSize( - absl::integer_sequence) { - return PoolIndexForSize::kPool; -} - -template -constexpr size_t AllocationSizeFromObjectSize( - absl::integer_sequence) { - return PoolIndexForSize::kSize; -} - -template -struct ChoosePoolForAllocationSizeImpl; - -template -struct ChoosePoolForAllocationSizeImpl { - static PoolAndSize Fn(size_t n) { - if (n <= kBucketSize) return {kBucketSize, kIndex}; - return ChoosePoolForAllocationSizeImpl::Fn(n); - } -}; +class Arena; -template -struct ChoosePoolForAllocationSizeImpl { - static PoolAndSize Fn(size_t n) { - return PoolAndSize{n, std::numeric_limits::max()}; - } -}; +namespace arena_detail { -template -PoolAndSize ChoosePoolForAllocationSize( - size_t n, absl::integer_sequence) { - return ChoosePoolForAllocationSizeImpl<0, kBucketSizes...>::Fn(n); -} -#else template struct IfArray { using Result = A; @@ -119,30 +55,36 @@ template struct IfArray { using Result = B; }; -#endif + +struct UnrefDestroy { + void operator()(const Arena* arena) const; +}; } // namespace arena_detail -class Arena { -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC - // Selected pool sizes. - // How to tune: see tools/codegen/core/optimize_arena_pool_sizes.py - using PoolSizes = absl::integer_sequence; - struct FreePoolNode { - FreePoolNode* next; - }; -#endif +class ArenaFactory : public RefCounted { + public: + virtual RefCountedPtr MakeArena() = 0; + virtual void FinalizeArena(Arena* arena) = 0; + MemoryAllocator& allocator() { return allocator_; } + + protected: + explicit ArenaFactory(MemoryAllocator allocator) + : allocator_(std::move(allocator)) {} + + private: + MemoryAllocator allocator_; +}; + +RefCountedPtr SimpleArenaAllocator(size_t initial_size = 1024); + +class Arena final : public RefCounted { public: // Create an arena, with \a initial_size bytes in the first allocated buffer. - static Arena* Create(size_t initial_size, MemoryAllocator* memory_allocator); - - // Create an arena, with \a initial_size bytes in the first allocated buffer, - // and return both a void pointer to the returned arena and a void* with the - // first allocation. - static std::pair CreateWithAlloc( - size_t initial_size, size_t alloc_size, - MemoryAllocator* memory_allocator); + static RefCountedPtr Create(size_t initial_size, + RefCountedPtr arena_factory); // Destroy all `ManagedNew` allocated objects. // Allows safe destruction of these objects even if they need context held by @@ -151,9 +93,6 @@ class Arena { // TODO(ctiller): eliminate ManagedNew. void DestroyManagedNewObjects(); - // Destroy an arena. - void Destroy(); - // Return the total amount of memory allocated by this arena. size_t TotalUsedBytes() const { return total_used_.load(std::memory_order_relaxed); @@ -194,95 +133,6 @@ class Arena { return &p->t; } -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC - class PooledDeleter { - public: - explicit PooledDeleter(std::atomic* free_list) - : free_list_(free_list) {} - PooledDeleter() = default; - template - void operator()(T* p) { - // TODO(ctiller): promise based filter hijacks ownership of some pointers - // to make them appear as PoolPtr without really transferring ownership, - // by setting the arena to nullptr. - // This is a transitional hack and should be removed once promise based - // filter is removed. - if (free_list_ != nullptr) { - p->~T(); - FreePooled(p, free_list_); - } - } - - bool has_freelist() const { return free_list_ != nullptr; } - - private: - std::atomic* free_list_; - }; - - template - using PoolPtr = std::unique_ptr; - - // Make a unique_ptr to T that is allocated from the arena. - // When the pointer is released, the memory may be reused for other - // MakePooled(.*) calls. - // CAUTION: The amount of memory allocated is rounded up to the nearest - // value in Arena::PoolSizes, and so this may pessimize total - // arena size. - template - PoolPtr MakePooled(Args&&... args) { - auto* free_list = - &pools_[arena_detail::PoolFromObjectSize(PoolSizes())]; - return PoolPtr( - new (AllocPooled( - sizeof(T), - arena_detail::AllocationSizeFromObjectSize(PoolSizes()), - free_list)) T(std::forward(args)...), - PooledDeleter(free_list)); - } - - // Make a unique_ptr to an array of T that is allocated from the arena. - // When the pointer is released, the memory may be reused for other - // MakePooled(.*) calls. - // One can use MakePooledArray to allocate a buffer of bytes. - // CAUTION: The amount of memory allocated is rounded up to the nearest - // value in Arena::PoolSizes, and so this may pessimize total - // arena size. - template - PoolPtr MakePooledArray(size_t n) { - auto where = - arena_detail::ChoosePoolForAllocationSize(n * sizeof(T), PoolSizes()); - if (where.pool_index == std::numeric_limits::max()) { - return PoolPtr(new (Alloc(where.alloc_size)) T[n], - PooledDeleter(nullptr)); - } else { - return PoolPtr(new (AllocPooled(where.alloc_size, where.alloc_size, - &pools_[where.pool_index])) T[n], - PooledDeleter(&pools_[where.pool_index])); - } - } - - // Like MakePooled, but with manual memory management. - // The caller is responsible for calling DeletePooled() on the returned - // pointer, and expected to call it with the same type T as was passed to this - // function (else the free list returned to the arena will be corrupted). - template - T* NewPooled(Args&&... args) { - auto* free_list = - &pools_[arena_detail::PoolFromObjectSize(PoolSizes())]; - return new (AllocPooled( - sizeof(T), - arena_detail::AllocationSizeFromObjectSize(PoolSizes()), - free_list)) T(std::forward(args)...); - } - - template - void DeletePooled(T* p) { - auto* free_list = - &pools_[arena_detail::PoolFromObjectSize(PoolSizes())]; - p->~T(); - FreePooled(p, free_list); - } -#else class PooledDeleter { public: PooledDeleter() = default; @@ -364,9 +214,10 @@ class Arena { void DeletePooled(T* p) { delete p; } -#endif private: + friend struct arena_detail::UnrefDestroy; + struct Zone { Zone* prev; }; @@ -397,34 +248,12 @@ class Arena { // where we wish to create an arena and then perform an immediate // allocation. explicit Arena(size_t initial_size, size_t initial_alloc, - MemoryAllocator* memory_allocator) - : total_used_(GPR_ROUND_UP_TO_ALIGNMENT_SIZE(initial_alloc)), - initial_zone_size_(initial_size), - memory_allocator_(memory_allocator) {} + RefCountedPtr arena_factory); ~Arena(); void* AllocZone(size_t size); - -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC - void* AllocPooled(size_t obj_size, size_t alloc_size, - std::atomic* head); - static void FreePooled(void* p, std::atomic* head); -#endif - - void TracePoolAlloc(size_t size, void* ptr) { - (void)size; - (void)ptr; -#ifdef GRPC_ARENA_TRACE_POOLED_ALLOCATIONS - gpr_log(GPR_ERROR, "ARENA %p ALLOC %" PRIdPTR " @ %p", this, size, ptr); -#endif - } - static void TracePoolFree(void* ptr) { - (void)ptr; -#ifdef GRPC_ARENA_TRACE_POOLED_ALLOCATIONS - gpr_log(GPR_ERROR, "FREE %p", ptr); -#endif - } + void Destroy() const; // Keep track of the total used size. We use this in our call sizing // hysteresis. @@ -438,27 +267,19 @@ class Arena { // last zone; the zone list is reverse-walked during arena destruction only. std::atomic last_zone_{nullptr}; std::atomic managed_new_head_{nullptr}; -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC - std::atomic pools_[PoolSizes::size()]{}; -#endif - // The backing memory quota - MemoryAllocator* const memory_allocator_; + RefCountedPtr arena_factory_; }; -// Smart pointer for arenas when the final size is not required. -struct ScopedArenaDeleter { - void operator()(Arena* arena) { arena->Destroy(); } -}; -using ScopedArenaPtr = std::unique_ptr; -inline ScopedArenaPtr MakeScopedArena(size_t initial_size, - MemoryAllocator* memory_allocator) { - return ScopedArenaPtr(Arena::Create(initial_size, memory_allocator)); -} - // Arenas form a context for activities template <> struct ContextType {}; +namespace arena_detail { +inline void UnrefDestroy::operator()(const Arena* arena) const { + arena->Destroy(); +} +} // namespace arena_detail + } // namespace grpc_core #endif // GRPC_SRC_CORE_LIB_RESOURCE_QUOTA_ARENA_H diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 9a54226862b..5f0d4a99310 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -380,15 +380,15 @@ void Call::Run() { class ChannelBasedCall : public Call { protected: - ChannelBasedCall(Arena* arena, bool is_client, Timestamp send_deadline, - RefCountedPtr channel) + ChannelBasedCall(RefCountedPtr arena, bool is_client, + Timestamp send_deadline, RefCountedPtr channel) : Call(is_client, send_deadline, channel->event_engine()), - arena_(arena), + arena_(std::move(arena)), channel_(std::move(channel)) { - DCHECK_NE(arena_, nullptr); + DCHECK_NE(arena_.get(), nullptr); } - Arena* arena() final { return arena_; } + Arena* arena() final { return arena_.get(); } char* GetPeer() final { Slice peer_slice = GetPeerString(); @@ -415,18 +415,17 @@ class ChannelBasedCall : public Call { void DeleteThis() { RefCountedPtr channel = std::move(channel_); - Arena* arena = arena_; + RefCountedPtr arena = arena_; this->~ChannelBasedCall(); - channel->DestroyArena(arena); } Channel* channel() const { return channel_.get(); } // Non-virtual arena accessor -- needed by PipeBasedCall - Arena* GetArena() { return arena_; } + Arena* GetArena() { return arena_.get(); } private: - Arena* const arena_; + const RefCountedPtr arena_; RefCountedPtr channel_; }; @@ -597,8 +596,9 @@ class FilterStackCall final : public ChannelBasedCall { void FinishBatch(grpc_error_handle error); }; - FilterStackCall(Arena* arena, const grpc_call_create_args& args) - : ChannelBasedCall(arena, args.server_transport_data == nullptr, + FilterStackCall(RefCountedPtr arena, const grpc_call_create_args& args) + : ChannelBasedCall(std::move(arena), + args.server_transport_data == nullptr, args.send_deadline, args.channel->Ref()), cq_(args.cq), stream_op_payload_(context_) { @@ -732,7 +732,7 @@ grpc_error_handle FilterStackCall::Create(grpc_call_create_args* args, GPR_ROUND_UP_TO_ALIGNMENT_SIZE(sizeof(FilterStackCall)) + channel_stack->call_stack_size; - Arena* arena = channel->CreateArena(); + RefCountedPtr arena = channel->call_arena_allocator()->MakeArena(); call = new (arena->Alloc(call_alloc_size)) FilterStackCall(arena, *args); DCHECK(FromC(call->c_ptr()) == call); DCHECK(FromCallStack(call->call_stack()) == call); @@ -771,7 +771,7 @@ grpc_error_handle FilterStackCall::Create(grpc_call_create_args* args, args->server->server_call_tracer_factory() != nullptr) { auto* server_call_tracer = args->server->server_call_tracer_factory()->CreateNewServerCallTracer( - arena, args->server->channel_args()); + arena.get(), args->server->channel_args()); if (server_call_tracer != nullptr) { // Note that we are setting both // GRPC_CONTEXT_CALL_TRACER_ANNOTATION_INTERFACE and @@ -1906,10 +1906,12 @@ class BasicPromiseBasedCall : public ChannelBasedCall, public Party { public: using Call::arena; - BasicPromiseBasedCall(Arena* arena, uint32_t initial_external_refs, + BasicPromiseBasedCall(RefCountedPtr arena, + uint32_t initial_external_refs, uint32_t initial_internal_refs, const grpc_call_create_args& args) - : ChannelBasedCall(arena, args.server_transport_data == nullptr, + : ChannelBasedCall(std::move(arena), + args.server_transport_data == nullptr, args.send_deadline, args.channel->Ref()), Party(initial_internal_refs), external_refs_(initial_external_refs), @@ -2067,7 +2069,7 @@ class BasicPromiseBasedCall : public ChannelBasedCall, public Party { class PromiseBasedCall : public BasicPromiseBasedCall { public: - PromiseBasedCall(Arena* arena, uint32_t initial_external_refs, + PromiseBasedCall(RefCountedPtr arena, uint32_t initial_external_refs, const grpc_call_create_args& args); bool Completed() final { return finished_.IsSet(); } @@ -2348,17 +2350,17 @@ template grpc_error_handle MakePromiseBasedCall(grpc_call_create_args* args, grpc_call** out_call) { Channel* channel = args->channel.get(); - - auto* arena = channel->CreateArena(); + auto arena = channel->call_arena_allocator()->MakeArena(); PromiseBasedCall* call = arena->New(arena, args); *out_call = call->c_ptr(); DCHECK(Call::FromC(*out_call) == call); return absl::OkStatus(); } -PromiseBasedCall::PromiseBasedCall(Arena* arena, uint32_t initial_external_refs, +PromiseBasedCall::PromiseBasedCall(RefCountedPtr arena, + uint32_t initial_external_refs, const grpc_call_create_args& args) - : BasicPromiseBasedCall(arena, initial_external_refs, + : BasicPromiseBasedCall(std::move(arena), initial_external_refs, initial_external_refs != 0 ? 1 : 0, args) {} static void CToMetadata(grpc_metadata* metadata, size_t count, @@ -2591,8 +2593,9 @@ void PublishMetadataArray(grpc_metadata_batch* md, grpc_metadata_array* array, #ifdef GRPC_EXPERIMENT_IS_INCLUDED_PROMISE_BASED_CLIENT_CALL class ClientPromiseBasedCall final : public PromiseBasedCall { public: - ClientPromiseBasedCall(Arena* arena, grpc_call_create_args* args) - : PromiseBasedCall(arena, 1, *args), + ClientPromiseBasedCall(RefCountedPtr arena, + grpc_call_create_args* args) + : PromiseBasedCall(std::move(arena), 1, *args), polling_entity_( args->cq != nullptr ? grpc_polling_entity_create_from_pollset( diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index b7f91e6003c..3d2deba0698 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -66,21 +66,11 @@ Channel::Channel(std::string target, const ChannelArgs& channel_args) : target_(std::move(target)), channelz_node_(channel_args.GetObjectRef()), compression_options_(CompressionOptionsFromChannelArgs(channel_args)), - call_size_estimator_(1024), - allocator_(channel_args.GetObject() - ->memory_quota() - ->CreateMemoryOwner()) {} - -Arena* Channel::CreateArena() { - const size_t initial_size = call_size_estimator_.CallSizeEstimate(); - global_stats().IncrementCallInitialSize(initial_size); - return Arena::Create(initial_size, &allocator_); -} - -void Channel::DestroyArena(Arena* arena) { - call_size_estimator_.UpdateCallSizeEstimate(arena->TotalUsedBytes()); - arena->Destroy(); -} + call_arena_allocator_(MakeRefCounted( + channel_args.GetObject() + ->memory_quota() + ->CreateMemoryOwner(), + 1024)) {} Channel::RegisteredCall* Channel::RegisterCall(const char* method, const char* host) { diff --git a/src/core/lib/surface/channel.h b/src/core/lib/surface/channel.h index d0547c5bdf5..9d207ae5f2c 100644 --- a/src/core/lib/surface/channel.h +++ b/src/core/lib/surface/channel.h @@ -134,12 +134,14 @@ class Channel : public InternallyRefCounted, virtual void Ping(grpc_completion_queue* cq, void* tag) = 0; // TODO(roth): Remove these methods when LegacyChannel goes away. - Arena* CreateArena(); - void DestroyArena(Arena* arena); virtual grpc_channel_stack* channel_stack() const { return nullptr; } virtual bool is_client() const { return true; } virtual bool is_promising() const { return true; } + CallArenaAllocator* call_arena_allocator() const { + return call_arena_allocator_.get(); + } + protected: Channel(std::string target, const ChannelArgs& channel_args); @@ -148,16 +150,13 @@ class Channel : public InternallyRefCounted, const RefCountedPtr channelz_node_; const grpc_compression_options compression_options_; - // TODO(ctiller): move to use CallArenaAllocator - CallSizeEstimator call_size_estimator_; - MemoryAllocator allocator_; - Mutex mu_; // The map key needs to be owned strings rather than unowned char*'s to // guarantee that it outlives calls on the core channel (which may outlast // the C++ or other wrapped language Channel that registered these calls). std::map, RegisteredCall> registration_table_ ABSL_GUARDED_BY(mu_); + const RefCountedPtr call_arena_allocator_; }; } // namespace grpc_core diff --git a/src/core/lib/transport/call_arena_allocator.h b/src/core/lib/transport/call_arena_allocator.h index 1db02bf6902..ebc748854ec 100644 --- a/src/core/lib/transport/call_arena_allocator.h +++ b/src/core/lib/transport/call_arena_allocator.h @@ -28,7 +28,7 @@ namespace grpc_core { -class CallSizeEstimator { +class CallSizeEstimator final { public: explicit CallSizeEstimator(size_t initial_estimate) : call_size_estimate_(initial_estimate) {} @@ -52,19 +52,21 @@ class CallSizeEstimator { std::atomic call_size_estimate_; }; -class CallArenaAllocator : public RefCounted { +class CallArenaAllocator final : public ArenaFactory { public: CallArenaAllocator(MemoryAllocator allocator, size_t initial_size) - : allocator_(std::move(allocator)), call_size_estimator_(initial_size) {} + : ArenaFactory(std::move(allocator)), + call_size_estimator_(initial_size) {} - Arena* MakeArena() { - return Arena::Create(call_size_estimator_.CallSizeEstimate(), &allocator_); + RefCountedPtr MakeArena() override { + return Arena::Create(call_size_estimator_.CallSizeEstimate(), Ref()); } - void Destroy(Arena* arena) { arena->Destroy(); } + void FinalizeArena(Arena* arena) override { + call_size_estimator_.UpdateCallSizeEstimate(arena->TotalUsedBytes()); + } private: - MemoryAllocator allocator_; CallSizeEstimator call_size_estimator_; }; diff --git a/src/core/lib/transport/call_spine.cc b/src/core/lib/transport/call_spine.cc index eb05608663f..99270ae09d1 100644 --- a/src/core/lib/transport/call_spine.cc +++ b/src/core/lib/transport/call_spine.cc @@ -94,12 +94,12 @@ void ForwardCall(CallHandler call_handler, CallInitiator call_initiator) { CallInitiatorAndHandler MakeCallPair( ClientMetadataHandle client_initial_metadata, - grpc_event_engine::experimental::EventEngine* event_engine, Arena* arena, - RefCountedPtr call_arena_allocator_if_arena_is_owned, - grpc_call_context_element* legacy_context) { - auto spine = CallSpine::Create( - std::move(client_initial_metadata), event_engine, arena, - std::move(call_arena_allocator_if_arena_is_owned), legacy_context); + grpc_event_engine::experimental::EventEngine* event_engine, + RefCountedPtr arena, grpc_call_context_element* legacy_context) { + CHECK_NE(arena.get(), nullptr); + auto spine = + CallSpine::Create(std::move(client_initial_metadata), event_engine, + std::move(arena), legacy_context); return {CallInitiator(spine), UnstartedCallHandler(spine)}; } diff --git a/src/core/lib/transport/call_spine.h b/src/core/lib/transport/call_spine.h index 8a27c95f593..1dbc939d915 100644 --- a/src/core/lib/transport/call_spine.h +++ b/src/core/lib/transport/call_spine.h @@ -280,12 +280,12 @@ class CallSpine final : public CallSpineInterface, public Party { public: static RefCountedPtr Create( ClientMetadataHandle client_initial_metadata, - grpc_event_engine::experimental::EventEngine* event_engine, Arena* arena, - RefCountedPtr call_arena_allocator_if_arena_is_owned, - grpc_call_context_element* legacy_context) { - return RefCountedPtr(arena->New( - std::move(client_initial_metadata), event_engine, arena, - std::move(call_arena_allocator_if_arena_is_owned), legacy_context)); + grpc_event_engine::experimental::EventEngine* event_engine, + RefCountedPtr arena, grpc_call_context_element* legacy_context) { + auto* arena_ptr = arena.get(); + return RefCountedPtr(arena_ptr->New( + std::move(client_initial_metadata), event_engine, std::move(arena), + legacy_context)); } ~CallSpine() override { @@ -301,7 +301,7 @@ class CallSpine final : public CallSpineInterface, public Party { Party& party() override { return *this; } - Arena* arena() override { return arena_; } + Arena* arena() override { return arena_.get(); } void IncrementRefCount() override { Party::IncrementRefCount(); } @@ -385,18 +385,15 @@ class CallSpine final : public CallSpineInterface, public Party { friend class Arena; CallSpine(ClientMetadataHandle client_initial_metadata, grpc_event_engine::experimental::EventEngine* event_engine, - Arena* arena, - RefCountedPtr call_arena_allocator, + RefCountedPtr arena, grpc_call_context_element* legacy_context) : Party(1), + arena_(std::move(arena)), call_filters_(std::move(client_initial_metadata)), - arena_(arena), - event_engine_(event_engine), - call_arena_allocator_if_arena_is_owned_( - std::move(call_arena_allocator)) { + event_engine_(event_engine) { if (legacy_context == nullptr) { - legacy_context_ = static_cast( - arena->Alloc(sizeof(grpc_call_context_element) * GRPC_CONTEXT_COUNT)); + legacy_context_ = static_cast(arena_->Alloc( + sizeof(grpc_call_context_element) * GRPC_CONTEXT_COUNT)); memset(legacy_context_, 0, sizeof(grpc_call_context_element) * GRPC_CONTEXT_COUNT); legacy_context_is_owned_ = true; @@ -415,7 +412,7 @@ class CallSpine final : public CallSpineInterface, public Party { public: explicit ScopedContext(CallSpine* spine) : ScopedActivity(spine), - Context(spine->arena_), + Context(spine->arena_.get()), Context( spine->event_engine()), Context(spine->legacy_context_) {} @@ -427,29 +424,23 @@ class CallSpine final : public CallSpineInterface, public Party { } void PartyOver() override { - Arena* a = arena_; - RefCountedPtr call_arena_allocator_if_arena_is_owned = - std::move(call_arena_allocator_if_arena_is_owned_); + auto arena = arena_; { ScopedContext context(this); CancelRemainingParticipants(); - a->DestroyManagedNewObjects(); + arena->DestroyManagedNewObjects(); } this->~CallSpine(); - if (call_arena_allocator_if_arena_is_owned != nullptr) { - call_arena_allocator_if_arena_is_owned->Destroy(a); - } } + const RefCountedPtr arena_; // Call filters/pipes part of the spine CallFilters call_filters_; - Arena* const arena_; // Event engine associated with this call grpc_event_engine::experimental::EventEngine* const event_engine_; // Legacy context // TODO(ctiller): remove grpc_call_context_element* legacy_context_; - RefCountedPtr call_arena_allocator_if_arena_is_owned_; bool legacy_context_is_owned_; }; @@ -654,9 +645,8 @@ struct CallInitiatorAndHandler { CallInitiatorAndHandler MakeCallPair( ClientMetadataHandle client_initial_metadata, - grpc_event_engine::experimental::EventEngine* event_engine, Arena* arena, - RefCountedPtr call_arena_allocator_if_arena_is_owned, - grpc_call_context_element* legacy_context); + grpc_event_engine::experimental::EventEngine* event_engine, + RefCountedPtr arena, grpc_call_context_element* legacy_context); template auto OutgoingMessages(CallHalf h) { diff --git a/src/core/lib/transport/interception_chain.cc b/src/core/lib/transport/interception_chain.cc index 4d6ad34e315..667077af586 100644 --- a/src/core/lib/transport/interception_chain.cc +++ b/src/core/lib/transport/interception_chain.cc @@ -40,7 +40,7 @@ CallInitiator HijackedCall::MakeCall() { CallInitiator HijackedCall::MakeCallWithMetadata( ClientMetadataHandle metadata) { auto call = MakeCallPair(std::move(metadata), call_handler_.event_engine(), - call_handler_.arena(), nullptr, + call_handler_.arena()->Ref(), call_handler_.legacy_context()); destination_->StartCall(std::move(call.handler)); return std::move(call.initiator); diff --git a/test/core/call/yodel/yodel_test.h b/test/core/call/yodel/yodel_test.h index d80675750d2..75b9b96a997 100644 --- a/test/core/call/yodel/yodel_test.h +++ b/test/core/call/yodel/yodel_test.h @@ -361,10 +361,9 @@ class YodelTest : public ::testing::Test { } auto MakeCall(ClientMetadataHandle client_initial_metadata) { - auto* arena = state_->call_arena_allocator->MakeArena(); return MakeCallPair(std::move(client_initial_metadata), - state_->event_engine.get(), arena, - state_->call_arena_allocator, nullptr); + state_->event_engine.get(), + state_->call_arena_allocator->MakeArena(), nullptr); } void WaitForAllPendingWork(); diff --git a/test/core/channel/call_finalization_test.cc b/test/core/channel/call_finalization_test.cc index 38b3eaec280..2539d753ac3 100644 --- a/test/core/channel/call_finalization_test.cc +++ b/test/core/channel/call_finalization_test.cc @@ -30,9 +30,7 @@ namespace grpc_core { TEST(CallFinalizationTest, Works) { - auto memory_allocator = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); std::string evidence; TestContext context(arena.get()); CallFinalization finalization; diff --git a/test/core/client_channel/connected_subchannel_test.cc b/test/core/client_channel/connected_subchannel_test.cc index 613509d7456..78b00078da8 100644 --- a/test/core/client_channel/connected_subchannel_test.cc +++ b/test/core/client_channel/connected_subchannel_test.cc @@ -67,9 +67,9 @@ class ConnectedSubchannelTest : public YodelTest { CallInitiatorAndHandler MakeCall( ClientMetadataHandle client_initial_metadata) { - return MakeCallPair( - std::move(client_initial_metadata), event_engine().get(), - call_arena_allocator_->MakeArena(), call_arena_allocator_, nullptr); + return MakeCallPair(std::move(client_initial_metadata), + event_engine().get(), + SimpleArenaAllocator()->MakeArena(), nullptr); } CallHandler TickUntilCallStarted() { @@ -151,11 +151,6 @@ class ConnectedSubchannelTest : public YodelTest { } std::queue handlers_; - RefCountedPtr call_arena_allocator_ = - MakeRefCounted( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "test"), - 1024); }; #define CONNECTED_SUBCHANNEL_CHANNEL_TEST(name) \ diff --git a/test/core/client_channel/load_balanced_call_destination_test.cc b/test/core/client_channel/load_balanced_call_destination_test.cc index c8651fc305f..6e7004bf471 100644 --- a/test/core/client_channel/load_balanced_call_destination_test.cc +++ b/test/core/client_channel/load_balanced_call_destination_test.cc @@ -49,9 +49,9 @@ class LoadBalancedCallDestinationTest : public YodelTest { CallInitiatorAndHandler MakeCall( ClientMetadataHandle client_initial_metadata) { - return MakeCallPair( - std::move(client_initial_metadata), event_engine().get(), - call_arena_allocator_->MakeArena(), call_arena_allocator_, nullptr); + return MakeCallPair(std::move(client_initial_metadata), + event_engine().get(), + call_arena_allocator_->MakeArena(), nullptr); } CallHandler TickUntilCallStarted() { diff --git a/test/core/filters/filter_test.cc b/test/core/filters/filter_test.cc index d3178163c33..c6d11ddd56f 100644 --- a/test/core/filters/filter_test.cc +++ b/test/core/filters/filter_test.cc @@ -83,8 +83,7 @@ class FilterTestBase::Call::Impl Call* const call_; std::shared_ptr const channel_; - ScopedArenaPtr arena_{MakeScopedArena(channel_->initial_arena_size, - &channel_->memory_allocator)}; + RefCountedPtr arena_ = channel_->arena_factory->MakeArena(); bool run_call_finalization_ = false; CallFinalization call_finalization_; absl::optional> promise_; diff --git a/test/core/filters/filter_test.h b/test/core/filters/filter_test.h index c557e6aee16..f48890f2a58 100644 --- a/test/core/filters/filter_test.h +++ b/test/core/filters/filter_test.h @@ -101,19 +101,12 @@ class FilterTestBase : public ::testing::Test { struct Impl { Impl(std::unique_ptr filter, FilterTestBase* test) : filter(std::move(filter)), test(test) {} - size_t initial_arena_size = 1024; - MemoryAllocator memory_allocator = - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "test"); + RefCountedPtr arena_factory = SimpleArenaAllocator(); std::unique_ptr filter; FilterTestBase* const test; }; public: - void set_initial_arena_size(size_t size) { - impl_->initial_arena_size = size; - } - Call MakeCall(); protected: diff --git a/test/core/gprpp/chunked_vector_fuzzer.cc b/test/core/gprpp/chunked_vector_fuzzer.cc index 8463fdd797f..2a83b3df935 100644 --- a/test/core/gprpp/chunked_vector_fuzzer.cc +++ b/test/core/gprpp/chunked_vector_fuzzer.cc @@ -169,7 +169,7 @@ class Fuzzer { MemoryAllocator memory_allocator_ = MemoryAllocator( ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - ScopedArenaPtr arena_ = MakeScopedArena(128, &memory_allocator_); + RefCountedPtr arena_ = SimpleArenaAllocator(128)->MakeArena(); std::map vectors_; }; } // namespace grpc_core diff --git a/test/core/gprpp/chunked_vector_test.cc b/test/core/gprpp/chunked_vector_test.cc index 535374c9b2b..6299b0209d7 100644 --- a/test/core/gprpp/chunked_vector_test.cc +++ b/test/core/gprpp/chunked_vector_test.cc @@ -31,20 +31,14 @@ namespace testing { static constexpr size_t kInitialArenaSize = 1024; static constexpr size_t kChunkSize = 3; -class ChunkedVectorTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(ChunkedVectorTest, Noop) { - auto arena = MakeScopedArena(kInitialArenaSize, &memory_allocator_); +TEST(ChunkedVectorTest, Noop) { + auto arena = SimpleArenaAllocator(kInitialArenaSize)->MakeArena(); ChunkedVector v(arena.get()); EXPECT_EQ(0, v.size()); } -TEST_F(ChunkedVectorTest, Stack) { - auto arena = MakeScopedArena(kInitialArenaSize, &memory_allocator_); +TEST(ChunkedVectorTest, Stack) { + auto arena = SimpleArenaAllocator(kInitialArenaSize)->MakeArena(); ChunkedVector v(arena.get()); // Populate 2 chunks of memory, and 2/3 of a final chunk. @@ -85,8 +79,8 @@ TEST_F(ChunkedVectorTest, Stack) { EXPECT_EQ(0, v.size()); } -TEST_F(ChunkedVectorTest, Iterate) { - auto arena = MakeScopedArena(kInitialArenaSize, &memory_allocator_); +TEST(ChunkedVectorTest, Iterate) { + auto arena = SimpleArenaAllocator(kInitialArenaSize)->MakeArena(); ChunkedVector v(arena.get()); v.EmplaceBack(1); v.EmplaceBack(2); @@ -117,8 +111,8 @@ TEST_F(ChunkedVectorTest, Iterate) { EXPECT_EQ(v.end(), it); } -TEST_F(ChunkedVectorTest, ConstIterate) { - auto arena = MakeScopedArena(kInitialArenaSize, &memory_allocator_); +TEST(ChunkedVectorTest, ConstIterate) { + auto arena = SimpleArenaAllocator(kInitialArenaSize)->MakeArena(); ChunkedVector v(arena.get()); v.EmplaceBack(1); v.EmplaceBack(2); @@ -149,8 +143,8 @@ TEST_F(ChunkedVectorTest, ConstIterate) { EXPECT_EQ(v.cend(), it); } -TEST_F(ChunkedVectorTest, Clear) { - auto arena = MakeScopedArena(kInitialArenaSize, &memory_allocator_); +TEST(ChunkedVectorTest, Clear) { + auto arena = SimpleArenaAllocator(kInitialArenaSize)->MakeArena(); ChunkedVector v(arena.get()); v.EmplaceBack(1); EXPECT_EQ(v.size(), 1); @@ -159,8 +153,8 @@ TEST_F(ChunkedVectorTest, Clear) { EXPECT_EQ(v.begin(), v.end()); } -TEST_F(ChunkedVectorTest, RemoveIf) { - auto arena = MakeScopedArena(kInitialArenaSize, &memory_allocator_); +TEST(ChunkedVectorTest, RemoveIf) { + auto arena = SimpleArenaAllocator(kInitialArenaSize)->MakeArena(); ChunkedVector v(arena.get()); v.EmplaceBack(1); v.SetEnd(std::remove_if(v.begin(), v.end(), [](int i) { return i == 1; })); diff --git a/test/core/promise/arena_promise_test.cc b/test/core/promise/arena_promise_test.cc index 7a412035a5d..d8f43426012 100644 --- a/test/core/promise/arena_promise_test.cc +++ b/test/core/promise/arena_promise_test.cc @@ -30,22 +30,16 @@ namespace grpc_core { -class ArenaPromiseTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(ArenaPromiseTest, DefaultInitializationYieldsNoValue) { - auto arena = MakeScopedArena(1024, &memory_allocator_); +TEST(ArenaPromiseTest, DefaultInitializationYieldsNoValue) { + auto arena = SimpleArenaAllocator()->MakeArena(); TestContext context(arena.get()); ArenaPromise p; EXPECT_FALSE(p.has_value()); } -TEST_F(ArenaPromiseTest, AllocatedWorks) { +TEST(ArenaPromiseTest, AllocatedWorks) { ExecCtx exec_ctx; - auto arena = MakeScopedArena(1024, &memory_allocator_); + auto arena = SimpleArenaAllocator()->MakeArena(); TestContext context(arena.get()); int x = 42; ArenaPromise p([x] { return Poll(x); }); @@ -55,9 +49,9 @@ TEST_F(ArenaPromiseTest, AllocatedWorks) { EXPECT_EQ(p(), Poll(43)); } -TEST_F(ArenaPromiseTest, DestructionWorks) { +TEST(ArenaPromiseTest, DestructionWorks) { ExecCtx exec_ctx; - auto arena = MakeScopedArena(1024, &memory_allocator_); + auto arena = SimpleArenaAllocator()->MakeArena(); TestContext context(arena.get()); auto x = std::make_shared(42); auto p = ArenaPromise([x] { return Poll(*x); }); @@ -65,18 +59,18 @@ TEST_F(ArenaPromiseTest, DestructionWorks) { EXPECT_EQ(q(), Poll(42)); } -TEST_F(ArenaPromiseTest, MoveAssignmentWorks) { +TEST(ArenaPromiseTest, MoveAssignmentWorks) { ExecCtx exec_ctx; - auto arena = MakeScopedArena(1024, &memory_allocator_); + auto arena = SimpleArenaAllocator()->MakeArena(); TestContext context(arena.get()); auto x = std::make_shared(42); auto p = ArenaPromise([x] { return Poll(*x); }); p = ArenaPromise(); } -TEST_F(ArenaPromiseTest, AllocatedUniquePtrWorks) { +TEST(ArenaPromiseTest, AllocatedUniquePtrWorks) { ExecCtx exec_ctx; - auto arena = MakeScopedArena(1024, &memory_allocator_); + auto arena = SimpleArenaAllocator()->MakeArena(); TestContext context(arena.get()); std::array garbage = {0, 1, 2, 3, 4}; auto freer = [garbage](int* p) { free(p + garbage[0]); }; diff --git a/test/core/promise/for_each_test.cc b/test/core/promise/for_each_test.cc index b251176a332..4ce3914004f 100644 --- a/test/core/promise/for_each_test.cc +++ b/test/core/promise/for_each_test.cc @@ -40,13 +40,7 @@ using testing::StrictMock; namespace grpc_core { -class ForEachTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(ForEachTest, SendThriceWithPipe) { +TEST(ForEachTest, SendThriceWithPipe) { int num_received = 0; StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); @@ -76,12 +70,12 @@ TEST_F(ForEachTest, SendThriceWithPipe) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); Mock::VerifyAndClearExpectations(&on_done); EXPECT_EQ(num_received, 3); } -TEST_F(ForEachTest, SendThriceWithInterActivityPipe) { +TEST(ForEachTest, SendThriceWithInterActivityPipe) { int num_received = 0; StrictMock> on_done_sender; StrictMock> on_done_receiver; @@ -148,7 +142,7 @@ class MoveableUntilPolled { int polls_ = 0; }; -TEST_F(ForEachTest, NoMoveAfterPoll) { +TEST(ForEachTest, NoMoveAfterPoll) { int num_received = 0; StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); @@ -179,12 +173,12 @@ TEST_F(ForEachTest, NoMoveAfterPoll) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); Mock::VerifyAndClearExpectations(&on_done); EXPECT_EQ(num_received, 1); } -TEST_F(ForEachTest, NextResultHeldThroughCallback) { +TEST(ForEachTest, NextResultHeldThroughCallback) { int num_received = 0; StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); @@ -230,7 +224,7 @@ TEST_F(ForEachTest, NextResultHeldThroughCallback) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); Mock::VerifyAndClearExpectations(&on_done); EXPECT_EQ(num_received, 1); } diff --git a/test/core/promise/interceptor_list_test.cc b/test/core/promise/interceptor_list_test.cc index ad8941e72b1..f9ff107d289 100644 --- a/test/core/promise/interceptor_list_test.cc +++ b/test/core/promise/interceptor_list_test.cc @@ -31,9 +31,7 @@ namespace { class InterceptorListTest : public ::testing::Test { protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - ScopedArenaPtr arena_ = MakeScopedArena(1024, &memory_allocator_); + RefCountedPtr arena_ = SimpleArenaAllocator()->MakeArena(); TestContext arena_ctx_{arena_.get()}; }; diff --git a/test/core/promise/map_pipe_test.cc b/test/core/promise/map_pipe_test.cc index f8faa3d01bd..0bcc4f7c739 100644 --- a/test/core/promise/map_pipe_test.cc +++ b/test/core/promise/map_pipe_test.cc @@ -58,13 +58,7 @@ class Delayed { T x_; }; -class MapPipeTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(MapPipeTest, SendThriceWithPipeInterceptingReceive) { +TEST(MapPipeTest, SendThriceWithPipeInterceptingReceive) { int num_received = 0; StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); @@ -99,12 +93,12 @@ TEST_F(MapPipeTest, SendThriceWithPipeInterceptingReceive) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); Mock::VerifyAndClearExpectations(&on_done); EXPECT_EQ(num_received, 3); } -TEST_F(MapPipeTest, SendThriceWithPipeInterceptingSend) { +TEST(MapPipeTest, SendThriceWithPipeInterceptingSend) { int num_received = 0; StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); @@ -139,7 +133,7 @@ TEST_F(MapPipeTest, SendThriceWithPipeInterceptingSend) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); Mock::VerifyAndClearExpectations(&on_done); EXPECT_EQ(num_received, 3); } diff --git a/test/core/promise/pipe_test.cc b/test/core/promise/pipe_test.cc index abc7e44f904..e1219c55400 100644 --- a/test/core/promise/pipe_test.cc +++ b/test/core/promise/pipe_test.cc @@ -41,13 +41,7 @@ using testing::StrictMock; namespace grpc_core { -class PipeTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(PipeTest, CanSendAndReceive) { +TEST(PipeTest, CanSendAndReceive) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -68,10 +62,10 @@ TEST_F(PipeTest, CanSendAndReceive) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanInterceptAndMapAtSender) { +TEST(PipeTest, CanInterceptAndMapAtSender) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -93,10 +87,10 @@ TEST_F(PipeTest, CanInterceptAndMapAtSender) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanInterceptAndMapAtReceiver) { +TEST(PipeTest, CanInterceptAndMapAtReceiver) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -118,10 +112,10 @@ TEST_F(PipeTest, CanInterceptAndMapAtReceiver) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, InterceptionOrderingIsCorrect) { +TEST(PipeTest, InterceptionOrderingIsCorrect) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -154,10 +148,10 @@ TEST_F(PipeTest, InterceptionOrderingIsCorrect) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanReceiveAndSend) { +TEST(PipeTest, CanReceiveAndSend) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -178,10 +172,10 @@ TEST_F(PipeTest, CanReceiveAndSend) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanSeeClosedOnSend) { +TEST(PipeTest, CanSeeClosedOnSend) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -208,10 +202,10 @@ TEST_F(PipeTest, CanSeeClosedOnSend) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanSeeClosedOnReceive) { +TEST(PipeTest, CanSeeClosedOnReceive) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -240,10 +234,10 @@ TEST_F(PipeTest, CanSeeClosedOnReceive) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanCloseSend) { +TEST(PipeTest, CanCloseSend) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -270,10 +264,10 @@ TEST_F(PipeTest, CanCloseSend) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanCloseWithErrorSend) { +TEST(PipeTest, CanCloseWithErrorSend) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -300,10 +294,10 @@ TEST_F(PipeTest, CanCloseWithErrorSend) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanCloseWithErrorRecv) { +TEST(PipeTest, CanCloseWithErrorRecv) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -330,10 +324,10 @@ TEST_F(PipeTest, CanCloseWithErrorRecv) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanCloseSendWithInterceptor) { +TEST(PipeTest, CanCloseSendWithInterceptor) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -361,10 +355,10 @@ TEST_F(PipeTest, CanCloseSendWithInterceptor) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanCancelSendWithInterceptor) { +TEST(PipeTest, CanCancelSendWithInterceptor) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -389,10 +383,10 @@ TEST_F(PipeTest, CanCancelSendWithInterceptor) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } -TEST_F(PipeTest, CanFlowControlThroughManyStages) { +TEST(PipeTest, CanFlowControlThroughManyStages) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); auto done = std::make_shared(false); @@ -437,11 +431,11 @@ TEST_F(PipeTest, CanFlowControlThroughManyStages) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); ASSERT_TRUE(*done); } -TEST_F(PipeTest, AwaitClosedWorks) { +TEST(PipeTest, AwaitClosedWorks) { StrictMock> on_done; EXPECT_CALL(on_done, Call(absl::OkStatus())); MakeActivity( @@ -468,7 +462,7 @@ TEST_F(PipeTest, AwaitClosedWorks) { }, NoWakeupScheduler(), [&on_done](absl::Status status) { on_done.Call(std::move(status)); }, - MakeScopedArena(1024, &memory_allocator_)); + SimpleArenaAllocator()->MakeArena()); } class FakeActivity final : public Activity { @@ -483,7 +477,7 @@ class FakeActivity final : public Activity { } }; -TEST_F(PipeTest, PollAckWaitsForReadyClosed) { +TEST(PipeTest, PollAckWaitsForReadyClosed) { FakeActivity().Run([]() { pipe_detail::Center c; int i = 1; diff --git a/test/core/resource_quota/arena_test.cc b/test/core/resource_quota/arena_test.cc index c6055a1e0e5..27ea809eddc 100644 --- a/test/core/resource_quota/arena_test.cc +++ b/test/core/resource_quota/arena_test.cc @@ -57,9 +57,8 @@ class AllocTest : public ::testing::TestWithParam {}; TEST_P(AllocTest, Works) { ExecCtx exec_ctx; - MemoryAllocator memory_allocator = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - Arena* a = Arena::Create(GetParam().initial_size, &memory_allocator); + RefCountedPtr a = + SimpleArenaAllocator(GetParam().initial_size)->MakeArena(); std::vector allocated; for (auto alloc : GetParam().allocs) { void* p = a->Alloc(alloc); @@ -73,7 +72,6 @@ TEST_P(AllocTest, Works) { memset(p, 1, alloc); allocated.push_back(p); } - a->Destroy(); } INSTANTIATE_TEST_SUITE_P( @@ -92,33 +90,23 @@ size_t concurrent_test_iterations() { typedef struct { gpr_event ev_start; - Arena* arena; + RefCountedPtr arena; } concurrent_test_args; -class ArenaTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(ArenaTest, NoOp) { - ExecCtx exec_ctx; - Arena::Create(1, &memory_allocator_)->Destroy(); -} +TEST(ArenaTest, NoOp) { SimpleArenaAllocator()->MakeArena(); } -TEST_F(ArenaTest, ManagedNew) { +TEST(ArenaTest, ManagedNew) { ExecCtx exec_ctx; - Arena* arena = Arena::Create(1, &memory_allocator_); + auto arena = SimpleArenaAllocator(1)->MakeArena(); for (int i = 0; i < 100; i++) { arena->ManagedNew>(std::make_unique(i)); } - arena->Destroy(); } -TEST_F(ArenaTest, ConcurrentAlloc) { +TEST(ArenaTest, ConcurrentAlloc) { concurrent_test_args args; gpr_event_init(&args.ev_start); - args.arena = Arena::Create(1024, &memory_allocator_); + args.arena = SimpleArenaAllocator()->MakeArena(); Thread thds[CONCURRENT_TEST_THREADS]; @@ -141,14 +129,12 @@ TEST_F(ArenaTest, ConcurrentAlloc) { for (auto& th : thds) { th.Join(); } - - args.arena->Destroy(); } -TEST_F(ArenaTest, ConcurrentManagedNew) { +TEST(ArenaTest, ConcurrentManagedNew) { concurrent_test_args args; gpr_event_init(&args.ev_start); - args.arena = Arena::Create(1024, &memory_allocator_); + args.arena = SimpleArenaAllocator()->MakeArena(); Thread thds[CONCURRENT_TEST_THREADS]; @@ -172,8 +158,6 @@ TEST_F(ArenaTest, ConcurrentManagedNew) { for (auto& th : thds) { th.Join(); } - - args.arena->Destroy(); } template @@ -191,31 +175,11 @@ bool IsScribbled(Int* ints, int n, int offset) { return true; } -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC -TEST_F(ArenaTest, PooledObjectsArePooled) { +TEST(ArenaTest, CreateManyObjects) { struct TestObj { char a[100]; }; - - auto arena = MakeScopedArena(1024, &memory_allocator_); - auto obj = arena->MakePooled(); - Scribble(obj->a, 100, 1); - EXPECT_TRUE(IsScribbled(obj->a, 100, 1)); - void* p = obj.get(); - obj.reset(); - obj = arena->MakePooled(); - EXPECT_FALSE(IsScribbled(obj->a, 100, 1)); - EXPECT_EQ(p, obj.get()); - Scribble(obj->a, 100, 2); - EXPECT_TRUE(IsScribbled(obj->a, 100, 2)); -} -#endif - -TEST_F(ArenaTest, CreateManyObjects) { - struct TestObj { - char a[100]; - }; - auto arena = MakeScopedArena(1024, &memory_allocator_); + auto arena = SimpleArenaAllocator()->MakeArena(); std::vector> objs; objs.reserve(1000); for (int i = 0; i < 1000; i++) { @@ -227,9 +191,9 @@ TEST_F(ArenaTest, CreateManyObjects) { } } -TEST_F(ArenaTest, CreateManyObjectsWithDestructors) { +TEST(ArenaTest, CreateManyObjectsWithDestructors) { using TestObj = std::unique_ptr; - auto arena = MakeScopedArena(1024, &memory_allocator_); + auto arena = SimpleArenaAllocator()->MakeArena(); std::vector> objs; objs.reserve(1000); for (int i = 0; i < 1000; i++) { @@ -237,24 +201,20 @@ TEST_F(ArenaTest, CreateManyObjectsWithDestructors) { } } -TEST_F(ArenaTest, CreatePoolArray) { - auto arena = MakeScopedArena(1024, &memory_allocator_); +TEST(ArenaTest, CreatePoolArray) { + auto arena = SimpleArenaAllocator()->MakeArena(); auto p = arena->MakePooledArray(1024); -#ifndef GRPC_ARENA_POOLED_ALLOCATIONS_USE_MALLOC - EXPECT_FALSE(p.get_deleter().has_freelist()); -#else EXPECT_TRUE(p.get_deleter().has_freelist()); -#endif p = arena->MakePooledArray(5); EXPECT_TRUE(p.get_deleter().has_freelist()); Scribble(p.get(), 5, 1); EXPECT_TRUE(IsScribbled(p.get(), 5, 1)); } -TEST_F(ArenaTest, ConcurrentMakePooled) { +TEST(ArenaTest, ConcurrentMakePooled) { concurrent_test_args args; gpr_event_init(&args.ev_start); - args.arena = Arena::Create(1024, &memory_allocator_); + args.arena = SimpleArenaAllocator()->MakeArena(); class BaseClass { public: @@ -309,8 +269,6 @@ TEST_F(ArenaTest, ConcurrentMakePooled) { for (auto& th : thds2) { th.Join(); } - - args.arena->Destroy(); } } // namespace grpc_core diff --git a/test/core/security/credentials_test.cc b/test/core/security/credentials_test.cc index 35e1aad2be8..0e3ed6b59c0 100644 --- a/test/core/security/credentials_test.cc +++ b/test/core/security/credentials_test.cc @@ -515,9 +515,7 @@ class RequestMetadataState : public RefCounted { grpc_error_handle expected_error_; std::string expected_; - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - ScopedArenaPtr arena_ = MakeScopedArena(1024, &memory_allocator_); + RefCountedPtr arena_ = SimpleArenaAllocator()->MakeArena(); grpc_metadata_batch md_; grpc_call_credentials::GetRequestMetadataArgs get_request_metadata_args_; grpc_polling_entity pollent_; diff --git a/test/core/security/oauth2_utils.cc b/test/core/security/oauth2_utils.cc index 9bc6845db86..b135ac3eef2 100644 --- a/test/core/security/oauth2_utils.cc +++ b/test/core/security/oauth2_utils.cc @@ -51,11 +51,7 @@ char* grpc_test_fetch_oauth2_token_with_credentials( auto pops = grpc_polling_entity_create_from_pollset(pollset); bool is_done = false; grpc_core::Notification done; - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); + auto arena = grpc_core::SimpleArenaAllocator()->MakeArena(); grpc_metadata_batch initial_metadata; char* token = nullptr; diff --git a/test/core/surface/channel_init_test.cc b/test/core/surface/channel_init_test.cc index a548b221eac..67c35c482fd 100644 --- a/test/core/surface/channel_init_test.cc +++ b/test/core/surface/channel_init_test.cc @@ -268,7 +268,7 @@ TEST(ChannelInitTest, CanCreateFilterWithCall) { "test"), 1024); auto call = MakeCallPair(Arena::MakePooled(), nullptr, - allocator->MakeArena(), allocator, nullptr); + allocator->MakeArena(), nullptr); (*stack)->StartCall(std::move(call.handler)); EXPECT_EQ(p, 1); EXPECT_EQ(handled, 1); diff --git a/test/core/telemetry/call_tracer_test.cc b/test/core/telemetry/call_tracer_test.cc index 3adfb0903d5..3adbc5d6204 100644 --- a/test/core/telemetry/call_tracer_test.cc +++ b/test/core/telemetry/call_tracer_test.cc @@ -37,20 +37,7 @@ namespace { class CallTracerTest : public ::testing::Test { protected: - void SetUp() override { - memory_allocator_ = new MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "test")); - arena_ = Arena::Create(1024, memory_allocator_); - } - - void TearDown() override { - arena_->Destroy(); - delete memory_allocator_; - } - - MemoryAllocator* memory_allocator_ = nullptr; - Arena* arena_ = nullptr; + RefCountedPtr arena_ = SimpleArenaAllocator()->MakeArena(); grpc_call_context_element context_[GRPC_CONTEXT_COUNT] = {}; std::vector annotation_logger_; }; @@ -65,7 +52,7 @@ TEST_F(CallTracerTest, BasicClientCallTracer) { } TEST_F(CallTracerTest, MultipleClientCallTracers) { - promise_detail::Context arena_ctx(arena_); + promise_detail::Context arena_ctx(arena_.get()); FakeClientCallTracer client_call_tracer1(&annotation_logger_); FakeClientCallTracer client_call_tracer2(&annotation_logger_); FakeClientCallTracer client_call_tracer3(&annotation_logger_); @@ -80,7 +67,7 @@ TEST_F(CallTracerTest, MultipleClientCallTracers) { } TEST_F(CallTracerTest, MultipleClientCallAttemptTracers) { - promise_detail::Context arena_ctx(arena_); + promise_detail::Context arena_ctx(arena_.get()); FakeClientCallTracer client_call_tracer1(&annotation_logger_); FakeClientCallTracer client_call_tracer2(&annotation_logger_); FakeClientCallTracer client_call_tracer3(&annotation_logger_); @@ -110,7 +97,7 @@ TEST_F(CallTracerTest, BasicServerCallTracerTest) { } TEST_F(CallTracerTest, MultipleServerCallTracers) { - promise_detail::Context arena_ctx(arena_); + promise_detail::Context arena_ctx(arena_.get()); FakeServerCallTracer server_call_tracer1(&annotation_logger_); FakeServerCallTracer server_call_tracer2(&annotation_logger_); FakeServerCallTracer server_call_tracer3(&annotation_logger_); diff --git a/test/core/transport/binder/binder_transport_test.cc b/test/core/transport/binder/binder_transport_test.cc index 24ddd071b77..933648a6a22 100644 --- a/test/core/transport/binder/binder_transport_test.cc +++ b/test/core/transport/binder/binder_transport_test.cc @@ -64,7 +64,6 @@ class BinderTransportTest : public ::testing::Test { gbs->~grpc_binder_stream(); gpr_free(gbs); } - arena_->Destroy(); } void PerformStreamOp(grpc_binder_stream* gbs, @@ -81,7 +80,7 @@ class BinderTransportTest : public ::testing::Test { grpc_binder_stream* gbs = static_cast( gpr_malloc(transport_->filter_stack_transport()->SizeOfStream())); transport_->filter_stack_transport()->InitStream( - reinterpret_cast(gbs), &ref_, nullptr, arena_); + reinterpret_cast(gbs), &ref_, nullptr, arena_.get()); stream_buffer_.push_back(gbs); return gbs; } @@ -95,12 +94,8 @@ class BinderTransportTest : public ::testing::Test { static void TearDownTestSuite() { grpc_shutdown(); } protected: - grpc_core::MemoryAllocator memory_allocator_ = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - grpc_core::Arena* arena_ = - grpc_core::Arena::Create(/* initial_size = */ 1, &memory_allocator_); + grpc_core::RefCountedPtr arena_ = + grpc_core::SimpleArenaAllocator()->MakeArena(); grpc_core::Transport* transport_; grpc_stream_refcount ref_; std::vector stream_buffer_; @@ -234,12 +229,6 @@ struct MakeSendInitialMetadata { } ~MakeSendInitialMetadata() {} - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - grpc_core::ScopedArenaPtr arena = - grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch grpc_initial_metadata; }; @@ -269,8 +258,6 @@ struct MakeSendTrailingMetadata { grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() ->memory_quota() ->CreateMemoryAllocator("test")); - grpc_core::ScopedArenaPtr arena = - grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch grpc_trailing_metadata; }; @@ -297,8 +284,6 @@ struct MakeRecvInitialMetadata { grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() ->memory_quota() ->CreateMemoryAllocator("test")); - grpc_core::ScopedArenaPtr arena = - grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch grpc_initial_metadata; grpc_core::Notification notification; }; @@ -345,8 +330,6 @@ struct MakeRecvTrailingMetadata { grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() ->memory_quota() ->CreateMemoryAllocator("test")); - grpc_core::ScopedArenaPtr arena = - grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch grpc_trailing_metadata; grpc_core::Notification notification; }; diff --git a/test/core/transport/call_filters_test.cc b/test/core/transport/call_filters_test.cc index 750f16b2ec1..da58188eec6 100644 --- a/test/core/transport/call_filters_test.cc +++ b/test/core/transport/call_filters_test.cc @@ -407,9 +407,7 @@ TEST(StackDataTest, InstantClientInitialMetadataReturningAbslStatus) { // Check promise init void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); d.filter_constructor[0].call_init(call_data, &f1); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); promise_detail::Context ctx(arena.get()); // A succeeding call @@ -467,9 +465,7 @@ TEST(StackDataTest, // Check promise init void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); d.filter_constructor[0].call_init(call_data, &f1); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); promise_detail::Context ctx(arena.get()); // A succeeding call @@ -526,9 +522,7 @@ TEST(StackDataTest, InstantClientInitialMetadataReturningServerMetadata) { // Check promise init void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); d.filter_constructor[0].call_init(call_data, &f1); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); promise_detail::Context ctx(arena.get()); // A succeeding call @@ -588,9 +582,7 @@ TEST(StackDataTest, // Check promise init void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); d.filter_constructor[0].call_init(call_data, &f1); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); promise_detail::Context ctx(arena.get()); // A succeeding call @@ -648,9 +640,7 @@ TEST(StackDataTest, PromiseClientInitialMetadataReturningAbslStatus) { // Check promise init void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); d.filter_constructor[0].call_init(call_data, &f1); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); promise_detail::Context ctx(arena.get()); // A succeeding call @@ -731,9 +721,7 @@ TEST(StackDataTest, // Check promise init void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); d.filter_constructor[0].call_init(call_data, &f1); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); promise_detail::Context ctx(arena.get()); // A succeeding call @@ -801,9 +789,7 @@ TEST(StackDataTest, InstantServerInitialMetadataReturningVoid) { EXPECT_EQ(d.server_initial_metadata.ops[0].poll, nullptr); EXPECT_EQ(d.server_initial_metadata.ops[0].early_destroy, nullptr); // Check promise init - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); EXPECT_EQ(md->get_pointer(HttpPathMetadata()), nullptr); char call_data; @@ -838,9 +824,7 @@ TEST(StackDataTest, InstantClientToServerMessagesReturningVoid) { EXPECT_EQ(d.client_to_server_messages.ops[0].poll, nullptr); EXPECT_EQ(d.client_to_server_messages.ops[0].early_destroy, nullptr); // Check promise init - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto message = Arena::MakePooled(SliceBuffer(), 0); char call_data; auto r = d.client_to_server_messages.ops[0].promise_init( @@ -873,9 +857,7 @@ TEST(StackDataTest, InstantServerToClientMessagesReturningVoid) { EXPECT_EQ(d.server_to_client_messages.ops[0].poll, nullptr); EXPECT_EQ(d.server_to_client_messages.ops[0].early_destroy, nullptr); // Check promise init - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto message = Arena::MakePooled(SliceBuffer(), 0); char call_data; auto r = d.server_to_client_messages.ops[0].promise_init( @@ -908,9 +890,7 @@ TEST(StackDataTest, InstantServerTrailingMetadataReturningVoid) { EXPECT_EQ(d.server_trailing_metadata.ops[0].poll, nullptr); EXPECT_EQ(d.server_trailing_metadata.ops[0].early_destroy, nullptr); // Check promise init - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); EXPECT_EQ(md->get_pointer(HttpPathMetadata()), nullptr); char call_data; @@ -948,9 +928,7 @@ TEST(StackDataTest, EXPECT_EQ(d.server_trailing_metadata.ops[0].poll, nullptr); EXPECT_EQ(d.server_trailing_metadata.ops[0].early_destroy, nullptr); // Check promise init - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); auto md = Arena::MakePooled(); EXPECT_EQ(md->get_pointer(HttpPathMetadata()), nullptr); char call_data; @@ -1038,9 +1016,7 @@ TEST(OperationExecutorTest, InstantTwo) { d.filter_constructor[0].call_init(call_data1, &f1); d.filter_constructor[1].call_init(call_data2, &f2); OperationExecutor transformer; - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); promise_detail::Context ctx(arena.get()); // First call succeeds auto md = Arena::MakePooled(); @@ -1103,9 +1079,7 @@ TEST(OperationExecutorTest, PromiseTwo) { d.filter_constructor[0].call_init(call_data1, &f1); d.filter_constructor[1].call_init(call_data2, &f2); OperationExecutor transformer; - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); promise_detail::Context ctx(arena.get()); // First call succeeds after two sets of two step delays. auto md = Arena::MakePooled(); @@ -1175,9 +1149,7 @@ TEST(InfallibleOperationExecutor, InstantTwo) { ASSERT_EQ(d.server_trailing_metadata.ops.size(), 2u); void* call_data = gpr_malloc_aligned(d.call_data_size, d.call_data_alignment); InfallibleOperationExecutor transformer; - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); promise_detail::Context ctx(arena.get()); auto md = Arena::MakePooled(); EXPECT_EQ(md->get_pointer(HttpPathMetadata()), nullptr); @@ -1381,9 +1353,7 @@ TEST(CallFiltersTest, UnaryCall) { CallFilters::StackBuilder builder; builder.Add(&f1); builder.Add(&f2); - auto memory_allocator = - MakeMemoryQuota("test-quota")->CreateMemoryAllocator("foo"); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); CallFilters filters(Arena::MakePooled()); filters.SetStack(builder.Build()); promise_detail::Context ctx(arena.get()); diff --git a/test/core/transport/chaotic_good/client_transport_error_test.cc b/test/core/transport/chaotic_good/client_transport_error_test.cc index 737f71a7aa2..0dcc5595c07 100644 --- a/test/core/transport/chaotic_good/client_transport_error_test.cc +++ b/test/core/transport/chaotic_good/client_transport_error_test.cc @@ -141,9 +141,8 @@ class ClientTransportTest : public ::testing::Test { } auto MakeCall(ClientMetadataHandle client_initial_metadata) { - auto* arena = call_arena_allocator_->MakeArena(); return MakeCallPair(std::move(client_initial_metadata), event_engine_.get(), - arena, call_arena_allocator_, nullptr); + call_arena_allocator_->MakeArena(), nullptr); } private: diff --git a/test/core/transport/chaotic_good/frame_fuzzer.cc b/test/core/transport/chaotic_good/frame_fuzzer.cc index d8e2e4499fb..92dc762e4ac 100644 --- a/test/core/transport/chaotic_good/frame_fuzzer.cc +++ b/test/core/transport/chaotic_good/frame_fuzzer.cc @@ -103,9 +103,7 @@ void Run(const frame_fuzzer::Test& test) { LOG(INFO) << "Read frame header: " << r->ToString(); control_data += 24; control_size -= 24; - MemoryAllocator memory_allocator = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); TestContext ctx(arena.get()); BufferPair buffers{ SliceBuffer(Slice::FromCopiedBuffer(control_data, control_size)), diff --git a/test/core/transport/chaotic_good/frame_test.cc b/test/core/transport/chaotic_good/frame_test.cc index 15389751a76..ba61e0b9f0c 100644 --- a/test/core/transport/chaotic_good/frame_test.cc +++ b/test/core/transport/chaotic_good/frame_test.cc @@ -50,7 +50,7 @@ void AssertRoundTrips(const T& input, FrameType expected_frame_type) { MemoryAllocator allocator = MakeResourceQuota("test-quota") ->memory_quota() ->CreateMemoryAllocator("test-allocator"); - ScopedArenaPtr arena = MakeScopedArena(1024, &allocator); + RefCountedPtr arena = SimpleArenaAllocator()->MakeArena(); auto deser = output.Deserialize(&hpack_parser, header.value(), absl::BitGenRef(bitgen), arena.get(), std::move(serialized), TestFrameLimits()); diff --git a/test/core/transport/chaotic_good/transport_test.h b/test/core/transport/chaotic_good/transport_test.h index 3604be1f190..a67a5ca7065 100644 --- a/test/core/transport/chaotic_good/transport_test.h +++ b/test/core/transport/chaotic_good/transport_test.h @@ -36,16 +36,17 @@ class TransportTest : public ::testing::Test { return event_engine_; } - Arena* MakeArena() { return call_arena_allocator_->MakeArena(); } + RefCountedPtr MakeArena() { + return call_arena_allocator_->MakeArena(); + } RefCountedPtr call_arena_allocator() { return call_arena_allocator_; } auto MakeCall(ClientMetadataHandle client_initial_metadata) { - auto* arena = call_arena_allocator_->MakeArena(); return MakeCallPair(std::move(client_initial_metadata), event_engine_.get(), - arena, call_arena_allocator_, nullptr); + MakeArena(), nullptr); } private: diff --git a/test/core/transport/chttp2/hpack_encoder_test.cc b/test/core/transport/chttp2/hpack_encoder_test.cc index 28668b730da..68a3e4fd93b 100644 --- a/test/core/transport/chttp2/hpack_encoder_test.cc +++ b/test/core/transport/chttp2/hpack_encoder_test.cc @@ -153,12 +153,6 @@ grpc_slice EncodeHeaderIntoBytes( const std::vector>& header_fields) { std::unique_ptr compressor = std::make_unique(); - - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch b; for (const auto& field : header_fields) { @@ -307,7 +301,6 @@ static void verify_continuation_headers(const char* key, const char* value, grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() ->memory_quota() ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_slice_buffer output; grpc_metadata_batch b; b.Append(key, grpc_core::Slice::FromStaticString(value), CrashOnAppendError); @@ -344,11 +337,6 @@ TEST(HpackEncoderTest, TestContinuationHeaders) { } TEST(HpackEncoderTest, EncodeBinaryAsBase64) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch b; // Haiku by Bard b.Append("grpc-trace-bin", @@ -374,11 +362,6 @@ TEST(HpackEncoderTest, EncodeBinaryAsBase64) { } TEST(HpackEncoderTest, EncodeBinaryAsTrueBinary) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch b; // Haiku by Bard b.Append("grpc-trace-bin", diff --git a/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc b/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc index 44edee9f029..1b21b228935 100644 --- a/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc +++ b/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc @@ -69,7 +69,7 @@ DEFINE_PROTO_FUZZER(const hpack_parser_fuzzer::Msg& msg) { bool can_update_max_length = true; bool can_add_priority = true; for (int i = 0; i < msg.frames_size(); i++) { - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); + auto arena = grpc_core::SimpleArenaAllocator()->MakeArena(); grpc_core::ExecCtx exec_ctx; grpc_metadata_batch b; const auto& frame = msg.frames(i); diff --git a/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc b/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc index ab31d065399..4094a669b3c 100644 --- a/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc +++ b/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc @@ -82,9 +82,7 @@ bool IsStreamError(const absl::Status& status) { absl::StatusOr TestVector(grpc_slice_split_mode mode, Slice input) { - MemoryAllocator memory_allocator = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); - auto arena = MakeScopedArena(1024, &memory_allocator); + auto arena = SimpleArenaAllocator()->MakeArena(); ExecCtx exec_ctx; grpc_slice* slices; size_t nslices; diff --git a/test/core/transport/chttp2/hpack_parser_test.cc b/test/core/transport/chttp2/hpack_parser_test.cc index ffed5b8ab52..d735ebc2b7c 100644 --- a/test/core/transport/chttp2/hpack_parser_test.cc +++ b/test/core/transport/chttp2/hpack_parser_test.cc @@ -107,10 +107,6 @@ class ParseTest : public ::testing::TestWithParam { absl::optional max_metadata_size, std::string hexstring, absl::StatusOr expect, uint32_t flags) { - MemoryAllocator memory_allocator = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "test")); - auto arena = MakeScopedArena(1024, &memory_allocator); ExecCtx exec_ctx; auto input = ParseHexstring(hexstring); grpc_slice* slices; diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.cc b/test/core/transport/chttp2/hpack_sync_fuzzer.cc index 31372ab876c..230ddc8a5cb 100644 --- a/test/core/transport/chttp2/hpack_sync_fuzzer.cc +++ b/test/core/transport/chttp2/hpack_sync_fuzzer.cc @@ -117,10 +117,6 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) { // STAGE 2: Decode the buffer (encode_output) into a list of headers HPackParser parser; - auto memory_allocator = - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator( - "test-allocator"); - auto arena = MakeScopedArena(1024, &memory_allocator); ExecCtx exec_ctx; grpc_metadata_batch read_metadata; parser.BeginFrame( diff --git a/test/core/transport/interception_chain_test.cc b/test/core/transport/interception_chain_test.cc index ba0b42d7f99..0763ab01a81 100644 --- a/test/core/transport/interception_chain_test.cc +++ b/test/core/transport/interception_chain_test.cc @@ -230,9 +230,8 @@ class InterceptionChainTest : public ::testing::Test { // Run a call through a UnstartedCallDestination until it's complete. FinishedCall RunCall(UnstartedCallDestination* destination) { - auto* arena = call_arena_allocator_->MakeArena(); auto call = MakeCallPair(Arena::MakePooled(), nullptr, - arena, call_arena_allocator_, nullptr); + call_arena_allocator_->MakeArena(), nullptr); Poll trailing_md; call.initiator.SpawnInfallible( "run_call", [destination, &call, &trailing_md]() mutable { diff --git a/test/core/transport/metadata_map_test.cc b/test/core/transport/metadata_map_test.cc index 5a3fb59c16b..12e42c7167a 100644 --- a/test/core/transport/metadata_map_test.cc +++ b/test/core/transport/metadata_map_test.cc @@ -56,24 +56,11 @@ struct StreamNetworkStateMetadataMap GrpcStreamNetworkState>::MetadataMap; }; -class MetadataMapTest : public ::testing::Test { - protected: - MemoryAllocator memory_allocator_ = MemoryAllocator( - ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test")); -}; - -TEST_F(MetadataMapTest, Noop) { - auto arena = MakeScopedArena(1024, &memory_allocator_); - EmptyMetadataMap(); -} +TEST(MetadataMapTest, Noop) { EmptyMetadataMap(); } -TEST_F(MetadataMapTest, NoopWithDeadline) { - auto arena = MakeScopedArena(1024, &memory_allocator_); - TimeoutOnlyMetadataMap(); -} +TEST(MetadataMapTest, NoopWithDeadline) { TimeoutOnlyMetadataMap(); } -TEST_F(MetadataMapTest, SimpleOps) { - auto arena = MakeScopedArena(1024, &memory_allocator_); +TEST(MetadataMapTest, SimpleOps) { TimeoutOnlyMetadataMap map; EXPECT_EQ(map.get_pointer(GrpcTimeoutMetadata()), nullptr); EXPECT_EQ(map.get(GrpcTimeoutMetadata()), absl::nullopt); @@ -110,17 +97,15 @@ class FakeEncoder { std::string output_; }; -TEST_F(MetadataMapTest, EmptyEncodeTest) { +TEST(MetadataMapTest, EmptyEncodeTest) { FakeEncoder encoder; - auto arena = MakeScopedArena(1024, &memory_allocator_); TimeoutOnlyMetadataMap map; map.Encode(&encoder); EXPECT_EQ(encoder.output(), ""); } -TEST_F(MetadataMapTest, TimeoutEncodeTest) { +TEST(MetadataMapTest, TimeoutEncodeTest) { FakeEncoder encoder; - auto arena = MakeScopedArena(1024, &memory_allocator_); TimeoutOnlyMetadataMap map; map.Set(GrpcTimeoutMetadata(), Timestamp::FromMillisecondsAfterProcessEpoch(1234)); @@ -128,13 +113,12 @@ TEST_F(MetadataMapTest, TimeoutEncodeTest) { EXPECT_EQ(encoder.output(), "grpc-timeout: deadline=1234\n"); } -TEST_F(MetadataMapTest, NonEncodableTrait) { +TEST(MetadataMapTest, NonEncodableTrait) { struct EncoderWithNoTraitEncodeFunctions { void Encode(const Slice&, const Slice&) { abort(); // should not be called } }; - auto arena = MakeScopedArena(1024, &memory_allocator_); StreamNetworkStateMetadataMap map; map.Set(GrpcStreamNetworkState(), GrpcStreamNetworkState::kNotSentOnWire); EXPECT_EQ(map.get(GrpcStreamNetworkState()), diff --git a/test/cpp/microbenchmarks/bm_arena.cc b/test/cpp/microbenchmarks/bm_arena.cc index f5f8bba1a13..98b9b080354 100644 --- a/test/cpp/microbenchmarks/bm_arena.cc +++ b/test/cpp/microbenchmarks/bm_arena.cc @@ -26,50 +26,36 @@ #include "test/cpp/microbenchmarks/helpers.h" #include "test/cpp/util/test_config.h" -using grpc_core::Arena; - static void BM_Arena_NoOp(benchmark::State& state) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); + auto factory = grpc_core::SimpleArenaAllocator(); for (auto _ : state) { - Arena::Create(state.range(0), &memory_allocator)->Destroy(); + factory->MakeArena(); } } BENCHMARK(BM_Arena_NoOp)->Range(1, 1024 * 1024); static void BM_Arena_ManyAlloc(benchmark::State& state) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - Arena* a = Arena::Create(state.range(0), &memory_allocator); + auto allocator = grpc_core::SimpleArenaAllocator(state.range(0)); + auto a = allocator->MakeArena(); const size_t realloc_after = 1024 * 1024 * 1024 / ((state.range(1) + 15) & 0xffffff0u); while (state.KeepRunning()) { a->Alloc(state.range(1)); // periodically recreate arena to avoid OOM if (state.iterations() % realloc_after == 0) { - a->Destroy(); - a = Arena::Create(state.range(0), &memory_allocator); + a = allocator->MakeArena(); } } - a->Destroy(); } BENCHMARK(BM_Arena_ManyAlloc)->Ranges({{1, 1024 * 1024}, {1, 32 * 1024}}); static void BM_Arena_Batch(benchmark::State& state) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); + auto allocator = grpc_core::SimpleArenaAllocator(state.range(0)); for (auto _ : state) { - Arena* a = Arena::Create(state.range(0), &memory_allocator); + auto a = allocator->MakeArena(); for (int i = 0; i < state.range(1); i++) { a->Alloc(state.range(2)); } - a->Destroy(); } } BENCHMARK(BM_Arena_Batch)->Ranges({{1, 64 * 1024}, {1, 64}, {1, 1024}}); @@ -82,30 +68,20 @@ struct TestThingToAllocate { }; static void BM_Arena_MakePooled_Small(benchmark::State& state) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - Arena* a = Arena::Create(1024, &memory_allocator); + auto a = grpc_core::SimpleArenaAllocator()->MakeArena(); for (auto _ : state) { a->MakePooled(); } - a->Destroy(); } BENCHMARK(BM_Arena_MakePooled_Small); static void BM_Arena_MakePooled3_Small(benchmark::State& state) { - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - Arena* a = Arena::Create(1024, &memory_allocator); + auto a = grpc_core::SimpleArenaAllocator()->MakeArena(); for (auto _ : state) { auto x = a->MakePooled(); auto y = a->MakePooled(); auto z = a->MakePooled(); } - a->Destroy(); } BENCHMARK(BM_Arena_MakePooled3_Small); diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 9c752a6342f..2f18b1de233 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -75,7 +75,6 @@ static void BM_HpackEncoderEncodeDeadline(benchmark::State& state) { grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() ->memory_quota() ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch b; b.Set(grpc_core::GrpcTimeoutMetadata(), saved_now + grpc_core::Duration::Seconds(30)); @@ -107,11 +106,6 @@ static void BM_HpackEncoderEncodeHeader(benchmark::State& state) { grpc_core::ExecCtx exec_ctx; static bool logged_representative_output = false; - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch b; Fixture::Prepare(&b); @@ -345,12 +339,6 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { std::vector benchmark_slices = Fixture::GetBenchmarkSlices(); grpc_core::ExecCtx exec_ctx; grpc_core::HPackParser p; - const int kArenaSize = 4096 * 4096; - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto* arena = grpc_core::Arena::Create(kArenaSize, &memory_allocator); grpc_core::ManualConstructor b; b.Init(); p.BeginFrame(&*b, std::numeric_limits::max(), @@ -373,25 +361,11 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { b->Clear(); parse_vec(benchmark_slices); grpc_core::ExecCtx::Get()->Flush(); - // Recreate arena every 4k iterations to avoid oom - if (0 == (state.iterations() & 0xfff)) { - b.Destroy(); - arena->Destroy(); - arena = grpc_core::Arena::Create(kArenaSize, &memory_allocator); - b.Init(); - p.BeginFrame(&*b, std::numeric_limits::max(), - std::numeric_limits::max(), - grpc_core::HPackParser::Boundary::None, - grpc_core::HPackParser::Priority::None, - grpc_core::HPackParser::LogInfo{ - 1, grpc_core::HPackParser::LogInfo::kHeaders, false}); - } } // Clean up b.Destroy(); for (auto slice : init_slices) grpc_slice_unref(slice); for (auto slice : benchmark_slices) grpc_slice_unref(slice); - arena->Destroy(); } namespace hpack_parser_fixtures { @@ -405,12 +379,6 @@ class FromEncoderFixture { private: static std::vector Generate(int iteration) { grpc_core::ExecCtx exec_ctx; - - grpc_core::MemoryAllocator memory_allocator = - grpc_core::MemoryAllocator(grpc_core::ResourceQuota::Default() - ->memory_quota() - ->CreateMemoryAllocator("test")); - auto arena = grpc_core::MakeScopedArena(1024, &memory_allocator); grpc_metadata_batch b; EncoderFixture::Prepare(&b);