Add url_request_quic_perftest.cc

net/url_request/url_request_quic_perftest.cc

Index: net/url_request/url_request_quic_perftest.cc
diff --git a/net/url_request/url_request_quic_perftest.cc b/net/url_request/url_request_quic_perftest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..45e50a2753e00b28e7e60d8a88475c398e43b68d
--- /dev/null
+++ b/net/url_request/url_request_quic_perftest.cc
@@ -0,0 +1,359 @@
+// Copyright 2017 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 <inttypes.h>
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "base/time/time.h"
+#include "base/trace_event/memory_dump_manager.h"
+#include "base/trace_event/memory_dump_request_args.h"
+#include "base/trace_event/process_memory_dump.h"
+#include "base/trace_event/trace_config.h"
+#include "base/trace_event/trace_config_memory_test_util.h"
+#include "base/trace_event/trace_event_argument.h"
+#include "base/trace_event/trace_log.h"
+#include "net/base/load_timing_info.h"
+#include "net/cert/mock_cert_verifier.h"
+#include "net/dns/mapped_host_resolver.h"
+#include "net/dns/mock_host_resolver.h"
+#include "net/http/http_status_code.h"
+#include "net/quic/chromium/crypto/proof_source_chromium.h"
+#include "net/quic/test_tools/crypto_test_utils.h"
+#include "net/test/cert_test_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "net/test/gtest_util.h"
+#include "net/test/test_data_directory.h"
+#include "net/tools/quic/quic_http_response_cache.h"
+#include "net/tools/quic/quic_simple_server.h"
+#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/perf/perf_test.h"
+#include "url/gurl.h"
+
+using testing::_;
+using testing::Invoke;
+
+namespace net {
+
+namespace {
+
+const int kAltSvcPort = 6121;
+const char kOriginHost[] = "mail.example.com";
+const char kAltSvcHost[] = "test.example.com";
+// Used as a simple response from the server.
+const char kHelloPath[] = "/hello.txt";
+const char kHelloAltSvcResponse[] = "Hello from QUIC Server";
+const char kHelloOriginResponse[] = "Hello from TCP Server";
+const int kHelloStatus = 200;
+
+std::unique_ptr<test_server::HttpResponse> HandleRequest(
+    const test_server::HttpRequest& request) {
+  std::unique_ptr<test_server::BasicHttpResponse> http_response(
+      new test_server::BasicHttpResponse());
+  http_response->AddCustomHeader(
+      "Alt-Svc", base::StringPrintf(
+                     "quic=\"%s:%d\"; v=\"%u\"", kAltSvcHost, kAltSvcPort,
+                     HttpNetworkSession::Params().quic_supported_versions[0]));
+  http_response->set_code(HTTP_OK);
+  http_response->set_content(kHelloOriginResponse);
+  http_response->set_content_type("text/plain");
+  return std::move(http_response);
+}
+
+void PrintPerfTest(const std::string& name,
+                   int value,
+                   const std::string& unit) {
+  const ::testing::TestInfo* test_info =
+      ::testing::UnitTest::GetInstance()->current_test_info();
+  perf_test::PrintResult(test_info->test_case_name(),
+                         std::string(".") + test_info->name(), name,
+                         static_cast<double>(value), unit, true);
+}
+
+// This mocks the RequestGlobalDumpFunction which is typically handled by
+// process_local_dump_manager_impl.cc.
+class GlobalMemoryDumpHandler {
+ public:
+  MOCK_METHOD2(
+      RequestGlobalMemoryDump,
+      void(const base::trace_event::MemoryDumpRequestArgs& args,
+           const base::trace_event::GlobalMemoryDumpCallback& callback));
+  GlobalMemoryDumpHandler() {}
+};
+
+// A TestURLRequestContext that records memory dump stats when OnMemoryDump() is
+// invoked.
+class MemoryTestURLRequestContext : public TestURLRequestContext {
+ public:
+  MemoryTestURLRequestContext() : TestURLRequestContext(true) {}
+
+  bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
+                    base::trace_event::ProcessMemoryDump* pmd) override {
+    bool result = TestURLRequestContext::OnMemoryDump(args, pmd);
+
+    const base::trace_event::MemoryAllocatorDump* url_request_context_dump =
+        pmd->GetAllocatorDump(
+            base::StringPrintf("net/url_request_context/unknown/0x%" PRIxPTR,
+                               reinterpret_cast<uintptr_t>(this)));
+    url_request_context_dump_attrs_ =
+        url_request_context_dump->attributes_for_testing()->ToBaseValue();
+
+    const base::trace_event::MemoryAllocatorDump* quic_stream_factory_dump =
+        pmd->GetAllocatorDump(base::StringPrintf(
+            "net/http_network_session_%p/quic_stream_factory",
+            http_transaction_factory()->GetSession()));
+    quic_stream_factory_attrs_ =
+        quic_stream_factory_dump->attributes_for_testing()->ToBaseValue();
+
+    const base::trace_event::MemoryAllocatorDump* http_stream_factory_dump =
+        pmd->GetAllocatorDump(
+            base::StringPrintf("net/http_network_session_%p/stream_factory",
+                               http_transaction_factory()->GetSession()));
+    EXPECT_EQ(nullptr, http_stream_factory_dump);
+    return result;
+  }
+
+  std::unique_ptr<base::Value> url_request_context_dump_attrs_;
+  std::unique_ptr<base::Value> quic_stream_factory_attrs_;
+};
+
+class URLRequestQuicPerfTest : public ::testing::Test {
+ public:
+  void ProcessDumpRecordingCallbackAdapter(
+      base::trace_event::GlobalMemoryDumpCallback callback,
+      uint64_t dump_guid,
+      bool success,
+      const base::Optional<base::trace_event::MemoryDumpCallbackResult>&
+          result) {
+    if (result.has_value()) {
+      results_.push_back(result.value());
+    }
+    callback.Run(dump_guid, success);
+  }
+
+ protected:
+  URLRequestQuicPerfTest() : message_loop_(new base::MessageLoopForIO()) {
+    InitializeMemoryDumpManager();
+    mdm_->set_dumper_registrations_ignored_for_testing(false);
+    context_ = base::MakeUnique<MemoryTestURLRequestContext>();
+    mdm_->set_dumper_registrations_ignored_for_testing(true);
+    StartTcpServer();
+    StartQuicServer();
+
+    // Host mapping.
+    std::unique_ptr<MockHostResolver> resolver(new MockHostResolver());
+    resolver->rules()->AddRule(kAltSvcHost, "127.0.0.1");
+    host_resolver_.reset(new MappedHostResolver(std::move(resolver)));
+    std::string map_rule = base::StringPrintf("MAP %s 127.0.0.1:%d",
+                                              kOriginHost, tcp_server_->port());
+    EXPECT_TRUE(host_resolver_->AddRuleFromString(map_rule));
+
+    std::unique_ptr<HttpNetworkSession::Params> params(
+        new HttpNetworkSession::Params);
+    params->cert_verifier = &cert_verifier_;
+    params->enable_quic = true;
+    params->enable_user_alternate_protocol_ports = true;
+    context_->set_host_resolver(host_resolver_.get());
+    context_->set_http_network_session_params(std::move(params));
+    context_->set_cert_verifier(&cert_verifier_);
+    context_->Init();
+  }
+
+  void TearDown() override {
+    if (quic_server_) {
+      quic_server_->Shutdown();

[nednguyen, 2017/05/27 00:28:39]

what about shutting down tcp_server_?

[xunjieli, 2017/05/29 20:16:00]

The destructor of EmbeddedTestServer calls ShutdownAndWaitUntilComplete().
I think it's okay not to call it explicitly.

[nednguyen, 2017/05/30 13:31:38]

I see. Can you add comment explaining why we don't need to explicitly shutdown
tcp_sever_?

Just curious, why doesn't QuickSever also call shutdown in destructor?

[xunjieli, 2017/06/02 20:43:59]

Done. 
I am not sure either :)

+      // If possible, deliver the conncetion close packet to the client before
+      // destruct the TestURLRequestContext.
+      base::RunLoop().RunUntilIdle();
+    }
+    mdm_.reset();
+    message_loop_.reset();
+  }
+
+  std::unique_ptr<URLRequest> CreateRequest(const GURL& url,
+                                            RequestPriority priority,
+                                            URLRequest::Delegate* delegate) {
+    return context_->CreateRequest(url, priority, delegate,
+                                   TRAFFIC_ANNOTATION_FOR_TESTS);
+  }
+
+  // Turns a Closure into a GlobalMemoryDumpCallback and takes care of posting
+  // the closure on the correct task runner.
+  void GlobalDumpCallbackAdapter(
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+      base::Closure closure,
+      uint64_t dump_guid,
+      bool success) {
+    task_runner->PostTask(FROM_HERE, closure);
+  }
+
+  void RequestGlobalDumpAndWait(
+      base::trace_event::MemoryDumpType dump_type,
+      base::trace_event::MemoryDumpLevelOfDetail level_of_detail) {
+    base::RunLoop run_loop;
+    base::trace_event::GlobalMemoryDumpCallback callback =
+        base::Bind(&URLRequestQuicPerfTest::GlobalDumpCallbackAdapter,
+                   base::Unretained(this), base::ThreadTaskRunnerHandle::Get(),
+                   run_loop.QuitClosure());
+    mdm_->RequestGlobalDump(dump_type, level_of_detail, callback);
+    run_loop.Run();
+  }
+
+  MemoryTestURLRequestContext* context() const { return context_.get(); }

[nednguyen, 2017/05/27 00:28:39]

why defining this if we can just use context_-> instead?

[xunjieli, 2017/05/29 20:16:00]

It's hard to reason about the tests if the tests start to modifying more member
variables. Ideally we should hide those member variables from the tests and
provide accessors for those that the tests need. I uploaded a new patch to do
the same for |global_dump_handler_| and |mdm_|, so hopefully it's more
consistent?

[nednguyen, 2017/05/30 13:31:38]

I think if the accessor just returns the non-const version of the member, then
it's essentially the same as accessing them directly. I do some code search and
see other unittests in Chromium also access private member directly:
https://cs.chromium.org/chromium/src/base/metrics/histogram_snapshot_manager_...

IIRC, the test class is instantiated once per test method, so we need not to
worry about all test methods mutating its state differently. 

Having said those, I don't code in Chrome C++ codebase much, so I defer to other
reviewers for the final call.

[xunjieli, 2017/06/02 20:43:59]

I hear you, but I find it easier to reason about test logic when members have
explicit accessors. This way, when people add more member fields, the members
are default to private.

+
+  const std::vector<base::trace_event::MemoryDumpCallbackResult>* GetResults()
+      const {
+    return &results_;
+  }
+
+  GlobalMemoryDumpHandler global_dump_handler_;
+  std::unique_ptr<base::trace_event::MemoryDumpManager> mdm_;
+
+ private:
+  void InitializeMemoryDumpManager() {
+    mdm_ = base::trace_event::MemoryDumpManager::CreateInstanceForTesting();
+    mdm_->set_dumper_registrations_ignored_for_testing(true);
+    mdm_->Initialize(
+        BindRepeating(&GlobalMemoryDumpHandler::RequestGlobalMemoryDump,
+                      base::Unretained(&global_dump_handler_)),
+        /*is_coordinator=*/false);
+  }
+
+  void StartQuicServer() {
+    net::QuicConfig config;
+    response_cache_.AddSimpleResponse(kOriginHost, kHelloPath, kHelloStatus,
+                                      kHelloAltSvcResponse);
+    quic_server_.reset(new QuicSimpleServer(
+        test::crypto_test_utils::ProofSourceForTesting(), config,
+        net::QuicCryptoServerConfig::ConfigOptions(), AllSupportedVersions(),
+        &response_cache_));
+    int rv = quic_server_->Listen(
+        net::IPEndPoint(net::IPAddress::IPv4AllZeros(), kAltSvcPort));
+    ASSERT_GE(rv, 0) << "Quic server fails to start";
+
+    CertVerifyResult verify_result;
+    verify_result.verified_cert = ImportCertFromFile(
+        GetTestCertsDirectory(), "quic_test.example.com.crt");
+    cert_verifier_.AddResultForCert(verify_result.verified_cert.get(),
+                                    verify_result, OK);
+  }
+
+  void StartTcpServer() {
+    tcp_server_ = base::MakeUnique<EmbeddedTestServer>(
+        net::EmbeddedTestServer::TYPE_HTTPS);
+    tcp_server_->RegisterRequestHandler(base::Bind(&HandleRequest));
+    ASSERT_TRUE(tcp_server_->Start()) << "HTTP/1.1 server fails to start";
+
+    CertVerifyResult verify_result;
+    verify_result.verified_cert = tcp_server_->GetCertificate();
+    cert_verifier_.AddResultForCert(tcp_server_->GetCertificate(),
+                                    verify_result, OK);
+  }
+
+  std::unique_ptr<MappedHostResolver> host_resolver_;
+  std::unique_ptr<EmbeddedTestServer> tcp_server_;
+  std::unique_ptr<QuicSimpleServer> quic_server_;
+  std::unique_ptr<base::MessageLoop> message_loop_;
+  std::unique_ptr<MemoryTestURLRequestContext> context_;
+  std::vector<base::trace_event::MemoryDumpCallbackResult> results_;
+  QuicHttpResponseCache response_cache_;
+  MockCertVerifier cert_verifier_;
+};
+
+}  // namespace
+
+TEST_F(URLRequestQuicPerfTest, TestGetRequest) {
+  bool quic_succeeded = false;
+  GURL url(base::StringPrintf("https://%s%s", kOriginHost, kHelloPath));
+  base::TimeTicks start = base::TimeTicks::Now();
+  const int kNumRequest = 1000;
+  for (int i = 0; i < kNumRequest; ++i) {
+    TestDelegate delegate;
+    std::unique_ptr<URLRequest> request =
+        CreateRequest(url, DEFAULT_PRIORITY, &delegate);
+
+    request->Start();
+    EXPECT_TRUE(request->is_pending());
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(request->status().is_success());
+    if (delegate.data_received() == kHelloAltSvcResponse) {
+      quic_succeeded = true;
+    } else {
+      EXPECT_EQ(kHelloOriginResponse, delegate.data_received());
+    }
+  }
+  base::TimeTicks end = base::TimeTicks::Now();
+  PrintPerfTest("time", (end - start).InMilliseconds() / kNumRequest, "ms");
+
+  EXPECT_TRUE(quic_succeeded);
+  base::trace_event::TraceLog::GetInstance()->SetEnabled(
+      base::trace_event::TraceConfig(
+          base::trace_event::MemoryDumpManager::kTraceCategory, ""),
+      base::trace_event::TraceLog::RECORDING_MODE);
+
+  EXPECT_CALL(global_dump_handler_, RequestGlobalMemoryDump(_, _))

[nednguyen, 2017/05/27 00:28:39]

silly question, what is this mock test for?

[xunjieli, 2017/05/29 20:16:00]

I am following the MemoryDumpManagerTest example. This handler adds a
RequestGlobalDumpFunction to intercept any request for a global dump, so it can
create a process dump with a custom callback to record the results (in
ProcessDumpRecordingCallbackAdapter).

[nednguyen, 2017/05/30 13:31:38]

It seems a bit unintuitive to me to rely on gtest EXPECT_CALL to just intercept
the request a global dump. 
However, if primiano@ thinks this is the supported way, I am ok :)

[xunjieli, 2017/06/02 20:43:59]

Acknowledged.

+      .WillOnce(Invoke([this](
+                           const base::trace_event::MemoryDumpRequestArgs& args,
+                           const base::trace_event::GlobalMemoryDumpCallback&
+                               callback) {
+        base::trace_event::ProcessMemoryDumpCallback process_callback =
+            Bind(&URLRequestQuicPerfTest::ProcessDumpRecordingCallbackAdapter,
+                 base::Unretained(this), callback);
+        mdm_->CreateProcessDump(args, process_callback);
+      }));
+
+  RequestGlobalDumpAndWait(
+      base::trace_event::MemoryDumpType::EXPLICITLY_TRIGGERED,
+      base::trace_event::MemoryDumpLevelOfDetail::LIGHT);
+  base::trace_event::TraceLog::GetInstance()->SetDisabled();
+
+  // Check ProcessMemoryDump stats.
+  ASSERT_EQ(1u, GetResults()->size());
+  base::trace_event::MemoryDumpCallbackResult result = GetResults()->front();
+  PrintPerfTest("chrome_malloc_total_kb", result.chrome_dump.malloc_total_kb,
+                "kB");
+  PrintPerfTest("os_resident_set_kb", result.os_dump.resident_set_kb, "kB");
+
+  // Check URLRequestContext::OnMemoryDump() stats.
+  base::DictionaryValue* attrs;
+  ASSERT_TRUE(
+      context()->url_request_context_dump_attrs_->GetAsDictionary(&attrs));
+  base::DictionaryValue* object_count_attrs;
+  ASSERT_TRUE(attrs->GetDictionary(
+      base::trace_event::MemoryAllocatorDump::kNameObjectCount,
+      &object_count_attrs));
+  std::string object_count_str;
+  ASSERT_TRUE(object_count_attrs->GetString("value", &object_count_str));
+  EXPECT_EQ("0", object_count_str);
+
+  ASSERT_TRUE(context()->quic_stream_factory_attrs_->GetAsDictionary(&attrs));
+  ASSERT_TRUE(attrs->GetDictionary("active_jobs", &object_count_attrs));
+  ASSERT_TRUE(object_count_attrs->GetString("value", &object_count_str));
+  EXPECT_EQ("0", object_count_str);
+  int object_count = -1;
+  ASSERT_TRUE(base::HexStringToInt(object_count_str, &object_count));
+  PrintPerfTest("active_quic_jobs", object_count, "count");
+
+  ASSERT_TRUE(attrs->GetDictionary("all_sessions", &object_count_attrs));
+  ASSERT_TRUE(object_count_attrs->GetString("value", &object_count_str));
+  EXPECT_EQ("1", object_count_str);
+  ASSERT_TRUE(base::HexStringToInt(object_count_str, &object_count));
+  PrintPerfTest("active_quic_sessions", object_count, "count");
+}
+
+}  // namespace net