Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(10)

Issue 2766533002: Using multi-tab story in TabSwitching Benchmark (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months ago by vovoy
Modified:
2 months, 1 week ago
CC:
chromium-reviews, telemetry-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Using multi-tab story in TabSwitching Benchmark and rewrite measurement to fit the new multi-tab story BUG=689388 Review-Url: https://codereview.chromium.org/2766533002 Cr-Commit-Position: refs/heads/master@{#463563} Committed: https://chromium.googlesource.com/chromium/src/+/cd01f90472cd2aa9fa824c902f39d0b9e50e4fba

Patch Set 1 #

Patch Set 2 : Use the new story for TabSwitching Benchmark #

Total comments: 2

Patch Set 3 : Use the new story for TabSwitching Benchmark #

Patch Set 4 : Use the new story for TabSwitching Benchmark #

Patch Set 5 : Use the new story for TabSwitching Benchmark #

Patch Set 6 : Use the new story for TabSwitching Benchmark #

Total comments: 9

Patch Set 7 : Add integration test #

Patch Set 8 : Add integration test #

Patch Set 9 : Add Owner info #

Patch Set 10 : Disable IT on some platforms #

Total comments: 4

Patch Set 11 : Minor change to unittest #

Patch Set 12 : fix merge conflict #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -110 lines) Patch
M tools/perf/benchmarks/tab_switching.py View 1 2 3 4 5 6 7 8 2 chunks +4 lines, -1 line 0 comments Download
M tools/perf/measurements/tab_switching.py View 1 2 1 chunk +17 lines, -87 lines 0 comments Download
M tools/perf/measurements/tab_switching_unittest.py View 1 2 3 4 5 6 7 8 9 10 4 chunks +40 lines, -20 lines 0 comments Download
M tools/perf/page_sets/system_health/multi_tab_stories.py View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -2 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 89 (65 generated)
vovoy
I would like to refactor tab_switching benchmark and measure so it would be easier to ...
3 months ago (2017-03-21 08:18:38 UTC) #11
nednguyen
How about just keep one single tab_switching benchmark which is TabSwtiching2? https://codereview.chromium.org/2766533002/diff/20001/tools/perf/measurements/tab_switching.py File tools/perf/measurements/tab_switching.py (right): ...
3 months ago (2017-03-21 14:46:12 UTC) #12
vovoy
On 2017/03/21 14:46:12, nednguyen wrote: > How about just keep one single tab_switching benchmark which ...
3 months ago (2017-03-22 06:30:13 UTC) #13
vovoy
the second part: refactoring tab_switching measurement https://codereview.chromium.org/2766533002/diff/20001/tools/perf/measurements/tab_switching.py File tools/perf/measurements/tab_switching.py (right): https://codereview.chromium.org/2766533002/diff/20001/tools/perf/measurements/tab_switching.py#newcode179 tools/perf/measurements/tab_switching.py:179: self._power_metric.Close() On 2017/03/21 ...
3 months ago (2017-03-23 09:37:49 UTC) #31
nednguyen
https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py File tools/perf/benchmarks/tab_switching.py (right): https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py#newcode14 tools/perf/benchmarks/tab_switching.py:14: @benchmark.Disabled('android') # http://crbug.com/460084 Can you add @benchmark.Owner(..) for this? ...
3 months ago (2017-03-23 13:15:10 UTC) #32
nednguyen
3 months ago (2017-03-23 13:15:12 UTC) #33
vovoy
https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py File tools/perf/benchmarks/tab_switching.py (right): https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py#newcode14 tools/perf/benchmarks/tab_switching.py:14: @benchmark.Disabled('android') # http://crbug.com/460084 On 2017/03/23 13:15:10, nednguyen wrote: > ...
3 months ago (2017-03-23 15:23:41 UTC) #34
nednguyen
On 2017/03/23 15:23:41, vovoy wrote: > https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py > File tools/perf/benchmarks/tab_switching.py (right): > > https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py#newcode14 > ...
3 months ago (2017-03-23 15:24:07 UTC) #35
vovoy
On 2017/03/23 15:24:07, nednguyen wrote: > On 2017/03/23 15:23:41, vovoy wrote: > > > https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py ...
3 months ago (2017-03-23 16:14:16 UTC) #36
vovoy
https://codereview.chromium.org/2766533002/diff/100001/tools/perf/measurements/tab_switching_unittest.py File tools/perf/measurements/tab_switching_unittest.py (right): https://codereview.chromium.org/2766533002/diff/100001/tools/perf/measurements/tab_switching_unittest.py#newcode72 tools/perf/measurements/tab_switching_unittest.py:72: mock_get_histogram = mock.MagicMock(side_effect=expected_histogram) On 2017/03/23 13:15:10, nednguyen wrote: > ...
3 months ago (2017-03-23 16:55:58 UTC) #37
vovoy
On 2017/03/23 15:24:07, nednguyen wrote: > On 2017/03/23 15:23:41, vovoy wrote: > > > https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py ...
3 months ago (2017-03-23 23:46:02 UTC) #39
nednguyen
https://codereview.chromium.org/2766533002/diff/100001/tools/perf/measurements/tab_switching_unittest.py File tools/perf/measurements/tab_switching_unittest.py (right): https://codereview.chromium.org/2766533002/diff/100001/tools/perf/measurements/tab_switching_unittest.py#newcode72 tools/perf/measurements/tab_switching_unittest.py:72: mock_get_histogram = mock.MagicMock(side_effect=expected_histogram) On 2017/03/23 16:55:58, vovoy wrote: > ...
3 months ago (2017-03-23 23:49:06 UTC) #40
vovoy
https://codereview.chromium.org/2766533002/diff/100001/tools/perf/measurements/tab_switching_unittest.py File tools/perf/measurements/tab_switching_unittest.py (right): https://codereview.chromium.org/2766533002/diff/100001/tools/perf/measurements/tab_switching_unittest.py#newcode72 tools/perf/measurements/tab_switching_unittest.py:72: mock_get_histogram = mock.MagicMock(side_effect=expected_histogram) On 2017/03/23 23:49:06, nednguyen wrote: > ...
3 months ago (2017-03-24 00:25:51 UTC) #41
vovoy
added integration test and owner info https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py File tools/perf/benchmarks/tab_switching.py (right): https://codereview.chromium.org/2766533002/diff/100001/tools/perf/benchmarks/tab_switching.py#newcode14 tools/perf/benchmarks/tab_switching.py:14: @benchmark.Disabled('android') # http://crbug.com/460084 ...
3 months ago (2017-03-24 16:35:31 UTC) #59
nednguyen
lgtm with nits https://codereview.chromium.org/2766533002/diff/180001/tools/perf/measurements/tab_switching_unittest.py File tools/perf/measurements/tab_switching_unittest.py (right): https://codereview.chromium.org/2766533002/diff/180001/tools/perf/measurements/tab_switching_unittest.py#newcode113 tools/perf/measurements/tab_switching_unittest.py:113: self.assertEquals(summary.name, 'MPArch_RWH_TabSwitchPaintDuration') assert this is greater ...
2 months, 3 weeks ago (2017-03-27 17:10:43 UTC) #62
vovoy
https://codereview.chromium.org/2766533002/diff/180001/tools/perf/measurements/tab_switching_unittest.py File tools/perf/measurements/tab_switching_unittest.py (right): https://codereview.chromium.org/2766533002/diff/180001/tools/perf/measurements/tab_switching_unittest.py#newcode113 tools/perf/measurements/tab_switching_unittest.py:113: self.assertEquals(summary.name, 'MPArch_RWH_TabSwitchPaintDuration') On 2017/03/27 17:10:43, nednguyen wrote: > assert ...
2 months, 3 weeks ago (2017-03-28 13:02:56 UTC) #67
nednguyen
On 2017/03/28 13:02:56, vovoy wrote: > https://codereview.chromium.org/2766533002/diff/180001/tools/perf/measurements/tab_switching_unittest.py > File tools/perf/measurements/tab_switching_unittest.py (right): > > https://codereview.chromium.org/2766533002/diff/180001/tools/perf/measurements/tab_switching_unittest.py#newcode113 > ...
2 months, 3 weeks ago (2017-03-28 15:07:55 UTC) #72
nednguyen
On 2017/03/28 15:07:55, nednguyen (slow til 4-10) wrote: > On 2017/03/28 13:02:56, vovoy wrote: > ...
2 months, 1 week ago (2017-04-10 14:21:16 UTC) #73
vovoy
On 2017/04/10 14:21:16, nednguyen wrote: > On 2017/03/28 15:07:55, nednguyen (slow til 4-10) wrote: > ...
2 months, 1 week ago (2017-04-11 02:44:34 UTC) #74
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2766533002/200001
2 months, 1 week ago (2017-04-11 02:46:30 UTC) #76
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/188002) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
2 months, 1 week ago (2017-04-11 02:49:11 UTC) #78
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2766533002/220001
2 months, 1 week ago (2017-04-11 07:21:16 UTC) #85
commit-bot: I haz the power
Committed patchset #12 (id:220001) as https://chromium.googlesource.com/chromium/src/+/cd01f90472cd2aa9fa824c902f39d0b9e50e4fba
2 months, 1 week ago (2017-04-11 07:27:51 UTC) #88
Mike West (OOO until 19th)
2 months, 1 week ago (2017-04-11 10:58:23 UTC) #89
Message was sent while issue was closed.
A revert of this CL (patchset #12 id:220001) has been created in
https://codereview.chromium.org/2809813003/ by mkwst@chromium.org.

The reason for reverting is: It looks like this caused some breakage on Mac
telemetry_perf_unittests:
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/10856.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318