reviewer feedback

src/core/lib/channel/channelz_registry.cc

@@ -59,7 +59,7 @@ intptr_t ChannelzRegistry::InternalRegister(BaseNode* node) {
   return ++uuid_generator_;
 }
 
-void ChannelzRegistry::MaybePerformCompaction() {
+void ChannelzRegistry::MaybePerformCompactionLocked() {
   constexpr double kEmptinessTheshold = 1 / 3;
   double emptiness_ratio =
       double(num_empty_slots_) / double(entities_.capacity());
@@ -77,7 +77,7 @@ void ChannelzRegistry::MaybePerformCompaction() {
   }
 }
 
-int ChannelzRegistry::FindByUuid(intptr_t target_uuid) {
+int ChannelzRegistry::FindByUuidLocked(intptr_t target_uuid) {
   size_t left = 0;
   size_t right = entities_.size() - 1;
   while (left <= right) {
@@ -107,11 +107,11 @@ void ChannelzRegistry::InternalUnregister(intptr_t uuid) {
   GPR_ASSERT(uuid >= 1);
   MutexLock lock(&mu_);
   GPR_ASSERT(uuid <= uuid_generator_);
-  int idx = FindByUuid(uuid);
+  int idx = FindByUuidLocked(uuid);
   GPR_ASSERT(idx >= 0);
   entities_[idx] = nullptr;
   num_empty_slots_++;
-  MaybePerformCompaction();
+  MaybePerformCompactionLocked();
 }
 
 BaseNode* ChannelzRegistry::InternalGet(intptr_t uuid) {
@@ -119,7 +119,7 @@ BaseNode* ChannelzRegistry::InternalGet(intptr_t uuid) {
   if (uuid < 1 || uuid > uuid_generator_) {
     return nullptr;
   }
-  int idx = FindByUuid(uuid);
+  int idx = FindByUuidLocked(uuid);
   return idx < 0 ? nullptr : entities_[idx];
 }
 

src/core/lib/channel/channelz_registry.h

@@ -87,12 +87,12 @@ class ChannelzRegistry {
   char* InternalGetTopChannels(intptr_t start_channel_id);
   char* InternalGetServers(intptr_t start_server_id);
 
-  // If entities_ has a over a certain threshold of empty slots, it will
+  // If entities_ has over a certain threshold of empty slots, it will
   // compact the vector and move all used slots to the front.
-  void MaybePerformCompaction();
+  void MaybePerformCompactionLocked();
 
   // Performs binary search on entities_ to find the index with that uuid.
-  int FindByUuid(intptr_t uuid);
+  int FindByUuidLocked(intptr_t uuid);
 
   // protects members
   gpr_mu mu_;

test/core/channel/channelz_registry_test.cc

@@ -62,8 +62,8 @@ class ChannelzRegistryTest : public ::testing::Test {
 };
 
 TEST_F(ChannelzRegistryTest, UuidStartsAboveZeroTest) {
-  UniquePtr<BaseNode> channelz_channel(
-      new BaseNode(BaseNode::EntityType::kTopLevelChannel));
+  UniquePtr<BaseNode> channelz_channel =
+      MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel);
   intptr_t uuid = channelz_channel->uuid();
   EXPECT_GT(uuid, 0) << "First uuid chose must be greater than zero. Zero if "
                         "reserved according to "
@@ -75,8 +75,8 @@ TEST_F(ChannelzRegistryTest, UuidsAreIncreasing) {
   std::vector<UniquePtr<BaseNode>> channelz_channels;
   channelz_channels.reserve(10);
   for (int i = 0; i < 10; ++i) {
-    channelz_channels.push_back(UniquePtr<BaseNode>(
-        New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
+    channelz_channels.push_back(
+        MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
   }
   for (size_t i = 1; i < channelz_channels.size(); ++i) {
     EXPECT_LT(channelz_channels[i - 1]->uuid(), channelz_channels[i]->uuid())
@@ -85,8 +85,8 @@ TEST_F(ChannelzRegistryTest, UuidsAreIncreasing) {
 }
 
 TEST_F(ChannelzRegistryTest, RegisterGetTest) {
-  UniquePtr<BaseNode> channelz_channel(
-      New<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
+  UniquePtr<BaseNode> channelz_channel =
+      MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel);
   BaseNode* retrieved = ChannelzRegistry::Get(channelz_channel->uuid());
   EXPECT_EQ(channelz_channel.get(), retrieved);
 }
@@ -94,16 +94,16 @@ TEST_F(ChannelzRegistryTest, RegisterGetTest) {
 TEST_F(ChannelzRegistryTest, RegisterManyItems) {
   std::vector<UniquePtr<BaseNode>> channelz_channels;
   for (int i = 0; i < 100; i++) {
-    channelz_channels.push_back(UniquePtr<BaseNode>(
-        New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
+    channelz_channels.push_back(
+        MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
     BaseNode* retrieved = ChannelzRegistry::Get(channelz_channels[i]->uuid());
     EXPECT_EQ(channelz_channels[i].get(), retrieved);
   }
 }
 
 TEST_F(ChannelzRegistryTest, NullIfNotPresentTest) {
-  UniquePtr<BaseNode> channelz_channel(
-      New<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
+  UniquePtr<BaseNode> channelz_channel =
+      MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel);
   // try to pull out a uuid that does not exist.
   BaseNode* nonexistant = ChannelzRegistry::Get(channelz_channel->uuid() + 1);
   EXPECT_EQ(nonexistant, nullptr);
@@ -121,10 +121,10 @@ TEST_F(ChannelzRegistryTest, TestCompaction) {
     std::vector<UniquePtr<BaseNode>> odd_channels;
     odd_channels.reserve(kLoopIterations);
     for (int i = 0; i < kLoopIterations; i++) {
-      even_channels.push_back(UniquePtr<BaseNode>(
-          New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
-      odd_channels.push_back(UniquePtr<BaseNode>(
-          New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
+      even_channels.push_back(
+          MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
+      odd_channels.push_back(
+          MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
     }
   }
   // without compaction, there would be exactly kLoopIterations empty slots at
@@ -146,10 +146,10 @@ TEST_F(ChannelzRegistryTest, TestGetAfterCompaction) {
     std::vector<UniquePtr<BaseNode>> odd_channels;
     odd_channels.reserve(kLoopIterations);
     for (int i = 0; i < kLoopIterations; i++) {
-      even_channels.push_back(UniquePtr<BaseNode>(
-          New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
-      odd_channels.push_back(UniquePtr<BaseNode>(
-          New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
+      even_channels.push_back(
+          MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
+      odd_channels.push_back(
+          MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
       odd_uuids.push_back(odd_channels[i]->uuid());
     }
   }
@@ -173,18 +173,18 @@ TEST_F(ChannelzRegistryTest, TestAddAfterCompaction) {
     std::vector<UniquePtr<BaseNode>> odd_channels;
     odd_channels.reserve(kLoopIterations);
     for (int i = 0; i < kLoopIterations; i++) {
-      even_channels.push_back(UniquePtr<BaseNode>(
-          New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
-      odd_channels.push_back(UniquePtr<BaseNode>(
-          New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
+      even_channels.push_back(
+          MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
+      odd_channels.push_back(
+          MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
       odd_uuids.push_back(odd_channels[i]->uuid());
     }
   }
   std::vector<UniquePtr<BaseNode>> more_channels;
   more_channels.reserve(kLoopIterations);
   for (int i = 0; i < kLoopIterations; i++) {
-    more_channels.push_back(UniquePtr<BaseNode>(
-        New<BaseNode>(BaseNode::EntityType::kTopLevelChannel)));
+    more_channels.push_back(
+        MakeUnique<BaseNode>(BaseNode::EntityType::kTopLevelChannel));
     BaseNode* retrieved = ChannelzRegistry::Get(more_channels[i]->uuid());
     EXPECT_EQ(more_channels[i].get(), retrieved);
   }