Skip to content

Commit 3cdd834

Browse files
authored
cleanup(storage): report transfer size in benchmark (#8447)
For the most part this just creates a placeholder to report the transfer size. While the transfer size and object size could be different in case of (unrecoverable) errors during download, the real motivation is to support partial downloads in a future PR.
1 parent a49d569 commit 3cdd834

File tree

4 files changed

+27
-13
lines changed

4 files changed

+27
-13
lines changed

google/cloud/storage/benchmarks/throughput_experiment.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
#include "google/cloud/storage/benchmarks/benchmark_utils.h"
1717
#include "google/cloud/storage/client.h"
1818
#include "google/cloud/storage/grpc_plugin.h"
19-
#if GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
20-
#include "google/cloud/storage/internal/grpc_client.h"
21-
#endif // GOOGLE_CLOUD_CPP_STORAGE_HAVE_GRPC
2219
#include "google/cloud/grpc_error_delegate.h"
2320
#include "google/cloud/internal/getenv.h"
2421
#include "absl/algorithm/container.h"
@@ -72,6 +69,7 @@ class UploadObject : public ThroughputExperiment {
7269
gcs::DisableMD5Hash(!config.enable_md5), api_selector);
7370
auto const usage = timer.Sample();
7471
return ThroughputResult{kOpInsert,
72+
config.object_size,
7573
config.object_size,
7674
config.app_buffer_size,
7775
config.lib_buffer_size,
@@ -99,6 +97,7 @@ class UploadObject : public ThroughputExperiment {
9997
auto const usage = timer.Sample();
10098

10199
return ThroughputResult{kOpWrite,
100+
config.object_size,
102101
config.object_size,
103102
config.app_buffer_size,
104103
config.lib_buffer_size,
@@ -143,12 +142,14 @@ class DownloadObject : public ThroughputExperiment {
143142
bucket_name, object_name,
144143
gcs::DisableCrc32cChecksum(!config.enable_crc32c),
145144
gcs::DisableMD5Hash(!config.enable_md5), api_selector);
146-
for (std::uint64_t num_read = 0; reader.read(buffer.data(), buffer.size());
147-
num_read += reader.gcount()) {
145+
std::int64_t transfer_size = 0;
146+
while (reader.read(buffer.data(), buffer.size())) {
147+
transfer_size += reader.gcount();
148148
}
149149
auto const usage = timer.Sample();
150150
return ThroughputResult{config.op,
151151
config.object_size,
152+
transfer_size,
152153
config.app_buffer_size,
153154
config.lib_buffer_size,
154155
config.enable_crc32c,
@@ -228,6 +229,7 @@ class DownloadObjectLibcurl : public ThroughputExperiment {
228229
curl_slist_free_all(slist1);
229230
auto const usage = timer.Sample();
230231
return ThroughputResult{config.op,
232+
config.object_size,
231233
config.object_size,
232234
config.app_buffer_size,
233235
config.lib_buffer_size,
@@ -301,6 +303,7 @@ class DownloadObjectRawGrpc : public ThroughputExperiment {
301303

302304
return ThroughputResult{config.op,
303305
config.object_size,
306+
bytes_received,
304307
config.app_buffer_size,
305308
/*lib_buffer_size=*/0,
306309
/*crc_enabled=*/false,

google/cloud/storage/benchmarks/throughput_result.cc

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,22 @@ std::string QuoteCsv(T const& element) {
3939
} // namespace
4040

4141
void PrintAsCsv(std::ostream& os, ThroughputResult const& r) {
42-
os << ToString(r.op) << ',' << r.object_size << ',' << r.app_buffer_size
43-
<< ',' << r.lib_buffer_size << ',' << r.crc_enabled << ',' << r.md5_enabled
44-
<< ',' << ToString(r.api) << ',' << r.elapsed_time.count() << ','
45-
<< r.cpu_time.count() << ',' << QuoteCsv(r.status) << '\n';
42+
os << ToString(r.op) // force clang to keep one field per-line
43+
<< ',' << r.object_size //
44+
<< ',' << r.transfer_size //
45+
<< ',' << r.app_buffer_size //
46+
<< ',' << r.lib_buffer_size //
47+
<< ',' << r.crc_enabled //
48+
<< ',' << r.md5_enabled //
49+
<< ',' << ToString(r.api) //
50+
<< ',' << r.elapsed_time.count() //
51+
<< ',' << r.cpu_time.count() //
52+
<< ',' << QuoteCsv(r.status) //
53+
<< '\n';
4654
}
4755

4856
void PrintThroughputResultHeader(std::ostream& os) {
49-
os << "Op,ObjectSize,AppBufferSize,LibBufferSize"
57+
os << "Op,ObjectSize,TransferSize,AppBufferSize,LibBufferSize"
5058
<< ",Crc32cEnabled,MD5Enabled,ApiName"
5159
<< ",ElapsedTimeUs,CpuTimeUs,Status\n";
5260
}

google/cloud/storage/benchmarks/throughput_result.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ struct ThroughputResult {
6161
OpType op;
6262
/// The total size of the object involved in this experiment. Currently also
6363
/// represents the number of bytes transferred.
64-
// TODO(#4349) - use a separate field to represent the bytes transferred
6564
std::int64_t object_size;
65+
/// The size of the transfer. For uploads this is always equal to the object
66+
/// size. For downloads this can be smaller than the object size.
67+
std::int64_t transfer_size;
6668
/// The size of the application buffer (for .read() or .write() calls).
6769
std::size_t app_buffer_size;
6870
/// The size of the library buffers (if any).

google/cloud/storage/benchmarks/throughput_result_test.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ MATCHER_P(
3131
"status field from PrintAsCsv is properly quoted and contains substr") {
3232
// The status field is the 10th value.
3333
std::size_t pos = 0;
34-
for (int i = 0; i < 9; ++i) {
34+
for (int i = 0; i < 10; ++i) {
3535
pos = arg.find(',', pos);
3636
if (pos == std::string::npos) {
3737
*result_listener << "Couldn't find status field: " << arg;
@@ -68,7 +68,7 @@ TEST(ThroughputResult, HeaderMatches) {
6868
auto const header = std::move(header_stream).str();
6969

7070
auto const line = ToString(ThroughputResult{
71-
kOpInsert, /*object_size=*/3 * kMiB,
71+
kOpInsert, /*object_size=*/5 * kMiB, /*transfer_size=*/3 * kMiB,
7272
/*app_buffer_size=*/2 * kMiB, /*lib_buffer_size=*/4 * kMiB,
7373
/*crc_enabled=*/true, /*md5_enabled=*/false, ApiName::kApiGrpc,
7474
std::chrono::microseconds(234000), std::chrono::microseconds(345000),
@@ -86,6 +86,7 @@ TEST(ThroughputResult, HeaderMatches) {
8686
// We don't want to create a change detector test, but we can verify the basic
8787
// fields are formatted correctly.
8888
EXPECT_THAT(*line, HasSubstr(ToString(kOpInsert)));
89+
EXPECT_THAT(*line, HasSubstr("," + std::to_string(5 * kMiB) + ","));
8990
EXPECT_THAT(*line, HasSubstr("," + std::to_string(3 * kMiB) + ","));
9091
EXPECT_THAT(*line, HasSubstr("," + std::to_string(2 * kMiB) + ","));
9192
EXPECT_THAT(*line, HasSubstr("," + std::to_string(4 * kMiB) + ","));

0 commit comments

Comments
 (0)