diff options
31 files changed, 105 insertions, 77 deletions
diff --git a/.clang-tidy b/.clang-tidy index b8f74e973f..fbf0b6541e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,6 +1,6 @@ --- -Checks: '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' +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,performance-*' CheckOptions: - key: readability-function-size.StatementThreshold value: '450' diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 7f5b439dfb..5d1919cd1d 100644 --- a/src/cpp/server/server_builder.cc +++ b/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; } diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 391ca44962..0d77510e29 100644 --- a/src/cpp/server/server_cc.cc +++ b/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), diff --git a/test/core/channel/channel_trace_test.cc b/test/core/channel/channel_trace_test.cc index 64de05bc0a..d99a32d91d 100644 --- a/test/core/channel/channel_trace_test.cc +++ b/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(); diff --git a/test/core/channel/channelz_registry_test.cc b/test/core/channel/channelz_registry_test.cc index 37696dc0e8..eb6305eb4e 100644 --- a/test/core/channel/channelz_registry_test.cc +++ b/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)); diff --git a/test/core/gprpp/ref_counted_ptr_test.cc b/test/core/gprpp/ref_counted_ptr_test.cc index c810345166..aa30b72282 100644 --- a/test/core/gprpp/ref_counted_ptr_test.cc +++ b/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()); } diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index e8d2325b7d..87bba2263f 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/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; diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index eeec8e90bd..feea7c3907 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/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()); } diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 3c1d48cc1e..fc07681535 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/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, - grpc::string_ref metadata_value, + TestMetadataCredentialsPlugin(const grpc::string_ref& metadata_key, + 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); } diff --git a/test/cpp/end2end/mock_test.cc b/test/cpp/end2end/mock_test.cc index ff49902fea..ba3122c895 100644 --- a/test/cpp/end2end/mock_test.cc +++ b/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); } diff --git a/test/cpp/end2end/thread_stress_test.cc b/test/cpp/end2end/thread_stress_test.cc index e709a25356..ccf8400a87 100644 --- a/test/cpp/end2end/thread_stress_test.cc +++ b/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); } diff --git a/test/cpp/interop/interop_client.cc b/test/cpp/interop/interop_client.cc index 68bf1e6dc7..aaaa21de5b 100644 --- a/test/cpp/interop/interop_client.cc +++ b/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); diff --git a/test/cpp/interop/interop_client.h b/test/cpp/interop/interop_client.h index 79ff24fc47..a146212ff6 100644 --- a/test/cpp/interop/interop_client.h +++ b/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, diff --git a/test/cpp/interop/stress_interop_client.cc b/test/cpp/interop/stress_interop_client.cc index 30a8351cfe..9d373c3cd9 100644 --- a/test/cpp/interop/stress_interop_client.cc +++ b/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()); diff --git a/test/cpp/interop/stress_interop_client.h b/test/cpp/interop/stress_interop_client.h index a306dc3565..e4fa7d0973 100644 --- a/test/cpp/interop/stress_interop_client.h +++ b/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); diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 823c76f755..6fcf048bf3 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/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())); } diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc index c79a10d1b4..e6dbb7e076 100644 --- a/test/cpp/qps/client_async.cc +++ b/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), - next_issue_(next_issue), - prepare_req_(prepare_req) {} + callback_(std::move(on_done)), + next_issue_(std::move(next_issue)), + 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); } }; diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index e65e3b43f3..3a60c57e1d 100644 --- a/test/cpp/qps/client_sync.cc +++ b/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] { diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index 34f1291576..ecf5860b93 100644 --- a/test/cpp/qps/driver.cc +++ b/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 SystemTime(ClientStats s) { return s.time_system(); } -static double UserTime(ClientStats s) { return s.time_user(); } -static double CliPollCount(ClientStats s) { return s.cq_poll_count(); } -static double SvrPollCount(ServerStats s) { return s.cq_poll_count(); } -static double ServerWallTime(ServerStats s) { return s.time_elapsed(); } -static double ServerSystemTime(ServerStats s) { return s.time_system(); } -static double ServerUserTime(ServerStats s) { return s.time_user(); } -static double ServerTotalCpuTime(ServerStats s) { return s.total_cpu_time(); } -static double ServerIdleCpuTime(ServerStats s) { return s.idle_cpu_time(); } +static double WallTime(const ClientStats& s) { return s.time_elapsed(); } +static double SystemTime(const ClientStats& s) { return s.time_system(); } +static double UserTime(const ClientStats& s) { return s.time_user(); } +static double CliPollCount(const ClientStats& s) { return s.cq_poll_count(); } +static double SvrPollCount(const ServerStats& s) { return s.cq_poll_count(); } +static double ServerWallTime(const ServerStats& s) { return s.time_elapsed(); } +static double ServerSystemTime(const ServerStats& s) { return s.time_system(); } +static double ServerUserTime(const ServerStats& s) { return s.time_user(); } +static double ServerTotalCpuTime(const ServerStats& s) { + 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"); diff --git a/test/cpp/qps/qps_interarrival_test.cc b/test/cpp/qps/qps_interarrival_test.cc index 625b7db426..2cc22e9985 100644 --- a/test/cpp/qps/qps_interarrival_test.cc +++ b/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)); diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h index 8e62f4f449..b00b0a311f 100644 --- a/test/cpp/qps/report.h +++ b/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: diff --git a/test/cpp/qps/server_sync.cc b/test/cpp/qps/server_sync.cc index 82a9186989..b8facf9b56 100644 --- a/test/cpp/qps/server_sync.cc +++ b/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)) { diff --git a/test/cpp/server/load_reporter/load_data_store_test.cc b/test/cpp/server/load_reporter/load_data_store_test.cc index 8280dee6a4..aa37b7d6ba 100644 --- a/test/cpp/server/load_reporter/load_data_store_test.cc +++ b/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 load_key) { + const grpc::string& hostname, const grpc::string& lb_id, + 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, diff --git a/test/cpp/util/byte_buffer_test.cc b/test/cpp/util/byte_buffer_test.cc index b48a53eed1..9bffbf7ac1 100644 --- a/test/cpp/util/byte_buffer_test.cc +++ b/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()); } diff --git a/test/cpp/util/cli_call.cc b/test/cpp/util/cli_call.cc index a3992ab278..c258bde908 100644 --- a/test/cpp/util/cli_call.cc +++ b/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)) { diff --git a/test/cpp/util/cli_call.h b/test/cpp/util/cli_call.h index 51ffafd29f..3f279095a4 100644 --- a/test/cpp/util/cli_call.h +++ b/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(); diff --git a/test/cpp/util/proto_file_parser.cc b/test/cpp/util/proto_file_parser.cc index 3fc96f38ae..a530ed1ffc 100644 --- a/test/cpp/util/proto_file_parser.cc +++ b/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), diff --git a/test/cpp/util/proto_file_parser.h b/test/cpp/util/proto_file_parser.h index 2236b59451..eb1d793c2b 100644 --- a/test/cpp/util/proto_file_parser.h +++ b/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); diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index 0adbf20ce6..119272ca42 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/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", diff --git a/test/cpp/util/proto_reflection_descriptor_database.h b/test/cpp/util/proto_reflection_descriptor_database.h index e4cf2f207e..46190b3217 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.h +++ b/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(); diff --git a/test/cpp/util/string_ref_test.cc b/test/cpp/util/string_ref_test.cc index 8f7986e64e..031ec33241 100644 --- a/test/cpp/util/string_ref_test.cc +++ b/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()); } |