From 90102c2bfcf98f842f66567c67cbccbd0ede1d2b Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 3 Nov 2016 11:41:19 -0700 Subject: [PATCH] Eliminate unnecessary uses of new[]/delete[] that can be replaced with vector Also start eliminating uses of plain-old delete that are not helpful --- test/cpp/end2end/async_end2end_test.cc | 6 ++---- test/cpp/end2end/thread_stress_test.cc | 14 ++++++-------- test/cpp/qps/client.h | 7 ++----- test/cpp/qps/client_sync.cc | 12 +++--------- test/cpp/qps/driver.cc | 13 ++----------- 5 files changed, 15 insertions(+), 37 deletions(-) diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 3845582d5db..0993ead86fc 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -352,15 +352,13 @@ void ServerWait(Server* server, int* notify) { } TEST_P(AsyncEnd2endTest, WaitAndShutdownTest) { int notify = 0; - std::thread* wait_thread = - new std::thread(&ServerWait, server_.get(), ¬ify); + std::thread wait_thread(&ServerWait, server_.get(), ¬ify); ResetStub(); SendRpc(1); EXPECT_EQ(0, notify); server_->Shutdown(); - wait_thread->join(); + wait_thread.join(); EXPECT_EQ(1, notify); - delete wait_thread; } TEST_P(AsyncEnd2endTest, ShutdownThenWait) { diff --git a/test/cpp/end2end/thread_stress_test.cc b/test/cpp/end2end/thread_stress_test.cc index fe5a219eed7..51cc3d91df5 100644 --- a/test/cpp/end2end/thread_stress_test.cc +++ b/test/cpp/end2end/thread_stress_test.cc @@ -232,13 +232,13 @@ class CommonStressTestSyncServer : public CommonStressTest { class CommonStressTestAsyncServer : public CommonStressTest { public: + CommonStressTestAsyncServer() : contexts_(kNumAsyncServerThreads * 100) {} void SetUp() override { shutting_down_ = false; ServerBuilder builder; SetUpStart(&builder, &service_); cq_ = builder.AddCompletionQueue(); SetUpEnd(&builder); - contexts_ = new Context[kNumAsyncServerThreads * 100]; for (int i = 0; i < kNumAsyncServerThreads * 100; i++) { RefreshContext(i); } @@ -265,7 +265,6 @@ class CommonStressTestAsyncServer while (cq_->Next(&ignored_tag, &ignored_ok)) ; TearDownEnd(); - delete[] contexts_; } private: @@ -311,7 +310,8 @@ class CommonStressTestAsyncServer response_writer; EchoRequest recv_request; enum { READY, DONE } state; - } * contexts_; + }; + std::vector contexts_; ::grpc::testing::EchoTestService::AsyncService service_; std::unique_ptr cq_; bool shutting_down_; @@ -353,14 +353,12 @@ typedef ::testing::Typescommon_.ResetStub(); - std::vector threads; + std::vector threads; for (int i = 0; i < kNumThreads; ++i) { - threads.push_back( - new std::thread(SendRpc, this->common_.GetStub(), kNumRpcs)); + threads.emplace_back(SendRpc, this->common_.GetStub(), kNumRpcs); } for (int i = 0; i < kNumThreads; ++i) { - threads[i]->join(); - delete threads[i]; + threads[i].join(); } } diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index a8d125ad281..56bfa9fee12 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -163,10 +163,9 @@ class Client { MaybeStartRequests(); - // avoid std::vector for old compilers that expect a copy constructor if (reset) { - Histogram* to_merge = new Histogram[threads_.size()]; - StatusHistogram* to_merge_status = new StatusHistogram[threads_.size()]; + std::vector to_merge(threads_.size()); + std::vector to_merge_status(threads_.size()); for (size_t i = 0; i < threads_.size(); i++) { threads_[i]->BeginSwap(&to_merge[i], &to_merge_status[i]); @@ -177,8 +176,6 @@ class Client { latencies.Merge(to_merge[i]); MergeStatusHistogram(to_merge_status[i], &statuses); } - delete[] to_merge; - delete[] to_merge_status; timer_result = timer->Mark(); } else { // merge snapshots of each thread histogram diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index a88a24d89cc..3f402b5c36c 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -138,10 +138,7 @@ class SynchronousUnaryClient final : public SynchronousClient { class SynchronousStreamingClient final : public SynchronousClient { public: SynchronousStreamingClient(const ClientConfig& config) - : SynchronousClient(config) { - context_ = new grpc::ClientContext[num_threads_]; - stream_ = new std::unique_ptr< - grpc::ClientReaderWriter>[num_threads_]; + : SynchronousClient(config), context_(num_threads_), stream_(num_threads_) { for (size_t thread_idx = 0; thread_idx < num_threads_; thread_idx++) { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); stream_[thread_idx] = stub->StreamingCall(&context_[thread_idx]); @@ -161,8 +158,6 @@ class SynchronousStreamingClient final : public SynchronousClient { } } } - delete[] stream_; - delete[] context_; } bool ThreadFunc(HistogramEntry* entry, size_t thread_idx) override { @@ -182,9 +177,8 @@ class SynchronousStreamingClient final : public SynchronousClient { private: // These are both conceptually std::vector but cannot be for old compilers // that expect contained classes to support copy constructors - grpc::ClientContext* context_; - std::unique_ptr>* - stream_; + std::vector context_; + std::vector>> stream_; }; std::unique_ptr CreateSynchronousUnaryClient( diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index a440341ccf4..2f343d36885 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -262,10 +262,7 @@ std::unique_ptr RunScenario( workers.resize(num_clients + num_servers); // Start servers - using runsc::ServerData; - // servers is array rather than std::vector to avoid gcc-4.4 issues - // where class contained in std::vector must have a copy constructor - auto* servers = new ServerData[num_servers]; + std::vector servers(num_servers); for (size_t i = 0; i < num_servers; i++) { gpr_log(GPR_INFO, "Starting server on %s (worker #%" PRIuPTR ")", workers[i].c_str(), i); @@ -328,10 +325,7 @@ std::unique_ptr RunScenario( // Targets are all set by now result_client_config = client_config; // Start clients - using runsc::ClientData; - // clients is array rather than std::vector to avoid gcc-4.4 issues - // where class contained in std::vector must have a copy constructor - auto* clients = new ClientData[num_clients]; + std::vector clients(num_clients); size_t channels_allocated = 0; for (size_t i = 0; i < num_clients; i++) { const auto& worker = workers[i + num_servers]; @@ -501,7 +495,6 @@ std::unique_ptr RunScenario( s.error_message().c_str()); } } - delete[] clients; merged_latencies.FillProto(result->mutable_latencies()); for (std::unordered_map::iterator it = merged_statuses.begin(); @@ -544,8 +537,6 @@ std::unique_ptr RunScenario( } } - delete[] servers; - postprocess_scenario_result(result.get()); return result; }