Enable the performance-* clang-tidy checks

.clang-tidy

@@ -1,6 +1,6 @@
 ---
-Checks: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size'
+Checks: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*'
-WarningsAsErrors: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size'
+WarningsAsErrors: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*'
 CheckOptions:
   - key:    readability-function-size.StatementThreshold
     value:  '450'

src/cpp/server/server_builder.cc

@@ -24,6 +24,8 @@
 #include <grpcpp/resource_quota.h>
 #include <grpcpp/server.h>
 
+#include <utility>
+
 #include "src/core/lib/gpr/useful.h"
 #include "src/cpp/server/thread_pool_interface.h"
 
@@ -166,7 +168,7 @@ ServerBuilder& ServerBuilder::AddListeningPort(
     while (addr_uri[pos] == '/') ++pos;  // Skip slashes.
     addr = addr_uri.substr(pos);
   }
-  Port port = {addr, creds, selected_port};
+  Port port = {addr, std::move(creds), selected_port};
   ports_.push_back(port);
   return *this;
 }

src/cpp/server/server_cc.cc

@@ -219,7 +219,7 @@ class Server::SyncRequest final : public internal::CompletionQueueTag {
       }
     }
 
-    void Run(std::shared_ptr<GlobalCallbacks> global_callbacks) {
+    void Run(const std::shared_ptr<GlobalCallbacks>& global_callbacks) {
       ctx_.BeginCompletionOp(&call_);
       global_callbacks->PreSynchronousRequest(&ctx_);
       method_->handler()->RunHandler(internal::MethodHandler::HandlerParameter(
@@ -272,7 +272,7 @@ class Server::SyncRequestThreadManager : public ThreadManager {
         server_(server),
         server_cq_(server_cq),
         cq_timeout_msec_(cq_timeout_msec),
-        global_callbacks_(global_callbacks) {}
+        global_callbacks_(std::move(global_callbacks)) {}
 
   WorkStatus PollForWork(void** tag, bool* ok) override {
     *tag = nullptr;
@@ -378,7 +378,7 @@ Server::Server(
         sync_server_cqs,
     int min_pollers, int max_pollers, int sync_cq_timeout_msec)
     : max_receive_message_size_(max_receive_message_size),
-      sync_server_cqs_(sync_server_cqs),
+      sync_server_cqs_(std::move(sync_server_cqs)),
       started_(false),
       shutdown_(false),
       shutdown_notified_(false),

test/core/channel/channel_trace_test.cc

@@ -77,13 +77,13 @@ void ValidateChannelTraceData(grpc_json* json,
   ValidateJsonArraySize(json, "events", actual_num_events_expected);
 }
 
-void AddSimpleTrace(RefCountedPtr<ChannelTrace> tracer) {
+void AddSimpleTrace(const RefCountedPtr<ChannelTrace>& tracer) {
   tracer->AddTraceEvent(ChannelTrace::Severity::Info,
                         grpc_slice_from_static_string("simple trace"));
 }
 
 // checks for the existence of all the required members of the tracer.
-void ValidateChannelTrace(RefCountedPtr<ChannelTrace> tracer,
+void ValidateChannelTrace(const RefCountedPtr<ChannelTrace>& tracer,
                           size_t expected_num_event_logged, size_t max_nodes) {
   if (!max_nodes) return;
   char* json_str = tracer->RenderTrace();
@@ -95,7 +95,8 @@ void ValidateChannelTrace(RefCountedPtr<ChannelTrace> tracer,
   gpr_free(json_str);
 }
 
-void ValidateTraceDataMatchedUuidLookup(RefCountedPtr<ChannelTrace> tracer) {
+void ValidateTraceDataMatchedUuidLookup(
+    const RefCountedPtr<ChannelTrace>& tracer) {
   intptr_t uuid = tracer->GetUuid();
   if (uuid == -1) return;  // Doesn't make sense to lookup if tracing disabled
   char* tracer_json_str = tracer->RenderTrace();

test/core/channel/channelz_registry_test.cc

@@ -54,6 +54,7 @@ TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) {
 TEST(ChannelzRegistryTest, UuidsAreIncreasing) {
   int object_to_register;
   std::vector<intptr_t> uuids;
+  uuids.reserve(10);
   for (int i = 0; i < 10; ++i) {
     // reregister the same object. It's ok since we are just testing uuids
     uuids.push_back(ChannelzRegistry::Register(&object_to_register));

test/core/gprpp/ref_counted_ptr_test.cc

@@ -66,14 +66,14 @@ TEST(RefCountedPtr, MoveAssignment) {
 
 TEST(RefCountedPtr, CopyConstructor) {
   RefCountedPtr<Foo> foo(New<Foo>());
-  RefCountedPtr<Foo> foo2(foo);
+  const RefCountedPtr<Foo>& foo2(foo);
   EXPECT_NE(nullptr, foo.get());
   EXPECT_EQ(foo.get(), foo2.get());
 }
 
 TEST(RefCountedPtr, CopyAssignment) {
   RefCountedPtr<Foo> foo(New<Foo>());
-  RefCountedPtr<Foo> foo2 = foo;
+  const RefCountedPtr<Foo>& foo2 = foo;
   EXPECT_NE(nullptr, foo.get());
   EXPECT_EQ(foo.get(), foo2.get());
 }

test/cpp/end2end/async_end2end_test.cc

@@ -142,7 +142,7 @@ class Verifier {
   // to call the lambda
   void Verify(CompletionQueue* cq,
               std::chrono::system_clock::time_point deadline,
-              std::function<void(void)> lambda) {
+              const std::function<void(void)>& lambda) {
     if (expectations_.empty()) {
       bool ok;
       void* got_tag;

test/cpp/end2end/client_lb_end2end_test.cc

@@ -686,9 +686,11 @@ TEST_F(ClientLbEnd2endTest, RoundRobinReresolve) {
   const int kNumServers = 3;
   std::vector<int> first_ports;
   std::vector<int> second_ports;
+  first_ports.reserve(kNumServers);
   for (int i = 0; i < kNumServers; ++i) {
     first_ports.push_back(grpc_pick_unused_port_or_die());
   }
+  second_ports.reserve(kNumServers);
   for (int i = 0; i < kNumServers; ++i) {
     second_ports.push_back(grpc_pick_unused_port_or_die());
   }

test/cpp/end2end/end2end_test.cc

@@ -71,8 +71,8 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
   static const char kGoodMetadataKey[];
   static const char kBadMetadataKey[];
 
-  TestMetadataCredentialsPlugin(grpc::string_ref metadata_key,
+  TestMetadataCredentialsPlugin(const grpc::string_ref& metadata_key,
-                                grpc::string_ref metadata_value,
+                                const grpc::string_ref& metadata_value,
                                 bool is_blocking, bool is_successful)
       : metadata_key_(metadata_key.data(), metadata_key.length()),
         metadata_value_(metadata_value.data(), metadata_value.length()),
@@ -168,7 +168,7 @@ const char TestAuthMetadataProcessor::kIdentityPropName[] = "novel identity";
 
 class Proxy : public ::grpc::testing::EchoTestService::Service {
  public:
-  Proxy(std::shared_ptr<Channel> channel)
+  Proxy(const std::shared_ptr<Channel>& channel)
       : stub_(grpc::testing::EchoTestService::NewStub(channel)) {}
 
   Status Echo(ServerContext* server_context, const EchoRequest* request,
@@ -683,6 +683,7 @@ TEST_P(End2endTest, SimpleRpcWithCustomUserAgentPrefix) {
 TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) {
   ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(10);
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(SendRpc, stub_.get(), 10, true);
   }
@@ -694,6 +695,7 @@ TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) {
 TEST_P(End2endTest, MultipleRpcs) {
   ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(10);
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(SendRpc, stub_.get(), 10, false);
   }
@@ -1272,6 +1274,7 @@ TEST_P(ProxyEnd2endTest, SimpleRpcWithEmptyMessages) {
 TEST_P(ProxyEnd2endTest, MultipleRpcs) {
   ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(10);
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(SendRpc, stub_.get(), 10, false);
   }

test/cpp/end2end/mock_test.cc

@@ -186,7 +186,7 @@ class TestServiceImpl : public EchoTestService::Service {
                         ServerWriter<EchoResponse>* writer) override {
     EchoResponse response;
     vector<grpc::string> tokens = split(request->message());
-    for (grpc::string token : tokens) {
+    for (const grpc::string& token : tokens) {
       response.set_message(token);
       writer->Write(response);
     }

test/cpp/end2end/thread_stress_test.cc

@@ -322,6 +322,7 @@ TYPED_TEST_CASE(End2endTest, CommonTypes);
 TYPED_TEST(End2endTest, ThreadStress) {
   this->common_.ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(kNumThreads);
   for (int i = 0; i < kNumThreads; ++i) {
     threads.emplace_back(SendRpc, this->common_.GetStub(), kNumRpcs);
   }

test/cpp/interop/interop_client.cc

@@ -19,6 +19,7 @@
 #include <cinttypes>
 #include <fstream>
 #include <memory>
+#include <utility>
 
 #include <grpc/grpc.h>
 #include <grpc/support/alloc.h>
@@ -73,7 +74,7 @@ void UnaryCompressionChecks(const InteropClientContextInspector& inspector,
 }
 }  // namespace
 
-InteropClient::ServiceStub::ServiceStub(std::shared_ptr<Channel> channel,
+InteropClient::ServiceStub::ServiceStub(const std::shared_ptr<Channel>& channel,
                                         bool new_stub_every_call)
     : channel_(channel), new_stub_every_call_(new_stub_every_call) {
   // If new_stub_every_call is false, then this is our chance to initialize
@@ -99,7 +100,8 @@ InteropClient::ServiceStub::GetUnimplementedServiceStub() {
   return unimplemented_service_stub_.get();
 }
 
-void InteropClient::ServiceStub::Reset(std::shared_ptr<Channel> channel) {
+void InteropClient::ServiceStub::Reset(
+    const std::shared_ptr<Channel>& channel) {
   channel_ = channel;
 
   // Update stub_ as well. Note: If new_stub_every_call_ is true, we can reset
@@ -112,13 +114,13 @@ void InteropClient::ServiceStub::Reset(std::shared_ptr<Channel> channel) {
 }
 
 void InteropClient::Reset(std::shared_ptr<Channel> channel) {
-  serviceStub_.Reset(channel);
+  serviceStub_.Reset(std::move(channel));
 }
 
 InteropClient::InteropClient(std::shared_ptr<Channel> channel,
                              bool new_stub_every_test_case,
                              bool do_not_abort_on_transient_failures)
-    : serviceStub_(channel, new_stub_every_test_case),
+    : serviceStub_(std::move(channel), new_stub_every_test_case),
       do_not_abort_on_transient_failures_(do_not_abort_on_transient_failures) {}
 
 bool InteropClient::AssertStatusOk(const Status& s,
@@ -180,7 +182,7 @@ bool InteropClient::PerformLargeUnary(SimpleRequest* request,
 
 bool InteropClient::PerformLargeUnary(SimpleRequest* request,
                                       SimpleResponse* response,
-                                      CheckerFn custom_checks_fn) {
+                                      const CheckerFn& custom_checks_fn) {
   ClientContext context;
   InteropClientContextInspector inspector(context);
   request->set_response_size(kLargeResponseSize);

test/cpp/interop/interop_client.h

@@ -83,12 +83,13 @@ class InteropClient {
    public:
     // If new_stub_every_call = true, pointer to a new instance of
     // TestServce::Stub is returned by Get() everytime it is called
-    ServiceStub(std::shared_ptr<Channel> channel, bool new_stub_every_call);
+    ServiceStub(const std::shared_ptr<Channel>& channel,
+                bool new_stub_every_call);
 
     TestService::Stub* Get();
     UnimplementedService::Stub* GetUnimplementedServiceStub();
 
-    void Reset(std::shared_ptr<Channel> channel);
+    void Reset(const std::shared_ptr<Channel>& channel);
 
    private:
     std::unique_ptr<TestService::Stub> stub_;
@@ -102,7 +103,7 @@ class InteropClient {
 
   /// Run \a custom_check_fn as an additional check.
   bool PerformLargeUnary(SimpleRequest* request, SimpleResponse* response,
-                         CheckerFn custom_checks_fn);
+                         const CheckerFn& custom_checks_fn);
   bool AssertStatusOk(const Status& s,
                       const grpc::string& optional_debug_string);
   bool AssertStatusCode(const Status& s, StatusCode expected_code,

test/cpp/interop/stress_interop_client.cc

@@ -68,7 +68,7 @@ TestCaseType WeightedRandomTestSelector::GetNextTest() const {
 
 StressTestInteropClient::StressTestInteropClient(
     int test_id, const grpc::string& server_address,
-    std::shared_ptr<Channel> channel,
+    const std::shared_ptr<Channel>& channel,
     const WeightedRandomTestSelector& test_selector, long test_duration_secs,
     long sleep_duration_ms, bool do_not_abort_on_transient_failures)
     : test_id_(test_id),
@@ -80,7 +80,8 @@ StressTestInteropClient::StressTestInteropClient(
       test_duration_secs_(test_duration_secs),
       sleep_duration_ms_(sleep_duration_ms) {}
 
-void StressTestInteropClient::MainLoop(std::shared_ptr<QpsGauge> qps_gauge) {
+void StressTestInteropClient::MainLoop(
+    const std::shared_ptr<QpsGauge>& qps_gauge) {
   gpr_log(GPR_INFO, "Running test %d. ServerAddr: %s", test_id_,
           server_address_.c_str());
 

test/cpp/interop/stress_interop_client.h

@@ -91,14 +91,14 @@ class WeightedRandomTestSelector {
 class StressTestInteropClient {
  public:
   StressTestInteropClient(int test_id, const grpc::string& server_address,
-                          std::shared_ptr<Channel> channel,
+                          const std::shared_ptr<Channel>& channel,
                           const WeightedRandomTestSelector& test_selector,
                           long test_duration_secs, long sleep_duration_ms,
                           bool do_not_abort_on_transient_failures);
 
   // The main function. Use this as the thread entry point.
   // qps_gauge is the QpsGauge to record the requests per second metric
-  void MainLoop(std::shared_ptr<QpsGauge> qps_gauge);
+  void MainLoop(const std::shared_ptr<QpsGauge>& qps_gauge);
 
  private:
   bool RunTest(TestCaseType test_case);

test/cpp/microbenchmarks/bm_chttp2_hpack.cc

@@ -208,6 +208,7 @@ class SingleInternedBinaryElem {
  private:
   static grpc_slice MakeBytes() {
     std::vector<char> v;
+    v.reserve(kLength);
     for (int i = 0; i < kLength; i++) {
       v.push_back(static_cast<char>(rand()));
     }
@@ -246,6 +247,7 @@ class SingleNonInternedBinaryElem {
  private:
   static grpc_slice MakeBytes() {
     std::vector<char> v;
+    v.reserve(kLength);
     for (int i = 0; i < kLength; i++) {
       v.push_back(static_cast<char>(rand()));
     }

test/cpp/qps/client_async.cc

@@ -24,6 +24,7 @@
 #include <sstream>
 #include <string>
 #include <thread>
+#include <utility>
 #include <vector>
 
 #include <grpc/grpc.h>
@@ -78,7 +79,7 @@ class ClientRpcContextUnaryImpl : public ClientRpcContext {
         response_(),
         next_state_(State::READY),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req) {}
   ~ClientRpcContextUnaryImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
@@ -326,7 +327,7 @@ class AsyncUnaryClient final
                                     std::function<gpr_timespec()> next_issue,
                                     const SimpleRequest& req) {
     return new ClientRpcContextUnaryImpl<SimpleRequest, SimpleResponse>(
-        stub, req, next_issue, AsyncUnaryClient::PrepareReq,
+        stub, req, std::move(next_issue), AsyncUnaryClient::PrepareReq,
         AsyncUnaryClient::CheckDone);
   }
 };
@@ -349,7 +350,7 @@ class ClientRpcContextStreamingPingPongImpl : public ClientRpcContext {
         response_(),
         next_state_(State::INVALID),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req),
         coalesce_(false) {}
   ~ClientRpcContextStreamingPingPongImpl() override {}
@@ -510,7 +511,8 @@ class AsyncStreamingPingPongClient final
                                     const SimpleRequest& req) {
     return new ClientRpcContextStreamingPingPongImpl<SimpleRequest,
                                                      SimpleResponse>(
-        stub, req, next_issue, AsyncStreamingPingPongClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        AsyncStreamingPingPongClient::PrepareReq,
         AsyncStreamingPingPongClient::CheckDone);
   }
 };
@@ -533,7 +535,7 @@ class ClientRpcContextStreamingFromClientImpl : public ClientRpcContext {
         response_(),
         next_state_(State::INVALID),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req) {}
   ~ClientRpcContextStreamingFromClientImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
@@ -640,7 +642,8 @@ class AsyncStreamingFromClientClient final
                                     const SimpleRequest& req) {
     return new ClientRpcContextStreamingFromClientImpl<SimpleRequest,
                                                        SimpleResponse>(
-        stub, req, next_issue, AsyncStreamingFromClientClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        AsyncStreamingFromClientClient::PrepareReq,
         AsyncStreamingFromClientClient::CheckDone);
   }
 };
@@ -663,7 +666,7 @@ class ClientRpcContextStreamingFromServerImpl : public ClientRpcContext {
         response_(),
         next_state_(State::INVALID),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req) {}
   ~ClientRpcContextStreamingFromServerImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
@@ -754,7 +757,8 @@ class AsyncStreamingFromServerClient final
                                     const SimpleRequest& req) {
     return new ClientRpcContextStreamingFromServerImpl<SimpleRequest,
                                                        SimpleResponse>(
-        stub, req, next_issue, AsyncStreamingFromServerClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        AsyncStreamingFromServerClient::PrepareReq,
         AsyncStreamingFromServerClient::CheckDone);
   }
 };
@@ -775,9 +779,9 @@ class ClientRpcContextGenericStreamingImpl : public ClientRpcContext {
         req_(req),
         response_(),
         next_state_(State::INVALID),
-        callback_(on_done),
+        callback_(std::move(on_done)),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
-        prepare_req_(prepare_req) {}
+        prepare_req_(std::move(prepare_req)) {}
   ~ClientRpcContextGenericStreamingImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
     GPR_ASSERT(!config.use_coalesce_api());  // not supported yet.
@@ -918,7 +922,8 @@ class GenericAsyncStreamingClient final
                                     std::function<gpr_timespec()> next_issue,
                                     const ByteBuffer& req) {
     return new ClientRpcContextGenericStreamingImpl(
-        stub, req, next_issue, GenericAsyncStreamingClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        GenericAsyncStreamingClient::PrepareReq,
         GenericAsyncStreamingClient::CheckDone);
   }
 };

test/cpp/qps/client_sync.cc

@@ -192,7 +192,7 @@ class SynchronousStreamingClient : public SynchronousClient {
     new (&context_[thread_idx]) ClientContext();
   }
 
-  void CleanupAllStreams(std::function<void(size_t)> cleaner) {
+  void CleanupAllStreams(const std::function<void(size_t)>& cleaner) {
     std::vector<std::thread> cleanup_threads;
     for (size_t i = 0; i < num_threads_; i++) {
       cleanup_threads.emplace_back([this, i, cleaner] {

test/cpp/qps/driver.cc

@@ -96,16 +96,20 @@ static deque<string> get_workers(const string& env_name) {
 }
 
 // helpers for postprocess_scenario_result
-static double WallTime(ClientStats s) { return s.time_elapsed(); }
+static double WallTime(const ClientStats& s) { return s.time_elapsed(); }
-static double SystemTime(ClientStats s) { return s.time_system(); }
+static double SystemTime(const ClientStats& s) { return s.time_system(); }
-static double UserTime(ClientStats s) { return s.time_user(); }
+static double UserTime(const ClientStats& s) { return s.time_user(); }
-static double CliPollCount(ClientStats s) { return s.cq_poll_count(); }
+static double CliPollCount(const ClientStats& s) { return s.cq_poll_count(); }
-static double SvrPollCount(ServerStats s) { return s.cq_poll_count(); }
+static double SvrPollCount(const ServerStats& s) { return s.cq_poll_count(); }
-static double ServerWallTime(ServerStats s) { return s.time_elapsed(); }
+static double ServerWallTime(const ServerStats& s) { return s.time_elapsed(); }
-static double ServerSystemTime(ServerStats s) { return s.time_system(); }
+static double ServerSystemTime(const ServerStats& s) { return s.time_system(); }
-static double ServerUserTime(ServerStats s) { return s.time_user(); }
+static double ServerUserTime(const ServerStats& s) { return s.time_user(); }
-static double ServerTotalCpuTime(ServerStats s) { return s.total_cpu_time(); }
+static double ServerTotalCpuTime(const ServerStats& s) {
-static double ServerIdleCpuTime(ServerStats s) { return s.idle_cpu_time(); }
+  return s.total_cpu_time();
+}
+static double ServerIdleCpuTime(const ServerStats& s) {
+  return s.idle_cpu_time();
+}
 static int Cores(int n) { return n; }
 
 // Postprocess ScenarioResult and populate result summary.
@@ -156,7 +160,7 @@ static void postprocess_scenario_result(ScenarioResult* result) {
     int64_t successes = 0;
     int64_t failures = 0;
     for (int i = 0; i < result->request_results_size(); i++) {
-      RequestResultCount rrc = result->request_results(i);
+      const RequestResultCount& rrc = result->request_results(i);
       if (rrc.status_code() == 0) {
         successes += rrc.count();
       } else {
@@ -213,7 +217,7 @@ std::unique_ptr<ScenarioResult> RunScenario(
   // To be added to the result, containing the final configuration used for
   // client and config (including host, etc.)
   ClientConfig result_client_config;
-  const ServerConfig result_server_config = initial_server_config;
+  const ServerConfig& result_server_config = initial_server_config;
 
   // Get client, server lists; ignore if inproc test
   auto workers = (!run_inproc) ? get_workers("QPS_WORKERS") : deque<string>();
@@ -280,7 +284,7 @@ std::unique_ptr<ScenarioResult> RunScenario(
           local_workers[i]->InProcessChannel(channel_args));
     }
 
-    ServerConfig server_config = initial_server_config;
+    const ServerConfig& server_config = initial_server_config;
     if (server_config.core_limit() != 0) {
       gpr_log(GPR_ERROR,
               "server config core limit is set but ignored by driver");

test/cpp/qps/qps_interarrival_test.cc

@@ -28,7 +28,8 @@
 using grpc::testing::InterarrivalTimer;
 using grpc::testing::RandomDistInterface;
 
-static void RunTest(RandomDistInterface&& r, int threads, std::string title) {
+static void RunTest(RandomDistInterface&& r, int threads,
+                    const std::string& title) {
   InterarrivalTimer timer;
   timer.init(r, threads);
   grpc_histogram* h(grpc_histogram_create(0.01, 60e9));

test/cpp/qps/report.h

@@ -129,7 +129,7 @@ class JsonReporter : public Reporter {
 
 class RpcReporter : public Reporter {
  public:
-  RpcReporter(const string& name, std::shared_ptr<grpc::Channel> channel)
+  RpcReporter(const string& name, const std::shared_ptr<grpc::Channel>& channel)
       : Reporter(name), stub_(ReportQpsScenarioService::NewStub(channel)) {}
 
  private:

test/cpp/qps/server_sync.cc

@@ -129,7 +129,7 @@ class BenchmarkServiceImpl final : public BenchmarkService::Service {
   template <class W>
   static Status ServerPush(ServerContext* context, W* stream,
                            const SimpleResponse& response,
-                           std::function<bool()> done) {
+                           const std::function<bool()>& done) {
     while ((done == nullptr) || !done()) {
       // TODO(vjpai): Add potential for rate-pacing on this
       if (!stream->Write(response)) {

test/cpp/server/load_reporter/load_data_store_test.cc

@@ -50,8 +50,8 @@ class LoadDataStoreTest : public ::testing::Test {
   bool PerBalancerStoresContains(
       const LoadDataStore& load_data_store,
       const std::set<PerBalancerStore*>* per_balancer_stores,
-      const grpc::string hostname, const grpc::string lb_id,
+      const grpc::string& hostname, const grpc::string& lb_id,
-      const grpc::string load_key) {
+      const grpc::string& load_key) {
     auto original_per_balancer_store =
         load_data_store.FindPerBalancerStore(hostname, lb_id);
     EXPECT_NE(original_per_balancer_store, nullptr);
@@ -155,7 +155,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) {
   active_lb_ids.erase(orphaned_lb_id);
   // Find which LB is assigned the orphaned store.
   grpc::string assigned_lb_id = "";
-  for (auto lb_id : active_lb_ids) {
+  for (const auto& lb_id : active_lb_ids) {
     if (PerBalancerStoresContains(
             load_data_store,
             load_data_store.GetAssignedStores(kHostname1, lb_id), kHostname1,
@@ -169,7 +169,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) {
   // orphaned_lb_id shouldn't change.
   for (size_t _ = 0; _ < 10; ++_) {
     grpc::string lb_id_to_close = "";
-    for (auto lb_id : active_lb_ids) {
+    for (const auto& lb_id : active_lb_ids) {
       if (lb_id != assigned_lb_id) {
         lb_id_to_close = lb_id;
         break;
@@ -187,7 +187,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) {
   load_data_store.ReportStreamClosed(kHostname1, assigned_lb_id);
   active_lb_ids.erase(assigned_lb_id);
   size_t orphaned_lb_id_occurences = 0;
-  for (auto lb_id : active_lb_ids) {
+  for (const auto& lb_id : active_lb_ids) {
     if (PerBalancerStoresContains(
             load_data_store,
             load_data_store.GetAssignedStores(kHostname1, lb_id), kHostname1,

test/cpp/util/byte_buffer_test.cc

@@ -41,7 +41,7 @@ class ByteBufferTest : public ::testing::Test {};
 TEST_F(ByteBufferTest, CopyCtor) {
   ByteBuffer buffer1;
   EXPECT_FALSE(buffer1.Valid());
-  ByteBuffer buffer2 = buffer1;
+  const ByteBuffer& buffer2 = buffer1;
   EXPECT_FALSE(buffer2.Valid());
 }
 

test/cpp/util/cli_call.cc

@@ -19,6 +19,7 @@
 #include "test/cpp/util/cli_call.h"
 
 #include <iostream>
+#include <utility>
 
 #include <grpc/grpc.h>
 #include <grpc/slice.h>
@@ -39,7 +40,7 @@ Status CliCall::Call(std::shared_ptr<grpc::Channel> channel,
                      const OutgoingMetadataContainer& metadata,
                      IncomingMetadataContainer* server_initial_metadata,
                      IncomingMetadataContainer* server_trailing_metadata) {
-  CliCall call(channel, method, metadata);
+  CliCall call(std::move(channel), method, metadata);
   call.Write(request);
   call.WritesDone();
   if (!call.Read(response, server_initial_metadata)) {
@@ -48,7 +49,7 @@ Status CliCall::Call(std::shared_ptr<grpc::Channel> channel,
   return call.Finish(server_trailing_metadata);
 }
 
-CliCall::CliCall(std::shared_ptr<grpc::Channel> channel,
+CliCall::CliCall(const std::shared_ptr<grpc::Channel>& channel,
                  const grpc::string& method,
                  const OutgoingMetadataContainer& metadata)
     : stub_(new grpc::GenericStub(channel)) {

test/cpp/util/cli_call.h

@@ -42,7 +42,8 @@ class CliCall final {
   typedef std::multimap<grpc::string_ref, grpc::string_ref>
       IncomingMetadataContainer;
 
-  CliCall(std::shared_ptr<grpc::Channel> channel, const grpc::string& method,
+  CliCall(const std::shared_ptr<grpc::Channel>& channel,
+          const grpc::string& method,
           const OutgoingMetadataContainer& metadata);
   ~CliCall();
 

test/cpp/util/proto_file_parser.cc

@@ -63,7 +63,7 @@ class ErrorPrinter : public protobuf::compiler::MultiFileErrorCollector {
   ProtoFileParser* parser_;  // not owned
 };
 
-ProtoFileParser::ProtoFileParser(std::shared_ptr<grpc::Channel> channel,
+ProtoFileParser::ProtoFileParser(const std::shared_ptr<grpc::Channel>& channel,
                                  const grpc::string& proto_path,
                                  const grpc::string& protofiles)
     : has_error_(false),

test/cpp/util/proto_file_parser.h

@@ -36,7 +36,7 @@ class ProtoFileParser {
   // The parser will search proto files using the server reflection service
   // provided on the given channel. The given protofiles in a source tree rooted
   // from proto_path will also be searched.
-  ProtoFileParser(std::shared_ptr<grpc::Channel> channel,
+  ProtoFileParser(const std::shared_ptr<grpc::Channel>& channel,
                   const grpc::string& proto_path,
                   const grpc::string& protofiles);
 

test/cpp/util/proto_reflection_descriptor_database.cc

@@ -35,7 +35,7 @@ ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase(
     : stub_(std::move(stub)) {}
 
 ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase(
-    std::shared_ptr<grpc::Channel> channel)
+    const std::shared_ptr<grpc::Channel>& channel)
     : stub_(ServerReflection::NewStub(channel)) {}
 
 ProtoReflectionDescriptorDatabase::~ProtoReflectionDescriptorDatabase() {
@@ -79,7 +79,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileByName(
     AddFileFromResponse(response.file_descriptor_response());
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       gpr_log(GPR_INFO, "NOT_FOUND from server for FindFileByName(%s)",
               filename.c_str());
@@ -126,7 +126,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileContainingSymbol(
     AddFileFromResponse(response.file_descriptor_response());
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       missing_symbols_.insert(symbol_name);
       gpr_log(GPR_INFO,
@@ -182,7 +182,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileContainingExtension(
     AddFileFromResponse(response.file_descriptor_response());
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       if (missing_extensions_.find(containing_type) ==
           missing_extensions_.end()) {
@@ -238,7 +238,7 @@ bool ProtoReflectionDescriptorDatabase::FindAllExtensionNumbers(
     return true;
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       gpr_log(GPR_INFO, "NOT_FOUND from server for FindAllExtensionNumbers(%s)",
               extendee_type.c_str());
@@ -265,14 +265,14 @@ bool ProtoReflectionDescriptorDatabase::GetServices(
 
   if (response.message_response_case() ==
       ServerReflectionResponse::MessageResponseCase::kListServicesResponse) {
-    const ListServiceResponse ls_response = response.list_services_response();
+    const ListServiceResponse& ls_response = response.list_services_response();
     for (int i = 0; i < ls_response.service_size(); ++i) {
       (*output).push_back(ls_response.service(i).name());
     }
     return true;
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     gpr_log(GPR_INFO,
             "Error on GetServices()\n\tError code: %d\n"
             "\tError Message: %s",

test/cpp/util/proto_reflection_descriptor_database.h

@@ -38,7 +38,7 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase {
       std::unique_ptr<reflection::v1alpha::ServerReflection::Stub> stub);
 
   explicit ProtoReflectionDescriptorDatabase(
-      std::shared_ptr<grpc::Channel> channel);
+      const std::shared_ptr<grpc::Channel>& channel);
 
   virtual ~ProtoReflectionDescriptorDatabase();
 

test/cpp/util/string_ref_test.cc

@@ -60,7 +60,7 @@ TEST_F(StringRefTest, FromString) {
 TEST_F(StringRefTest, CopyConstructor) {
   string_ref s1(kTestString);
   ;
-  string_ref s2(s1);
+  const string_ref& s2(s1);
   EXPECT_EQ(s1.length(), s2.length());
   EXPECT_EQ(s1.data(), s2.data());
 }