Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(990)

Issue 414443009: Update ProtocolPerfTest to run host and client on different threads (Closed)

Created:
6 years, 5 months ago by Sergey Ulanov
Modified:
6 years, 5 months ago
Reviewers:
rmsousa
CC:
chromium-reviews, chromoting-reviews_chromium.org
Project:
chromium
Visibility:
Public.

Description

Update ProtocolPerfTest to run host and client on different threads Previously the host and client were running on the main thread, so they both would compete for it and that skews the perf numbers, particularly with NSS SSL sockets because they use a separate thread. BUG=394067 R=rmsousa@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285738

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+140 lines, -56 lines) Patch
M remoting/signaling/fake_signal_strategy.h View 4 chunks +20 lines, -5 lines 0 comments Download
M remoting/signaling/fake_signal_strategy.cc View 1 2 3 4 chunks +55 lines, -31 lines 0 comments Download
M remoting/test/protocol_perftest.cc View 1 2 9 chunks +65 lines, -20 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Sergey Ulanov
6 years, 5 months ago (2014-07-23 00:29:25 UTC) #1
rmsousa
lgtm https://codereview.chromium.org/414443009/diff/20001/remoting/signaling/fake_signal_strategy.cc File remoting/signaling/fake_signal_strategy.cc (right): https://codereview.chromium.org/414443009/diff/20001/remoting/signaling/fake_signal_strategy.cc#newcode124 remoting/signaling/fake_signal_strategy.cc:124: buzz::XmlElement* stanza_ptr = stanza.get(); nit: DCHECK current thread?
6 years, 5 months ago (2014-07-23 00:49:37 UTC) #2
Sergey Ulanov
The CQ bit was checked by sergeyu@chromium.org
6 years, 5 months ago (2014-07-25 18:21:35 UTC) #3
Sergey Ulanov
The CQ bit was unchecked by sergeyu@chromium.org
6 years, 5 months ago (2014-07-25 18:21:44 UTC) #4
Sergey Ulanov
https://codereview.chromium.org/414443009/diff/20001/remoting/signaling/fake_signal_strategy.cc File remoting/signaling/fake_signal_strategy.cc (right): https://codereview.chromium.org/414443009/diff/20001/remoting/signaling/fake_signal_strategy.cc#newcode124 remoting/signaling/fake_signal_strategy.cc:124: buzz::XmlElement* stanza_ptr = stanza.get(); On 2014/07/23 00:49:37, rmsousa wrote: ...
6 years, 5 months ago (2014-07-25 18:23:16 UTC) #5
Sergey Ulanov
The CQ bit was checked by sergeyu@chromium.org
6 years, 5 months ago (2014-07-25 18:23:18 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyu@chromium.org/414443009/60001
6 years, 5 months ago (2014-07-25 18:24:19 UTC) #7
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_chromium_x64_rel on tryserver.chromium ...
6 years, 5 months ago (2014-07-25 20:26:15 UTC) #8
Sergey Ulanov
6 years, 5 months ago (2014-07-26 00:44:22 UTC) #9
Message was sent while issue was closed.
Committed patchset #4 manually as r285738 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698