Skip to content

Commit e9bc8e5

Browse files
authored
feat(storage): reduce overhead in pooled handle factory (#6169)
Replace `std::vector<>` with `std::deque<>` because when the pool grows too large we want to remove the *oldest* handles (fron the front). Also, use a while loop during cleanup because it makes the intent clearer. I expect that while loop to run at most once, but why force humans to think this through?
1 parent c893624 commit e9bc8e5

File tree

2 files changed

+6
-9
lines changed

2 files changed

+6
-9
lines changed

google/cloud/storage/internal/curl_handle_factory.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,7 @@ void DefaultCurlHandleFactory::CleanupMultiHandle(CurlMulti&& m) { m.reset(); }
7474

7575
PooledCurlHandleFactory::PooledCurlHandleFactory(std::size_t maximum_size,
7676
ChannelOptions options)
77-
: maximum_size_(maximum_size), options_(std::move(options)) {
78-
handles_.reserve(maximum_size);
79-
multi_handles_.reserve(maximum_size);
80-
}
77+
: maximum_size_(maximum_size), options_(std::move(options)) {}
8178

8279
PooledCurlHandleFactory::~PooledCurlHandleFactory() {
8380
for (auto* h : handles_) {
@@ -111,7 +108,7 @@ void PooledCurlHandleFactory::CleanupHandle(CurlHandle&& h) {
111108
if (res == CURLE_OK && ip != nullptr) {
112109
last_client_ip_address_ = ip;
113110
}
114-
if (handles_.size() >= maximum_size_) {
111+
while (handles_.size() >= maximum_size_) {
115112
auto* tmp = handles_.front();
116113
handles_.erase(handles_.begin());
117114
curl_easy_cleanup(tmp);
@@ -133,7 +130,7 @@ CurlMulti PooledCurlHandleFactory::CreateMultiHandle() {
133130

134131
void PooledCurlHandleFactory::CleanupMultiHandle(CurlMulti&& m) {
135132
std::unique_lock<std::mutex> lk(mu_);
136-
if (multi_handles_.size() >= maximum_size_) {
133+
while (multi_handles_.size() >= maximum_size_) {
137134
auto* tmp = multi_handles_.front();
138135
multi_handles_.erase(multi_handles_.begin());
139136
curl_multi_cleanup(tmp);

google/cloud/storage/internal/curl_handle_factory.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
#include "google/cloud/storage/internal/curl_handle.h"
1919
#include "google/cloud/storage/internal/curl_wrappers.h"
2020
#include "google/cloud/storage/version.h"
21+
#include <deque>
2122
#include <mutex>
2223
#include <string>
23-
#include <vector>
2424

2525
namespace google {
2626
namespace cloud {
@@ -114,8 +114,8 @@ class PooledCurlHandleFactory : public CurlHandleFactory {
114114
private:
115115
std::size_t maximum_size_;
116116
mutable std::mutex mu_;
117-
std::vector<CURL*> handles_;
118-
std::vector<CURLM*> multi_handles_;
117+
std::deque<CURL*> handles_;
118+
std::deque<CURLM*> multi_handles_;
119119
std::string last_client_ip_address_;
120120
ChannelOptions options_;
121121
};

0 commit comments

Comments
 (0)