feat(storage): reduce copies in InsertObject()

.clang-format

@@ -47,4 +47,4 @@ RawStringFormats:
   - 'proto'
   BasedOnStyle: Google
 
-CommentPragmas: '(@copydoc|@copybrief|@see)'
+CommentPragmas: '(@copydoc|@copybrief|@see|@overload)'

google/cloud/storage/client.cc

@@ -165,7 +165,7 @@ StatusOr<ObjectMetadata> Client::UploadFileSimple(
     return Status(StatusCode::kInternal, std::move(os).str());
   }
   is.close();
-  request.set_contents(std::move(payload));
+  request.set_payload(payload);
 
   return raw_client_->InsertObjectMedia(request);
 }

google/cloud/storage/client.h

@@ -40,6 +40,7 @@
 #include "google/cloud/status.h"
 #include "google/cloud/status_or.h"
 #include "absl/meta/type_traits.h"
+#include "absl/strings/string_view.h"
 #include <string>
 #include <type_traits>
 #include <vector>
@@ -863,16 +864,38 @@ class Client {
   template <typename... Options>
   StatusOr<ObjectMetadata> InsertObject(std::string const& bucket_name,
                                         std::string const& object_name,
-                                        std::string contents,
+                                        absl::string_view contents,
                                         Options&&... options) {
     google::cloud::internal::OptionsSpan const span(
         SpanOptions(std::forward<Options>(options)...));
     internal::InsertObjectMediaRequest request(bucket_name, object_name,
-                                               std::move(contents));
+                                               contents);
     request.set_multiple_options(std::forward<Options>(options)...);
     return raw_client_->InsertObjectMedia(request);
   }
 
+  /// @overload InsertObject(std::string const& bucket_name, std::string const& object_name, absl::string_view contents, Options&&... options)
+  template <typename... Options>
+  StatusOr<ObjectMetadata> InsertObject(std::string const& bucket_name,
+                                        std::string const& object_name,
+                                        std::string const& contents,
+                                        Options&&... options) {
+    return InsertObject(bucket_name, object_name, absl::string_view(contents),
+                        std::forward<Options>(options)...);
+  }
+
+  /// @overload InsertObject(std::string const& bucket_name, std::string const& object_name, absl::string_view contents, Options&&... options)
+  template <typename... Options>
+  StatusOr<ObjectMetadata> InsertObject(std::string const& bucket_name,
+                                        std::string const& object_name,
+                                        char const* contents,
+                                        Options&&... options) {
+    auto c =
+        contents == nullptr ? absl::string_view{} : absl::string_view{contents};
+    return InsertObject(bucket_name, object_name, std::move(c),
+                        std::forward<Options>(options)...);
+  }
+
   /**
    * Copies an existing object.
    *

google/cloud/storage/client_object_test.cc

@@ -79,7 +79,7 @@ TEST_F(ObjectTest, InsertObjectMedia) {
         EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
         EXPECT_EQ("test-bucket-name", request.bucket_name());
         EXPECT_EQ("test-object-name", request.object_name());
-        EXPECT_EQ("test object contents", request.contents());
+        EXPECT_EQ("test object contents", request.payload());
         return make_status_or(expected);
       });
 
@@ -358,7 +358,7 @@ TEST_F(ObjectTest, UploadFile) {
         EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
         EXPECT_EQ("test-bucket-name", request.bucket_name());
         EXPECT_EQ("test-object-name", request.object_name());
-        EXPECT_EQ(contents, request.contents());
+        EXPECT_EQ(contents, request.payload());
         return make_status_or(expected);
       });
 

google/cloud/storage/compose_many_test.cc

@@ -73,7 +73,7 @@ TEST(ComposeMany, One) {
       .WillOnce([](internal::InsertObjectMediaRequest const& request) {
         EXPECT_EQ("test-bucket", request.bucket_name());
         EXPECT_EQ("prefix", request.object_name());
-        EXPECT_EQ("", request.contents());
+        EXPECT_EQ("", request.payload());
         return make_status_or(MockObject("test-bucket", "prefix", 42));
       });
   EXPECT_CALL(*mock, DeleteObject)
@@ -112,7 +112,7 @@ TEST(ComposeMany, Three) {
       .WillOnce([](internal::InsertObjectMediaRequest const& request) {
         EXPECT_EQ("test-bucket", request.bucket_name());
         EXPECT_EQ("prefix", request.object_name());
-        EXPECT_EQ("", request.contents());
+        EXPECT_EQ("", request.payload());
         return make_status_or(MockObject("test-bucket", "prefix", 42));
       });
   EXPECT_CALL(*mock, DeleteObject)
@@ -184,7 +184,7 @@ TEST(ComposeMany, ThreeLayers) {
       .WillOnce([](internal::InsertObjectMediaRequest const& request) {
         EXPECT_EQ("test-bucket", request.bucket_name());
         EXPECT_EQ("prefix", request.object_name());
-        EXPECT_EQ("", request.contents());
+        EXPECT_EQ("", request.payload());
         return make_status_or(MockObject("test-bucket", "prefix", 42));
       });
   EXPECT_CALL(*mock, DeleteObject)
@@ -234,7 +234,7 @@ TEST(ComposeMany, ComposeFails) {
       .WillOnce([](internal::InsertObjectMediaRequest const& request) {
         EXPECT_EQ("test-bucket", request.bucket_name());
         EXPECT_EQ("prefix", request.object_name());
-        EXPECT_EQ("", request.contents());
+        EXPECT_EQ("", request.payload());
         return make_status_or(MockObject("test-bucket", "prefix", 42));
       });
 
@@ -285,7 +285,7 @@ TEST(ComposeMany, CleanupFailsLoudly) {
       .WillOnce([](internal::InsertObjectMediaRequest const& request) {
         EXPECT_EQ("test-bucket", request.bucket_name());
         EXPECT_EQ("prefix", request.object_name());
-        EXPECT_EQ("", request.contents());
+        EXPECT_EQ("", request.payload());
         return make_status_or(MockObject("test-bucket", "prefix", 42));
       });
 
@@ -324,7 +324,7 @@ TEST(ComposeMany, CleanupFailsSilently) {
       .WillOnce([](internal::InsertObjectMediaRequest const& request) {
         EXPECT_EQ("test-bucket", request.bucket_name());
         EXPECT_EQ("prefix", request.object_name());
-        EXPECT_EQ("", request.contents());
+        EXPECT_EQ("", request.payload());
         return make_status_or(MockObject("test-bucket", "prefix", 42));
       });
 

google/cloud/storage/hashing_options.cc

@@ -22,17 +22,25 @@ namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 
-std::string ComputeMD5Hash(std::string const& payload) {
+std::string ComputeMD5Hash(absl::string_view payload) {
   return internal::Base64Encode(internal::MD5Hash(payload));
 }
 
-std::string ComputeCrc32cChecksum(std::string const& payload) {
+std::string ComputeMD5Hash(std::string const& payload) {
+  return ComputeMD5Hash(absl::string_view(payload));
+}
+
+std::string ComputeCrc32cChecksum(absl::string_view payload) {
   auto checksum = crc32c::Extend(
       0, reinterpret_cast<std::uint8_t const*>(payload.data()), payload.size());
   std::string const hash = google::cloud::internal::EncodeBigEndian(checksum);
   return internal::Base64Encode(hash);
 }
 
+std::string ComputeCrc32cChecksum(std::string const& payload) {
+  return ComputeCrc32cChecksum(absl::string_view(payload));
+}
+
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage
 }  // namespace cloud

google/cloud/storage/hashing_options.h

@@ -17,6 +17,7 @@
 
 #include "google/cloud/storage/internal/complex_option.h"
 #include "google/cloud/storage/version.h"
+#include "absl/strings/string_view.h"
 #include <string>
 
 namespace google {
@@ -45,10 +46,20 @@ struct MD5HashValue
 };
 
 /**
- * Compute the MD5 Hash of a string in the format preferred by GCS.
+ * Compute the MD5 Hash of a buffer in the format preferred by GCS.
  */
+std::string ComputeMD5Hash(absl::string_view payload);
+
+/// @copydoc ComputeMD5Hash(absl::string_view)
 std::string ComputeMD5Hash(std::string const& payload);
 
+/// @copydoc ComputeMD5Hash(absl::string_view)
+inline std::string ComputeMD5Hash(char const* payload) {
+  auto p =
+      payload == nullptr ? absl::string_view{} : absl::string_view{payload};
+  return ComputeMD5Hash(std::move(p));
+}
+
 /**
  * Disable or enable MD5 Hashing computations.
  *
@@ -99,10 +110,20 @@ struct Crc32cChecksumValue
 };
 
 /**
- * Compute the CRC32C checksum of a string in the format preferred by GCS.
+ * Compute the CRC32C checksum of a buffer in the format preferred by GCS.
  */
+std::string ComputeCrc32cChecksum(absl::string_view payload);
+
+/// @copydoc ComputeCrc32cChecksum(absl::string_view payload)
 std::string ComputeCrc32cChecksum(std::string const& payload);
 
+/// @copydoc ComputeCrc32cChecksum(absl::string_view payload)
+inline std::string ComputeCrc32cChecksum(char const* payload) {
+  auto p =
+      payload == nullptr ? absl::string_view{} : absl::string_view{payload};
+  return ComputeCrc32cChecksum(std::move(p));
+}
+
 /**
  * Disable CRC32C checksum computations.
  *

google/cloud/storage/internal/curl_client.cc

@@ -1122,13 +1122,13 @@ StatusOr<ObjectMetadata> CurlClient::InsertObjectMediaMultipart(
   if (request.HasOption<MD5HashValue>()) {
     metadata["md5Hash"] = request.GetOption<MD5HashValue>().value();
   } else if (!request.GetOption<DisableMD5Hash>().value_or(false)) {
-    metadata["md5Hash"] = ComputeMD5Hash(request.contents());
+    metadata["md5Hash"] = ComputeMD5Hash(request.payload());
   }
 
   if (request.HasOption<Crc32cChecksumValue>()) {
     metadata["crc32c"] = request.GetOption<Crc32cChecksumValue>().value();
   } else if (!request.GetOption<DisableCrc32cChecksum>().value_or(false)) {
-    metadata["crc32c"] = ComputeCrc32cChecksum(request.contents());
+    metadata["crc32c"] = ComputeCrc32cChecksum(request.payload());
   }
 
   std::string crlf = "\r\n";
@@ -1149,7 +1149,7 @@ StatusOr<ObjectMetadata> CurlClient::InsertObjectMediaMultipart(
   } else {
     writer << "content-type: application/octet-stream" << crlf;
   }
-  writer << crlf << request.contents() << crlf << marker << "--" << crlf;
+  writer << crlf << request.payload() << crlf << marker << "--" << crlf;
 
   // 6. Return the results as usual.
   auto contents = std::move(writer).str();
@@ -1179,9 +1179,9 @@ StatusOr<ObjectMetadata> CurlClient::InsertObjectMediaSimple(
   builder.AddQueryParameter("uploadType", "media");
   builder.AddQueryParameter("name", request.object_name());
   builder.AddHeader("Content-Length: " +
-                    std::to_string(request.contents().size()));
+                    std::to_string(request.payload().size()));
   return CheckedFromString<ObjectMetadataParser>(
-      std::move(builder).BuildRequest().MakeRequest(request.contents()));
+      std::move(builder).BuildRequest().MakeRequest(request.payload()));
 }
 
 }  // namespace internal

google/cloud/storage/internal/curl_request.cc

@@ -68,11 +68,11 @@ CurlRequest::~CurlRequest() {
   if (factory_) CurlHandle::ReturnToPool(*factory_, std::move(handle_));
 }
 
-StatusOr<HttpResponse> CurlRequest::MakeRequest(std::string const& payload) && {
+StatusOr<HttpResponse> CurlRequest::MakeRequest(absl::string_view payload) && {
   handle_.SetOption(CURLOPT_UPLOAD, 0L);
   if (!payload.empty()) {
     handle_.SetOption(CURLOPT_POSTFIELDSIZE, payload.length());
-    handle_.SetOption(CURLOPT_POSTFIELDS, payload.c_str());
+    handle_.SetOption(CURLOPT_POSTFIELDS, payload.data());
   }
   return MakeRequestImpl();
 }

google/cloud/storage/internal/curl_request.h

@@ -20,6 +20,7 @@
 #include "google/cloud/storage/internal/curl_handle_factory.h"
 #include "google/cloud/storage/internal/http_response.h"
 #include "google/cloud/storage/version.h"
+#include "absl/strings/string_view.h"
 #include <string>
 
 namespace google {
@@ -43,13 +44,23 @@ class CurlRequest {
   /**
    * Makes the prepared request.
    *
-   * This function can be called multiple times on the same request.
-   *
    * @return The response HTTP error code, the headers and an empty payload.
    */
-  StatusOr<HttpResponse> MakeRequest(std::string const& payload) &&;
+  StatusOr<HttpResponse> MakeRequest(absl::string_view payload) &&;
+
+  /// @overload MakeRequest(absl::string_view)
+  StatusOr<HttpResponse> MakeRequest(std::string const& payload) && {
+    return std::move(*this).MakeRequest(absl::string_view(payload));
+  }
+
+  /// @overload MakeRequest(absl::string_view)
+  StatusOr<HttpResponse> MakeRequest(char const* payload) && {
+    auto p =
+        payload == nullptr ? absl::string_view{} : absl::string_view{payload};
+    return std::move(*this).MakeRequest(std::move(p));
+  }
 
-  /// @copydoc MakeRequest(std::string const&)
+  /// @copydoc MakeRequest(absl::string_view)
   StatusOr<HttpResponse> MakeUploadRequest(ConstBufferSequence payload) &&;
 
  private:

google/cloud/storage/internal/grpc_client.cc

@@ -449,7 +449,7 @@ StatusOr<storage::ObjectMetadata> GrpcClient::InsertObjectMedia(
   using ContentType = std::remove_const_t<std::remove_reference_t<
       decltype(std::declval<google::storage::v2::ChecksummedData>()
                    .content())>>;
-  auto splitter = SplitObjectWriteData<ContentType>(request.contents());
+  auto splitter = SplitObjectWriteData<ContentType>(request.payload());
   std::int64_t offset = 0;
 
   // This loop must run at least once because we need to send at least one

google/cloud/storage/internal/grpc_object_metadata_parser.cc

@@ -68,7 +68,7 @@ StatusOr<std::string> MD5ToProto(std::string const& v) {
   return std::string{binary->begin(), binary->end()};
 }
 
-std::string ComputeMD5Hash(std::string const& payload) {
+std::string ComputeMD5Hash(absl::string_view payload) {
   auto b = storage::internal::MD5Hash(payload);
   return std::string{b.begin(), b.end()};
 }

google/cloud/storage/internal/grpc_object_metadata_parser.h

@@ -18,6 +18,7 @@
 #include "google/cloud/storage/object_metadata.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/options.h"
+#include "absl/strings/string_view.h"
 #include <google/storage/v2/storage.pb.h>
 
 namespace google {
@@ -34,7 +35,7 @@ std::string Crc32cFromProto(std::uint32_t);
 StatusOr<std::uint32_t> Crc32cToProto(std::string const&);
 std::string MD5FromProto(std::string const&);
 StatusOr<std::string> MD5ToProto(std::string const&);
-std::string ComputeMD5Hash(std::string const& payload);
+std::string ComputeMD5Hash(absl::string_view payload);
 
 storage::ObjectMetadata FromProto(google::storage::v2::Object object,
                                   Options const& options);

google/cloud/storage/internal/grpc_object_request_parser.cc

@@ -489,7 +489,8 @@ StatusOr<google::storage::v2::WriteObjectRequest> ToProto(
                  false)) {
     // Nothing to do, the option is disabled (mostly useful in tests).
   } else {
-    checksums.set_crc32c(crc32c::Crc32c(request.contents()));
+    checksums.set_crc32c(
+        crc32c::Crc32c(request.payload().data(), request.payload().size()));
   }
 
   if (request.HasOption<storage::MD5HashValue>()) {
@@ -500,8 +501,7 @@ StatusOr<google::storage::v2::WriteObjectRequest> ToProto(
   } else if (request.GetOption<storage::DisableMD5Hash>().value_or(false)) {
     // Nothing to do, the option is disabled.
   } else {
-    checksums.set_md5_hash(
-        storage_internal::ComputeMD5Hash(request.contents()));
+    checksums.set_md5_hash(storage_internal::ComputeMD5Hash(request.payload()));
   }
 
   return r;

google/cloud/storage/internal/object_requests.cc

@@ -161,14 +161,32 @@ std::ostream& operator<<(std::ostream& os, GetObjectMetadataRequest const& r) {
   return os << "}";
 }
 
+void InsertObjectMediaRequest::set_payload(absl::string_view payload) {
+  payload_ = payload;
+  dirty_ = true;
+}
+
+std::string const& InsertObjectMediaRequest::contents() const {
+  if (!dirty_) return contents_;
+  contents_ = std::string{payload_};
+  dirty_ = false;
+  return contents_;
+}
+
+void InsertObjectMediaRequest::set_contents(std::string v) {
+  contents_ = std::move(v);
+  payload_ = contents_;
+  dirty_ = false;
+}
+
 std::ostream& operator<<(std::ostream& os, InsertObjectMediaRequest const& r) {
   os << "InsertObjectMediaRequest={bucket_name=" << r.bucket_name()
      << ", object_name=" << r.object_name();
   r.DumpOptions(os, ", ");
   std::size_t constexpr kMaxDumpSize = 128;
+  auto const payload = r.payload();
   os << ", contents="
-     << BinaryDataAsDebugString(r.contents().data(), r.contents().size(),
-                                kMaxDumpSize);
+     << BinaryDataAsDebugString(payload.data(), payload.size(), kMaxDumpSize);
   return os << "}";
 }