diff options
author | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-09 19:13:03 +0000 |
---|---|---|
committer | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-09 19:13:03 +0000 |
commit | 8c2cfd6162bedfb3f8afba91ba2aad8fb24291f3 (patch) | |
tree | 5105c73b6e512ae053f0a5991d999e07f708693d /net | |
parent | a7e0f34e0c32f45f80c1cfde71ff6df622fcb4f0 (diff) | |
download | chromium_src-8c2cfd6162bedfb3f8afba91ba2aad8fb24291f3.zip chromium_src-8c2cfd6162bedfb3f8afba91ba2aad8fb24291f3.tar.gz chromium_src-8c2cfd6162bedfb3f8afba91ba2aad8fb24291f3.tar.bz2 |
Fix end_to_end_test performance regression caused by using mutexes
instead of notifications in server_thread. Testing only.
Merge internal change: 59322530
Reversed pause/resume changes of server_thread in the following CL:
https://codereview.chromium.org/127503002/
R=rch@chromium.org
Review URL: https://codereview.chromium.org/132073002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243948 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/quic/quic_end_to_end_unittest.cc | 4 | ||||
-rw-r--r-- | net/tools/quic/end_to_end_test.cc | 17 | ||||
-rw-r--r-- | net/tools/quic/test_tools/server_thread.cc | 49 | ||||
-rw-r--r-- | net/tools/quic/test_tools/server_thread.h | 22 |
4 files changed, 54 insertions, 38 deletions
diff --git a/net/quic/quic_end_to_end_unittest.cc b/net/quic/quic_end_to_end_unittest.cc index d6bff8a..1f5cc0d 100644 --- a/net/quic/quic_end_to_end_unittest.cc +++ b/net/quic/quic_end_to_end_unittest.cc @@ -135,10 +135,10 @@ class QuicEndToEndTest : public PlatformTest { server_thread_.reset(new ServerThread(server_address_, server_config_, QuicSupportedVersions(), strike_register_no_startup_period_)); - server_thread_->Start(); - server_thread_->WaitForServerStartup(); + server_thread_->Initialize(); server_address_ = IPEndPoint(server_address_.address(), server_thread_->GetPort()); + server_thread_->Start(); server_started_ = true; } diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index dd40344..f33deb7 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -218,19 +218,15 @@ class EndToEndTest : public ::testing::TestWithParam<TestParams> { server_thread_.reset(new ServerThread(server_address_, server_config_, server_supported_versions_, strike_register_no_startup_period_)); - server_thread_->Start(); - server_thread_->WaitForServerStartup(); + server_thread_->Initialize(); server_address_ = IPEndPoint(server_address_.address(), server_thread_->GetPort()); QuicDispatcher* dispatcher = QuicServerPeer::GetDispatcher(server_thread_->server()); + QuicDispatcherPeer::UseWriter(dispatcher, server_writer_); server_writer_->SetConnectionHelper( QuicDispatcherPeer::GetHelper(dispatcher)); - // TODO(rtenneti): Enable server_thread's Pause/Resume. - // server_thread_->Pause(); - QuicDispatcherPeer::UseWriter(dispatcher, server_writer_); - // TODO(rtenneti): Enable server_thread's Pause/Resume. - // server_thread_->Resume(); + server_thread_->Start(); server_started_ = true; } @@ -728,9 +724,7 @@ TEST_P(EndToEndTest, InitialRTT) { client_->client()->WaitForCryptoHandshakeConfirmed(); server_thread_->WaitForCryptoHandshakeConfirmed(); - // Pause the server so we can access the server's internals without races. - // TODO(rtenneti): Enable server_thread's Pause/Resume. - // server_thread_->Pause(); + server_thread_->Pause(); QuicDispatcher* dispatcher = QuicServerPeer::GetDispatcher(server_thread_->server()); ASSERT_EQ(1u, dispatcher->session_map().size()); @@ -751,8 +745,7 @@ TEST_P(EndToEndTest, InitialRTT) { EXPECT_FALSE(client_sent_packet_manager.SmoothedRtt().IsInfinite()); EXPECT_GE(static_cast<int64>(kMaxInitialRoundTripTimeUs), server_sent_packet_manager.SmoothedRtt().ToMicroseconds()); - // TODO(rtenneti): Enable server_thread's Pause/Resume. - // server_thread_->Resume(); + server_thread_->Resume(); } TEST_P(EndToEndTest, ResetConnection) { diff --git a/net/tools/quic/test_tools/server_thread.cc b/net/tools/quic/test_tools/server_thread.cc index ecc8ccd..e2b6506 100644 --- a/net/tools/quic/test_tools/server_thread.cc +++ b/net/tools/quic/test_tools/server_thread.cc @@ -15,33 +15,48 @@ ServerThread::ServerThread(IPEndPoint address, const QuicVersionVector& supported_versions, bool strike_register_no_startup_period) : SimpleThread("server_thread"), - listening_(true, false), confirmed_(true, false), + pause_(true, false), + paused_(true, false), + resume_(true, false), quit_(true, false), server_(config, supported_versions), address_(address), - port_(0) { + port_(0), + initialized_(false) { if (strike_register_no_startup_period) { server_.SetStrikeRegisterNoStartupPeriod(); } } -ServerThread::~ServerThread() { -} +ServerThread::~ServerThread() {} + +void ServerThread::Initialize() { + if (initialized_) { + return; + } -void ServerThread::Run() { server_.Listen(address_); port_lock_.Acquire(); port_ = server_.port(); port_lock_.Release(); - listening_.Signal(); + initialized_ = true; +} + +void ServerThread::Run() { + if (!initialized_) { + Initialize(); + } + while (!quit_.IsSignaled()) { - event_loop_mu_.Acquire(); + if (pause_.IsSignaled() && !resume_.IsSignaled()) { + paused_.Signal(); + resume_.Wait(); + } server_.WaitForEvents(); MaybeNotifyOfHandshakeConfirmation(); - event_loop_mu_.Release(); } server_.Shutdown(); @@ -51,11 +66,7 @@ int ServerThread::GetPort() { port_lock_.Acquire(); int rc = port_; port_lock_.Release(); - return rc; -} - -void ServerThread::WaitForServerStartup() { - listening_.Wait(); + return rc; } void ServerThread::WaitForCryptoHandshakeConfirmed() { @@ -63,15 +74,21 @@ void ServerThread::WaitForCryptoHandshakeConfirmed() { } void ServerThread::Pause() { - event_loop_mu_.Acquire(); + DCHECK(!pause_.IsSignaled()); + pause_.Signal(); + paused_.Wait(); } void ServerThread::Resume() { - event_loop_mu_.AssertAcquired(); // Checks the calling thread only! - event_loop_mu_.Release(); + DCHECK(!resume_.IsSignaled()); + DCHECK(pause_.IsSignaled()); + resume_.Signal(); } void ServerThread::Quit() { + if (pause_.IsSignaled() && !resume_.IsSignaled()) { + resume_.Signal(); + } quit_.Signal(); } diff --git a/net/tools/quic/test_tools/server_thread.h b/net/tools/quic/test_tools/server_thread.h index 520fffb..c2c4c61 100644 --- a/net/tools/quic/test_tools/server_thread.h +++ b/net/tools/quic/test_tools/server_thread.h @@ -24,19 +24,22 @@ class ServerThread : public base::SimpleThread { virtual ~ServerThread(); - // SimpleThread implementation. - virtual void Run() OVERRIDE; + // Prepares the server, but does not start accepting connections. Useful for + // injecting mocks. + void Initialize(); - // Waits until the server has started and is listening for requests. - void WaitForServerStartup(); + // Runs the event loop. Will initialize if necessary. + virtual void Run() OVERRIDE; // Waits for the handshake to be confirmed for the first session created. void WaitForCryptoHandshakeConfirmed(); - // Pauses execution of the server until Resume() is called. + // Pauses execution of the server until Resume() is called. May only be + // called once. void Pause(); - // Resumes execution of the server after Pause() has been called. + // Resumes execution of the server after Pause() has been called. May only + // be called once. void Resume(); // Stops the server from executing and shuts it down, destroying all @@ -54,10 +57,11 @@ class ServerThread : public base::SimpleThread { private: void MaybeNotifyOfHandshakeConfirmation(); - base::Lock event_loop_mu_; // Held when the server is processing events. - base::WaitableEvent listening_; // Notified when the server is listening. base::WaitableEvent confirmed_; // Notified when the first handshake is // confirmed. + base::WaitableEvent pause_; // Notified when the server should pause. + base::WaitableEvent paused_; // Notitied when the server has paused + base::WaitableEvent resume_; // Notified when the server should resume. base::WaitableEvent quit_; // Notified when the server should quit. tools::QuicServer server_; @@ -65,6 +69,8 @@ class ServerThread : public base::SimpleThread { base::Lock port_lock_; int port_; + bool initialized_; + DISALLOW_COPY_AND_ASSIGN(ServerThread); }; |