[Sync] Finch Experiment : Enable compression between sync client and server

sync/internal_api/http_bridge.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sync/internal_api/public/http_bridge.h"
 
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/static_http_user_agent_settings.h"
@@ skipping 12 matching lines @@
 // treated as a transient failure.
 const int kMaxHttpRequestTimeSeconds = 60 * 5;  // 5 minutes.
 
 // Helper method for logging timeouts via UMA.
 void LogTimeout(bool timed_out) {
   UMA_HISTOGRAM_BOOLEAN("Sync.URLFetchTimedOut", timed_out);
 }
 
 }  // namespace
 
+bool IsSyncHttpContentCompressionEnabled() {
+  const std::string group_name =
+      base::FieldTrialList::FindFullName("SyncHttpContentCompression");
+  return group_name == "Enabled";
+}
+
+bool GzipCompress(std::string source, std::string* dest) {
+  z_stream stream = {0};
+  int result = deflateInit2(&stream, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
+                            // 16 is added to produce a gzip header + trailer.
+                            MAX_WBITS + 16,
+                            8,  // memLevel = 8 is default.
+                            Z_DEFAULT_STRATEGY);
+  DCHECK_EQ(Z_OK, result);
+  size_t out_size = deflateBound(&stream, source.size());
+  scoped_ptr<char[]> out(new char[out_size]);
+
+  stream.next_in = reinterpret_cast<uint8*>(&source[0]);
+  stream.avail_in = source.size();
+  stream.next_out = reinterpret_cast<uint8*>(out.get());
+  stream.avail_out = out_size;
+
+  result = deflate(&stream, Z_FINISH);
+  if (result != Z_STREAM_END) {
+    return false;
+  }
+  out_size = out_size - stream.avail_out;
+
+  result = deflateEnd(&stream);
+  DCHECK_EQ(Z_OK, result);
+
+  dest->assign(out.get(), out_size);
+  return true;
+}
+
+void RecordSyncContentLengthHistograms(int64 this_content_length,

[Nicolas Zea, 2015/07/28 00:27:19]

nit: I think these variables could be named clearer. How about
compressed_content_length and original_content_length?

[Gang Wu, 2015/07/29 21:09:15]

Done.

+                                       int64 this_original_content_length) {
+  // Upload UMA data

[Nicolas Zea, 2015/07/28 00:27:19]

no need for this comment (all UMA histograms result in uploading data
eventually. And to be accurate, this doesn't actually immediately result in the
upload, but simply records the data for later upload).

[Gang Wu, 2015/07/29 21:09:15]

Done.

+  UMA_HISTOGRAM_COUNTS("Sync.ContentLengthCompressed", this_content_length);
+  UMA_HISTOGRAM_COUNTS("Sync.ContentLengthOriginal",
+                       this_original_content_length);
+}
+
 HttpBridgeFactory::HttpBridgeFactory(
     const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
     const NetworkTimeUpdateCallback& network_time_update_callback,
     CancelationSignal* cancelation_signal)
     : request_context_getter_(request_context_getter),
       network_time_update_callback_(network_time_update_callback),
       cancelation_signal_(cancelation_signal) {
   // Registration should never fail.  This should happen on the UI thread during
   // init.  It would be impossible for a shutdown to have been requested at this
   // point.
@@ skipping 146 matching lines @@
 
   // Start the timer on the network thread (the same thread progress is made
   // on, and on which the url fetcher lives).
   DCHECK(!fetch_state_.http_request_timeout_timer.get());
   fetch_state_.http_request_timeout_timer.reset(new base::Timer(false, false));
   fetch_state_.http_request_timeout_timer->Start(
       FROM_HERE, base::TimeDelta::FromSeconds(kMaxHttpRequestTimeSeconds),
       base::Bind(&HttpBridge::OnURLFetchTimedOut, this));
 
   DCHECK(request_context_getter_.get());
+  fetch_state_.start_time = base::Time::Now();
   fetch_state_.url_poster =
       net::URLFetcher::Create(url_for_request_, net::URLFetcher::POST, this)
           .release();
   fetch_state_.url_poster->SetRequestContext(request_context_getter_.get());
-  fetch_state_.url_poster->SetUploadData(content_type_, request_content_);
   fetch_state_.url_poster->SetExtraRequestHeaders(extra_headers_);
+
+  std::string content_to_be_sent = request_content_;

[Nicolas Zea, 2015/07/28 00:27:19]

I'd rather not pay the cost of a copy on all requests, even when we don't have
the feature enabled. How about pulling the SetUploadData call into the if
statements, and only declaring this variable within the if (IsSync...) body?

It would be something like:
int compressed_content_size = 0;
if (IsSyncHttp...) {
  std::string compress_content;
  GzipCompress(...);
  compressed_content_size = ...
  ...AddExtraRequestHeader...
  ...SetUploadData...)
} else {
  ...
}
RecordSyncContentLengthHistogram(...);

[Gang Wu, 2015/07/29 21:09:15]

Done.

+  if (IsSyncHttpContentCompressionEnabled()) {
+    GzipCompress(request_content_, &content_to_be_sent);
+    fetch_state_.url_poster->AddExtraRequestHeader("Content-Encoding: gzip");
+  } else {
+    fetch_state_.url_poster->AddExtraRequestHeader("Accept-Encoding: deflate");
+  }
+  fetch_state_.url_poster->SetUploadData(content_type_, content_to_be_sent);
+  RecordSyncContentLengthHistograms(content_to_be_sent.size(),
+                                    request_content_.size());
+
   fetch_state_.url_poster->AddExtraRequestHeader(base::StringPrintf(
       "%s: %s", net::HttpRequestHeaders::kUserAgent, user_agent_.c_str()));
   fetch_state_.url_poster->SetLoadFlags(net::LOAD_BYPASS_CACHE |
                                         net::LOAD_DISABLE_CACHE |
                                         net::LOAD_DO_NOT_SAVE_COOKIES |
                                         net::LOAD_DO_NOT_SEND_COOKIES);
-  fetch_state_.start_time = base::Time::Now();
 
   fetch_state_.url_poster->Start();
 }
 
 int HttpBridge::GetResponseContentLength() const {
   DCHECK_EQ(base::MessageLoop::current(), created_on_loop_);
   base::AutoLock lock(fetch_state_lock_);
   DCHECK(fetch_state_.request_completed);
   return fetch_state_.response_content.size();
 }
@@ skipping 79 matching lines @@
   // Use a real (non-debug) log to facilitate troubleshooting in the wild.
   VLOG(2) << "HttpBridge::OnURLFetchComplete for: "
           << fetch_state_.url_poster->GetURL().spec();
   VLOG(1) << "HttpBridge received response code: "
           << fetch_state_.http_response_code;
 
   source->GetResponseAsString(&fetch_state_.response_content);
   fetch_state_.response_headers = source->GetResponseHeaders();
   UpdateNetworkTime();
 
+  int64 compressed_content_length = fetch_state_.response_content.size();
+  int64 original_content_length = compressed_content_length;
+  if (fetch_state_.response_headers &&
+      fetch_state_.response_headers->HasHeaderValue("content-encoding",
+                                                    "gzip")) {
+    compressed_content_length =
+        fetch_state_.response_headers->GetContentLength();
+  }
+  RecordSyncContentLengthHistograms(compressed_content_length,

[Nicolas Zea, 2015/07/28 00:27:19]

Does it make sense to record the request separately from the response (in
different histograms)?

[Gang Wu, 2015/07/29 21:09:15]

Yes, since request is smaller than response, so separate then is better.

+                                    original_content_length);
+
   // End of the line for url_poster_. It lives only on the IO loop.
   // We defer deletion because we're inside a callback from a component of the
   // URLFetcher, so it seems most natural / "polite" to let the stack unwind.
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, fetch_state_.url_poster);
   fetch_state_.url_poster = NULL;
 
   // Wake the blocked syncer thread in MakeSynchronousPost.
   // WARNING: DONT DO ANYTHING AFTER THIS CALL! |this| may be deleted!
   http_post_completed_.Signal();
 }
@@ skipping 64 matching lines @@
   int64 sane_time_ms = 0;
   if (base::StringToInt64(sane_time_str, &sane_time_ms)) {
     network_time_update_callback_.Run(
         base::Time::FromJsTime(sane_time_ms),
         base::TimeDelta::FromMilliseconds(1),
         fetch_state_.end_time - fetch_state_.start_time);
   }
 }
 
 }  // namespace syncer