Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2190943002:
cc serialization perf: Add a couple more tests (Closed)
|
Created:
4 years, 4 months ago by Fady Samuel Modified:
4 years, 4 months ago Reviewers:
ajuma CC:
chromium-reviews, cc-bugs_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncc serialization perf: Add a couple more tests
Add two more tests that better represent real world CompositorFrame sizes
1. 100 Quads * 5 RenderPasses
2. 500 Quads * 10 RenderPasses
BUG=624459
Committed: https://crrev.com/0ef0ba7f069d3474c3b4aa7a7e9224d6f499da8f
Cr-Commit-Position: refs/heads/master@{#408660}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Addressed Ali's comments #
Total comments: 2
Patch Set 3 : Addressed Ali's comment #Messages
Total messages: 24 (11 generated)
Description was changed from ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 ========== to ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel ==========
fsamuel@chromium.org changed reviewers: + ajuma@chromium.org
FYI, Mojo is currently about 70% lower in these more realistic tests, but I'm working on optimizations
https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... File cc/ipc/cc_serialization_perftest.cc (right): https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... cc/ipc/cc_serialization_perftest.cc:303: RunTest("DelegatedFrame_ManyRenderPasses_10000_100", std::move(frame)); s/10000/5 https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... cc/ipc/cc_serialization_perftest.cc:327: RunTest("DelegatedFrame_ManyRenderPasses_10000_100", std::move(frame)); s/10000_100/10_500 https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... cc/ipc/cc_serialization_perftest.cc:330: TEST_F(CCSerializationPerfTest, DelegatedFrame_ManyRenderPasses_10000_100) { Since this and the two new tests all have the same logic aside from two constants, wdyt of extracting that shared logic into a helper?
PTAL Ali! Thanks! https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... File cc/ipc/cc_serialization_perftest.cc (right): https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... cc/ipc/cc_serialization_perftest.cc:303: RunTest("DelegatedFrame_ManyRenderPasses_10000_100", std::move(frame)); On 2016/07/28 14:19:13, ajuma wrote: > s/10000/5 Done. https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... cc/ipc/cc_serialization_perftest.cc:327: RunTest("DelegatedFrame_ManyRenderPasses_10000_100", std::move(frame)); On 2016/07/28 14:19:13, ajuma wrote: > s/10000_100/10_500 > Done. https://codereview.chromium.org/2190943002/diff/1/cc/ipc/cc_serialization_per... cc/ipc/cc_serialization_perftest.cc:330: TEST_F(CCSerializationPerfTest, DelegatedFrame_ManyRenderPasses_10000_100) { On 2016/07/28 14:19:13, ajuma wrote: > Since this and the two new tests all have the same logic aside from two > constants, wdyt of extracting that shared logic into a helper? Done.
lgtm https://codereview.chromium.org/2190943002/diff/20001/cc/ipc/cc_serialization... File cc/ipc/cc_serialization_perftest.cc (right): https://codereview.chromium.org/2190943002/diff/20001/cc/ipc/cc_serialization... cc/ipc/cc_serialization_perftest.cc:250: RunCompositorFrameTest("DelegatedFrame_ManyRenderPasses_10000_100", 100, 1000, Should the constant here be 10000 rather than 1000 to match the name of the test? (1000 does match the existing logic, but seems unexpected.)
CQ'ing https://codereview.chromium.org/2190943002/diff/20001/cc/ipc/cc_serialization... File cc/ipc/cc_serialization_perftest.cc (right): https://codereview.chromium.org/2190943002/diff/20001/cc/ipc/cc_serialization... cc/ipc/cc_serialization_perftest.cc:250: RunCompositorFrameTest("DelegatedFrame_ManyRenderPasses_10000_100", 100, 1000, On 2016/07/28 20:06:29, ajuma wrote: > Should the constant here be 10000 rather than 1000 to match the name of the > test? (1000 does match the existing logic, but seems unexpected.) Changed the name because I don't want to lose the test.
The CQ bit was checked by fsamuel@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org Link to the patchset: https://codereview.chromium.org/2190943002/#ps40001 (title: "Addressed Ali's comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Your CL was about to rely on recently removed CQ feature(s): * master.tryserver.blink builder linux_blink_rel was renamed to linux_precise_blink_rel (http://crbug.com/590036)
The CQ bit was checked by fsamuel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Your CL was about to rely on recently removed CQ feature(s): * master.tryserver.blink builder linux_blink_rel was renamed to linux_precise_blink_rel (http://crbug.com/590036)
Description was changed from ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel ========== to ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 ==========
The CQ bit was checked by fsamuel@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 ========== to ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 ========== to ========== cc serialization perf: Add a couple more tests Add two more tests that better represent real world CompositorFrame sizes 1. 100 Quads * 5 RenderPasses 2. 500 Quads * 10 RenderPasses BUG=624459 Committed: https://crrev.com/0ef0ba7f069d3474c3b4aa7a7e9224d6f499da8f Cr-Commit-Position: refs/heads/master@{#408660} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0ef0ba7f069d3474c3b4aa7a7e9224d6f499da8f Cr-Commit-Position: refs/heads/master@{#408660} |
