diff --git a/src/core/ext/transport/chttp2/server/chttp2_server.cc b/src/core/ext/transport/chttp2/server/chttp2_server.cc index a36c568575091..940c69979b161 100644 --- a/src/core/ext/transport/chttp2/server/chttp2_server.cc +++ b/src/core/ext/transport/chttp2/server/chttp2_server.cc @@ -833,8 +833,7 @@ void Chttp2ServerListener::OnAccept(void* arg, grpc_endpoint* tcp, return; } } - auto memory_owner = self->memory_quota_->CreateMemoryOwner( - absl::StrCat(grpc_endpoint_get_peer(tcp), ":server_channel")); + auto memory_owner = self->memory_quota_->CreateMemoryOwner(); EventEngine* const event_engine = self->args_.GetObject(); auto connection = memory_owner.MakeOrphanable( accepting_pollset, acceptor, event_engine, args, std::move(memory_owner)); diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 5330a119a07c3..29e149aa7293a 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -619,8 +619,7 @@ grpc_chttp2_transport::grpc_chttp2_transport( grpc_core::Slice::FromCopiedString(grpc_endpoint_get_peer(ep))), memory_owner(channel_args.GetObject() ->memory_quota() - ->CreateMemoryOwner(absl::StrCat( - grpc_endpoint_get_peer(ep), ":client_transport"))), + ->CreateMemoryOwner()), self_reservation( memory_owner.MakeReservation(sizeof(grpc_chttp2_transport))), event_engine( diff --git a/src/core/lib/event_engine/posix_engine/posix_endpoint.cc b/src/core/lib/event_engine/posix_engine/posix_endpoint.cc index 0fc192c3e5bdb..10558f3055288 100644 --- a/src/core/lib/event_engine/posix_engine/posix_endpoint.cc +++ b/src/core/lib/event_engine/posix_engine/posix_endpoint.cc @@ -1246,8 +1246,7 @@ PosixEndpointImpl::PosixEndpointImpl(EventHandle* handle, GPR_ASSERT(options.resource_quota != nullptr); auto peer_addr_string = sock.PeerAddressString(); mem_quota_ = options.resource_quota->memory_quota(); - memory_owner_ = mem_quota_->CreateMemoryOwner( - peer_addr_string.ok() ? *peer_addr_string : ""); + memory_owner_ = mem_quota_->CreateMemoryOwner(); self_reservation_ = memory_owner_.MakeReservation(sizeof(PosixEndpointImpl)); auto local_address = sock.LocalAddress(); if (local_address.ok()) { diff --git a/src/core/lib/iomgr/event_engine_shims/tcp_client.cc b/src/core/lib/iomgr/event_engine_shims/tcp_client.cc index 227a1ec237e96..f1a219b732030 100644 --- a/src/core/lib/iomgr/event_engine_shims/tcp_client.cc +++ b/src/core/lib/iomgr/event_engine_shims/tcp_client.cc @@ -72,8 +72,7 @@ int64_t event_engine_tcp_client_connect( }, CreateResolvedAddress(*addr), config, resource_quota != nullptr - ? resource_quota->memory_quota()->CreateMemoryOwner( - absl::StrCat("tcp-client:", addr_uri.value())) + ? resource_quota->memory_quota()->CreateMemoryOwner() : grpc_event_engine::experimental::MemoryAllocator(), std::max(grpc_core::Duration::Milliseconds(1), deadline - grpc_core::Timestamp::Now())); diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc index d46e1f9ed8f2e..d270c3c9fd2ba 100644 --- a/src/core/lib/iomgr/tcp_posix.cc +++ b/src/core/lib/iomgr/tcp_posix.cc @@ -1944,7 +1944,7 @@ grpc_endpoint* grpc_tcp_create(grpc_fd* em_fd, tcp->fd = grpc_fd_wrapped_fd(em_fd); GPR_ASSERT(options.resource_quota != nullptr); tcp->memory_owner = - options.resource_quota->memory_quota()->CreateMemoryOwner(peer_string); + options.resource_quota->memory_quota()->CreateMemoryOwner(); tcp->self_reservation = tcp->memory_owner.MakeReservation(sizeof(grpc_tcp)); grpc_resolved_address resolved_local_addr; memset(&resolved_local_addr, 0, sizeof(resolved_local_addr)); diff --git a/src/core/lib/resource_quota/memory_quota.cc b/src/core/lib/resource_quota/memory_quota.cc index 07a9e17a6036a..6335a54c484fd 100644 --- a/src/core/lib/resource_quota/memory_quota.cc +++ b/src/core/lib/resource_quota/memory_quota.cc @@ -203,8 +203,8 @@ Poll> ReclaimerQueue::PollNext() { // GrpcMemoryAllocatorImpl::GrpcMemoryAllocatorImpl( - std::shared_ptr memory_quota, std::string name) - : memory_quota_(memory_quota), name_(std::move(name)) { + std::shared_ptr memory_quota) + : memory_quota_(memory_quota) { memory_quota_->Take( /*allocator=*/this, taken_bytes_); memory_quota_->AddNewAllocator(this); @@ -314,8 +314,7 @@ void GrpcMemoryAllocatorImpl::MaybeDonateBack() { std::memory_order_acq_rel, std::memory_order_acquire)) { if (GRPC_TRACE_FLAG_ENABLED(grpc_resource_quota_trace)) { - gpr_log(GPR_INFO, "[%p|%s] Early return %" PRIdPTR " bytes", this, - name_.c_str(), ret); + gpr_log(GPR_INFO, "[%p] Early return %" PRIdPTR " bytes", this, ret); } GPR_ASSERT(taken_bytes_.fetch_sub(ret, std::memory_order_relaxed) >= ret); memory_quota_->Return(ret); @@ -735,15 +734,19 @@ double PressureTracker::AddSampleAndGetControlValue(double sample) { // MemoryQuota // -MemoryAllocator MemoryQuota::CreateMemoryAllocator(absl::string_view name) { - auto impl = std::make_shared( - memory_quota_, absl::StrCat(memory_quota_->name(), "/allocator/", name)); +MemoryAllocator MemoryQuota::CreateMemoryAllocator( + GRPC_UNUSED absl::string_view name) { + auto impl = std::make_shared(memory_quota_); return MemoryAllocator(std::move(impl)); } -MemoryOwner MemoryQuota::CreateMemoryOwner(absl::string_view name) { - auto impl = std::make_shared( - memory_quota_, absl::StrCat(memory_quota_->name(), "/owner/", name)); +MemoryOwner MemoryQuota::CreateMemoryOwner() { + // Note: we will likely want to add a name or some way to distinguish + // between memory owners once resource quota is fully rolled out and we need + // full metrics. One thing to note, however, is that manipulating the name + // here (e.g. concatenation) can add significant memory increase when many + // owners are created. + auto impl = std::make_shared(memory_quota_); return MemoryOwner(std::move(impl)); } diff --git a/src/core/lib/resource_quota/memory_quota.h b/src/core/lib/resource_quota/memory_quota.h index fa74f4b6762fb..a76f2594d206c 100644 --- a/src/core/lib/resource_quota/memory_quota.h +++ b/src/core/lib/resource_quota/memory_quota.h @@ -392,7 +392,7 @@ class BasicMemoryQuota final class GrpcMemoryAllocatorImpl final : public EventEngineMemoryAllocatorImpl { public: explicit GrpcMemoryAllocatorImpl( - std::shared_ptr memory_quota, std::string name); + std::shared_ptr memory_quota); ~GrpcMemoryAllocatorImpl() override; // Reserve bytes from the quota. @@ -445,9 +445,6 @@ class GrpcMemoryAllocatorImpl final : public EventEngineMemoryAllocatorImpl { return memory_quota_->GetPressureInfo(); } - // Name of this allocator - absl::string_view name() const { return name_; } - size_t GetFreeBytes() const { return free_bytes_.load(std::memory_order_relaxed); } @@ -494,9 +491,6 @@ class GrpcMemoryAllocatorImpl final : public EventEngineMemoryAllocatorImpl { OrphanablePtr reclamation_handles_[kNumReclamationPasses] ABSL_GUARDED_BY( reclaimer_mu_); - - // Name of this allocator. - std::string name_; }; // MemoryOwner is an enhanced MemoryAllocator that can also reclaim memory, and @@ -531,9 +525,6 @@ class MemoryOwner final : public MemoryAllocator { return OrphanablePtr(New(std::forward(args)...)); } - // Name of this object - absl::string_view name() const { return impl()->name(); } - // Is this object valid (ie has not been moved out of or reset) bool is_valid() const { return impl() != nullptr; } @@ -565,7 +556,7 @@ class MemoryQuota final MemoryQuota& operator=(MemoryQuota&&) = default; MemoryAllocator CreateMemoryAllocator(absl::string_view name) override; - MemoryOwner CreateMemoryOwner(absl::string_view name); + MemoryOwner CreateMemoryOwner(); // Resize the quota to new_size. void SetSize(size_t new_size) { memory_quota_->SetSize(new_size); } diff --git a/src/core/lib/security/transport/secure_endpoint.cc b/src/core/lib/security/transport/secure_endpoint.cc index 3155b29df4b3a..c67f0599fc8de 100644 --- a/src/core/lib/security/transport/secure_endpoint.cc +++ b/src/core/lib/security/transport/secure_endpoint.cc @@ -28,7 +28,6 @@ #include "absl/base/thread_annotations.h" #include "absl/status/status.h" -#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -85,11 +84,9 @@ struct secure_endpoint { grpc_core::CSliceRef(leftover_slices[i])); } grpc_slice_buffer_init(&output_buffer); - memory_owner = - grpc_core::ResourceQuotaFromChannelArgs(channel_args) - ->memory_quota() - ->CreateMemoryOwner(absl::StrCat(grpc_endpoint_get_peer(transport), - ":secure_endpoint")); + memory_owner = grpc_core::ResourceQuotaFromChannelArgs(channel_args) + ->memory_quota() + ->CreateMemoryOwner(); self_reservation = memory_owner.MakeReservation(sizeof(*this)); if (zero_copy_protector) { read_staging_buffer = grpc_empty_slice(); diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index 2338295f441cb..5df21b6745b8d 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -77,7 +77,7 @@ Channel::Channel(bool is_client, bool is_promising, std::string target, channelz_node_(channel_args.GetObjectRef()), allocator_(channel_args.GetObject() ->memory_quota() - ->CreateMemoryOwner(target)), + ->CreateMemoryOwner()), target_(std::move(target)), channel_stack_(std::move(channel_stack)) { // We need to make sure that grpc_shutdown() does not shut things down diff --git a/test/core/resource_quota/memory_quota_fuzzer.cc b/test/core/resource_quota/memory_quota_fuzzer.cc index 57e1b98ea5a04..aee653f1f2329 100644 --- a/test/core/resource_quota/memory_quota_fuzzer.cc +++ b/test/core/resource_quota/memory_quota_fuzzer.cc @@ -91,10 +91,9 @@ class Fuzzer { memory_quotas_.erase(action.quota()); break; case memory_quota_fuzzer::Action::kCreateAllocator: - WithQuota(action.quota(), [this, action, i](MemoryQuota* q) { - memory_allocators_.emplace( - action.allocator(), - q->CreateMemoryOwner(absl::StrCat("allocator-step-", i))); + WithQuota(action.quota(), [this, action](MemoryQuota* q) { + memory_allocators_.emplace(action.allocator(), + q->CreateMemoryOwner()); }); break; case memory_quota_fuzzer::Action::kDeleteAllocator: diff --git a/test/core/resource_quota/memory_quota_stress_test.cc b/test/core/resource_quota/memory_quota_stress_test.cc index 67af3ce3c2cb7..3cae0ee4bd234 100644 --- a/test/core/resource_quota/memory_quota_stress_test.cc +++ b/test/core/resource_quota/memory_quota_stress_test.cc @@ -51,8 +51,7 @@ class StressTest { std::random_device g; std::uniform_int_distribution dist(0, num_quotas - 1); for (size_t i = 0; i < num_allocators; ++i) { - allocators_.emplace_back(quotas_[dist(g)].CreateMemoryOwner( - absl::StrCat("allocator[", i, "]"))); + allocators_.emplace_back(quotas_[dist(g)].CreateMemoryOwner()); } } diff --git a/test/core/resource_quota/memory_quota_test.cc b/test/core/resource_quota/memory_quota_test.cc index 1c9d2fc62e710..150b617e3f0d8 100644 --- a/test/core/resource_quota/memory_quota_test.cc +++ b/test/core/resource_quota/memory_quota_test.cc @@ -82,7 +82,7 @@ TEST(MemoryQuotaTest, CreateSomeObjectsAndExpectReclamation) { MemoryQuota memory_quota("foo"); memory_quota.SetSize(4096); - auto memory_allocator = memory_quota.CreateMemoryOwner("bar"); + auto memory_allocator = memory_quota.CreateMemoryOwner(); auto object = memory_allocator.MakeUnique>(); auto checker1 = CallChecker::Make(); @@ -157,7 +157,7 @@ TEST(MemoryQuotaTest, NoBunchingIfIdle) { for (size_t i = 0; i < 10000; i++) { ExecCtx exec_ctx; - auto memory_owner = memory_quota.CreateMemoryOwner("bar"); + auto memory_owner = memory_quota.CreateMemoryOwner(); memory_owner.PostReclaimer( ReclamationPass::kDestructive, [&count_reclaimers_called](absl::optional sweep) { diff --git a/test/core/transport/chttp2/flow_control_fuzzer.cc b/test/core/transport/chttp2/flow_control_fuzzer.cc index 6b5fece0c248b..7c7ee41803992 100644 --- a/test/core/transport/chttp2/flow_control_fuzzer.cc +++ b/test/core/transport/chttp2/flow_control_fuzzer.cc @@ -134,7 +134,7 @@ class FlowControlFuzzer { } MemoryQuotaRefPtr memory_quota_ = MakeMemoryQuota("fuzzer"); - MemoryOwner memory_owner_ = memory_quota_->CreateMemoryOwner("owner"); + MemoryOwner memory_owner_ = memory_quota_->CreateMemoryOwner(); std::unique_ptr tfc_; absl::optional queued_initial_window_size_; absl::optional queued_send_max_frame_size_; diff --git a/test/core/transport/chttp2/flow_control_test.cc b/test/core/transport/chttp2/flow_control_test.cc index 786412b0165cb..8bf164044141b 100644 --- a/test/core/transport/chttp2/flow_control_test.cc +++ b/test/core/transport/chttp2/flow_control_test.cc @@ -83,7 +83,7 @@ class TransportTargetWindowEstimatesMocker class FlowControlTest : public ::testing::Test { protected: MemoryOwner memory_owner_ = MemoryOwner( - ResourceQuota::Default()->memory_quota()->CreateMemoryOwner("test")); + ResourceQuota::Default()->memory_quota()->CreateMemoryOwner()); }; TEST_F(FlowControlTest, NoOp) {