Skip to content

Commit 8a2efcf

Browse files
coryancoryan
authored
cleanup(storage): final cognitive complexity reduction (#9433)
1 parent bff6db6 commit 8a2efcf

File tree

2 files changed

+113
-94
lines changed

2 files changed

+113
-94
lines changed

‎google/cloud/storage/benchmarks/aggregate_download_throughput_benchmark.cc‎

Lines changed: 74 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,17 @@ gcs::Client MakeClient(AggregateDownloadThroughputOptions const& options) {
137137
return gcs::Client(std::move(client_options));
138138
}
139139

140+
std::string CurrentTime() {
141+
auto constexpr kFormat = "%E4Y-%m-%dT%H:%M:%E*SZ";
142+
auto const t = absl::FromChrono(std::chrono::system_clock::now());
143+
return absl::FormatTime(kFormat, t, absl::UTCTimeZone());
144+
};
145+
146+
void PrintResults(AggregateDownloadThroughputOptions const& options,
147+
std::size_t object_count, std::uint64_t dataset_size,
148+
std::vector<TaskResult> const& iteration_results,
149+
Timer::Snapshot usage);
150+
140151
} // namespace
141152

142153
int main(int argc, char* argv[]) {
@@ -169,13 +180,7 @@ int main(int argc, char* argv[]) {
169180
std::transform(notes.begin(), notes.end(), notes.begin(),
170181
[](char c) { return c == '\n' ? ';' : c; });
171182

172-
auto current_time = [] {
173-
auto constexpr kFormat = "%E4Y-%m-%dT%H:%M:%E*SZ";
174-
auto const t = absl::FromChrono(std::chrono::system_clock::now());
175-
return absl::FormatTime(kFormat, t, absl::UTCTimeZone());
176-
};
177-
178-
std::cout << "# Start time: " << current_time()
183+
std::cout << "# Start time: " << CurrentTime()
179184
<< "\n# Labels: " << options->labels
180185
<< "\n# Bucket Name: " << options->bucket_name
181186
<< "\n# Object Prefix: " << options->object_prefix
@@ -219,13 +224,6 @@ int main(int argc, char* argv[]) {
219224
objects.insert(objects.end(), dataset.begin(), dataset.end());
220225
}
221226

222-
auto accumulate_bytes_downloaded = [](std::vector<TaskResult> const& r) {
223-
return std::accumulate(r.begin(), r.end(), std::int64_t{0},
224-
[](std::int64_t a, TaskResult const& b) {
225-
return a + b.bytes_downloaded;
226-
});
227-
};
228-
229227
Counters accumulated;
230228
// Print the header, so it can be easily loaded using the tools available in
231229
// our analysis tools (typically Python pandas, but could be R). Flush the
@@ -255,57 +253,14 @@ int main(int argc, char* argv[]) {
255253
std::make_move_iterator(tasks.end()),
256254
iteration_results.begin(),
257255
[](std::future<TaskResult> f) { return f.get(); });
258-
auto const usage = timer.Sample();
259-
auto const downloaded_bytes =
260-
accumulate_bytes_downloaded(iteration_results);
261256

262-
auto clean_csv_field = [](std::string v) {
263-
std::replace(v.begin(), v.end(), ',', ';');
264-
return v;
265-
};
266-
auto const labels = clean_csv_field(options->labels);
267-
auto const grpc_plugin_config =
268-
clean_csv_field(options->grpc_plugin_config);
269-
auto const* client_per_thread =
270-
options->client_per_thread ? "true" : "false";
271-
// Print the results after each iteration. Makes it possible to interrupt
272-
// the benchmark in the middle and still get some data.
257+
// Update the counters.
273258
for (auto const& r : iteration_results) {
274-
for (auto const& d : r.details) {
275-
// Join the iteration details with the per-download details. That makes
276-
// it easier to analyze the data in external scripts.
277-
std::cout << labels //
278-
<< ',' << d.iteration //
279-
<< ',' << objects.size() //
280-
<< ',' << dataset_size //
281-
<< ',' << options->thread_count //
282-
<< ',' << options->repeats_per_iteration //
283-
<< ',' << options->read_size //
284-
<< ',' << options->read_buffer_size //
285-
<< ',' << ToString(options->api) //
286-
<< ',' << options->grpc_channel_count //
287-
<< ',' << grpc_plugin_config //
288-
<< ',' << client_per_thread //
289-
<< ',' << d.status.code() //
290-
<< ',' << d.peer //
291-
<< ',' << d.bytes_downloaded //
292-
<< ',' << d.elapsed_time.count() //
293-
<< ',' << downloaded_bytes //
294-
<< ',' << usage.elapsed_time.count() //
295-
<< ',' << usage.cpu_time.count() //
296-
<< "\n";
297-
}
298-
// Update the counters.
299259
for (auto const& kv : r.counters) accumulated[kv.first] += kv.second;
300260
}
301-
// After each iteration print a human-readable summary. Flush it because
302-
// the operator of these benchmarks (coryan@) is an impatient person.
303-
auto const bandwidth =
304-
FormatBandwidthGbPerSecond(downloaded_bytes, usage.elapsed_time);
305-
std::cout << "# " << current_time() << " downloaded=" << downloaded_bytes
306-
<< " cpu_time=" << absl::FromChrono(usage.cpu_time)
307-
<< " elapsed_time=" << absl::FromChrono(usage.elapsed_time)
308-
<< " Gbit/s=" << bandwidth << std::endl;
261+
262+
PrintResults(*options, objects.size(), dataset_size, iteration_results,
263+
timer.Sample());
309264
}
310265

311266
for (auto& kv : accumulated) {
@@ -426,4 +381,62 @@ google::cloud::StatusOr<AggregateDownloadThroughputOptions> ParseArgs(
426381
kDescription);
427382
}
428383

384+
void PrintResults(AggregateDownloadThroughputOptions const& options,
385+
std::size_t object_count, std::uint64_t dataset_size,
386+
std::vector<TaskResult> const& iteration_results,
387+
Timer::Snapshot usage) {
388+
auto accumulate_bytes_downloaded = [](std::vector<TaskResult> const& r) {
389+
return std::accumulate(r.begin(), r.end(), std::int64_t{0},
390+
[](std::int64_t a, TaskResult const& b) {
391+
return a + b.bytes_downloaded;
392+
});
393+
};
394+
395+
auto const downloaded_bytes = accumulate_bytes_downloaded(iteration_results);
396+
397+
auto clean_csv_field = [](std::string v) {
398+
std::replace(v.begin(), v.end(), ',', ';');
399+
return v;
400+
};
401+
auto const labels = clean_csv_field(options.labels);
402+
auto const grpc_plugin_config = clean_csv_field(options.grpc_plugin_config);
403+
auto const* client_per_thread = options.client_per_thread ? "true" : "false";
404+
// Print the results after each iteration. Makes it possible to interrupt
405+
// the benchmark in the middle and still get some data.
406+
for (auto const& r : iteration_results) {
407+
for (auto const& d : r.details) {
408+
// Join the iteration details with the per-download details. That makes
409+
// it easier to analyze the data in external scripts.
410+
std::cout << labels //
411+
<< ',' << d.iteration //
412+
<< ',' << object_count //
413+
<< ',' << dataset_size //
414+
<< ',' << options.thread_count //
415+
<< ',' << options.repeats_per_iteration //
416+
<< ',' << options.read_size //
417+
<< ',' << options.read_buffer_size //
418+
<< ',' << ToString(options.api) //
419+
<< ',' << options.grpc_channel_count //
420+
<< ',' << grpc_plugin_config //
421+
<< ',' << client_per_thread //
422+
<< ',' << d.status.code() //
423+
<< ',' << d.peer //
424+
<< ',' << d.bytes_downloaded //
425+
<< ',' << d.elapsed_time.count() //
426+
<< ',' << downloaded_bytes //
427+
<< ',' << usage.elapsed_time.count() //
428+
<< ',' << usage.cpu_time.count() //
429+
<< "\n";
430+
}
431+
}
432+
// After each iteration print a human-readable summary. Flush it because
433+
// the operator of these benchmarks (coryan@) is an impatient person.
434+
auto const bandwidth =
435+
FormatBandwidthGbPerSecond(downloaded_bytes, usage.elapsed_time);
436+
std::cout << "# " << CurrentTime() << " downloaded=" << downloaded_bytes
437+
<< " cpu_time=" << absl::FromChrono(usage.cpu_time)
438+
<< " elapsed_time=" << absl::FromChrono(usage.elapsed_time)
439+
<< " Gbit/s=" << bandwidth << std::endl;
440+
}
441+
429442
} // namespace

‎google/cloud/storage/benchmarks/throughput_options.cc‎

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,39 @@ Status ValidateQuantizedRange(std::string const& name, std::int64_t minimum,
4141
return Status{};
4242
}
4343

44+
std::vector<bool> ParseChecksums(std::string const& val) {
45+
if (val == "enabled") {
46+
return {true};
47+
}
48+
if (val == "disabled") {
49+
return {false};
50+
}
51+
if (val == "random") {
52+
return {false, true};
53+
}
54+
return {};
55+
}
56+
57+
std::vector<ExperimentLibrary> ParseLibraries(std::string const& val) {
58+
std::set<ExperimentLibrary> libs; // avoid duplicates
59+
for (auto const& token : absl::StrSplit(val, ',')) {
60+
auto lib = ParseExperimentLibrary(std::string{token});
61+
if (!lib) return {};
62+
libs.insert(*lib);
63+
}
64+
return {libs.begin(), libs.end()};
65+
}
66+
67+
std::vector<ExperimentTransport> ParseTransports(std::string const& val) {
68+
std::set<ExperimentTransport> transports; // avoid duplicates
69+
for (auto const& token : absl::StrSplit(val, ',')) {
70+
auto transport = ParseExperimentTransport(std::string{token});
71+
if (!transport) return {};
72+
transports.insert(*transport);
73+
}
74+
return {transports.begin(), transports.end()};
75+
}
76+
4477
} // namespace
4578

4679
using ::google::cloud::testing_util::OptionDescriptor;
@@ -51,19 +84,6 @@ google::cloud::StatusOr<ThroughputOptions> ParseThroughputOptions(
5184
bool wants_help = false;
5285
bool wants_description = false;
5386

54-
auto parse_checksums = [](std::string const& val) -> std::vector<bool> {
55-
if (val == "enabled") {
56-
return {true};
57-
}
58-
if (val == "disabled") {
59-
return {false};
60-
}
61-
if (val == "random") {
62-
return {false, true};
63-
}
64-
return {};
65-
};
66-
6787
std::vector<OptionDescriptor> desc{
6888
{"--help", "print usage information",
6989
[&wants_help](std::string const&) { wants_help = true; }},
@@ -145,34 +165,20 @@ google::cloud::StatusOr<ThroughputOptions> ParseThroughputOptions(
145165
}},
146166
{"--enabled-libs", "enable more libraries (e.g. Raw, CppClient)",
147167
[&options](std::string const& val) {
148-
options.libs.clear(); //
149-
std::set<ExperimentLibrary> libs; // avoid duplicates
150-
for (auto const& token : absl::StrSplit(val, ',')) {
151-
auto lib = ParseExperimentLibrary(std::string{token});
152-
if (!lib) return;
153-
libs.insert(*lib);
154-
}
155-
options.libs = {libs.begin(), libs.end()};
168+
options.libs = ParseLibraries(val);
156169
}},
157170
{"--enabled-transports",
158171
"enable a subset of the transports (DirectPath, Grpc, Json, Xml)",
159172
[&options](std::string const& val) {
160-
options.transports.clear();
161-
std::set<ExperimentTransport> transports; // avoid duplicates
162-
for (auto const& token : absl::StrSplit(val, ',')) {
163-
auto transport = ParseExperimentTransport(std::string{token});
164-
if (!transport) return;
165-
transports.insert(*transport);
166-
}
167-
options.transports = {transports.begin(), transports.end()};
173+
options.transports = ParseTransports(val);
168174
}},
169175
{"--enabled-crc32c", "run with CRC32C enabled, disabled, or both",
170-
[&options, &parse_checksums](std::string const& val) {
171-
options.enabled_crc32c = parse_checksums(val);
176+
[&options](std::string const& val) {
177+
options.enabled_crc32c = ParseChecksums(val);
172178
}},
173179
{"--enabled-md5", "run with MD5 enabled, disabled, or both",
174-
[&options, &parse_checksums](std::string const& val) {
175-
options.enabled_md5 = parse_checksums(val);
180+
[&options](std::string const& val) {
181+
options.enabled_md5 = ParseChecksums(val);
176182
}},
177183
{"--rest-endpoint", "sets the endpoint for REST-based benchmarks",
178184
[&options](std::string const& val) { options.rest_endpoint = val; }},

0 commit comments

Comments
 (0)