New issue
Advanced search Search tips

Issue 634013 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

cc_perftests failure on chromium.perf: Linux/Android

Project Member Reported by simonhatch@chromium.org, Aug 3 2016

Issue description

https://build.chromium.org/p/chromium.perf/builders/Linux%20Perf%20%283%29


See a couple changes to cc in the range

danakj: https://codereview.chromium.org/2193293004 seems like a possibility?

+cc owners
 
Ya it def is, those tests are also trying to disable delegated rendering which isn't a thing and would DCHECK shortly.

10 tests failed:
    BrowserCompositorInvalidateLayerTreePerfTest.DenseBrowserUIThreaded (../../cc/trees/layer_tree_host_perftest.cc:328)
    LayerTreeHostPerfTestJsonReader.HeavyPageThreaded (../../cc/trees/layer_tree_host_perftest.cc:336)
    LayerTreeHostPerfTestJsonReader.TenTenSingleThread (../../cc/trees/layer_tree_host_perftest.cc:151)
    LayerTreeHostPerfTestJsonReader.TenTenSingleThread_FullDamageEachFrame (../../cc/trees/layer_tree_host_perftest.cc:165)
    LayerTreeHostPerfTestJsonReader.TenTenThreaded (../../cc/trees/layer_tree_host_perftest.cc:157)
    LayerTreeHostPerfTestJsonReader.TenTenThreaded_FullDamageEachFrame (../../cc/trees/layer_tree_host_perftest.cc:172)
    LayerTreeHostPerfTestLeafInvalidates.TenTenSingleThread (../../cc/trees/layer_tree_host_perftest.cc:207)
    LayerTreeHostPerfTestLeafInvalidates.TenTenThreaded (../../cc/trees/layer_tree_host_perftest.cc:213)
    ScrollingLayerTreePerfTest.LongScrollablePageSingleThread (../../cc/trees/layer_tree_host_perftest.cc:244)
    ScrollingLayerTreePerfTest.LongScrollablePageThreaded (../../cc/trees/layer_tree_host_perftest.cc:250)

I think we could just disable these and I'll take a look at them when I'm back next week.
I think the problem is the tests try to run until they do 2s of thread time, instead of 2s of wall clock time. And now with a delegating renderer, they don't do 2s of thread work anymore.
If you can point me in the right direction I can disable those tests.
I don't think that patch changed this, so I'm not sure why it came up there, but disable_display_vsync moved to GPTF, and these tests set it. So it's not doing anything. If I make TDOS respect it and use a BackToBackBFS then they pass again.
@simon I think they are all in layer_tree_host_perftest.cc.

Summary: cc_perftests failure on chromium.perf: Linux/Android (was: cc_perftests failure on chromium.perf: Linux Perf (1))
I have a CL that I think fixes all the tests. They will probably give different numbers now than they used to but that's fine. They used to have a direct renderer and were measuring drawing time.

I considered measuring till the Display draws/swaps but many of the tests don't actually damage things and are just testing PrepareToDraw essentially. :)
Owner: danakj@chromium.org
Oh awesome, thanks for moving so quickly on this! I won't bother disabling the tests then.

=== Auto-CCing suspected CL author danakj@chromium.org ===

Hi danakj@chromium.org, the bisect results pointed to your CL below as possibly
causing a regression. Please have a look at this info and see whether
your CL be related.


===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : cc: Make LayerTreeTests use a DelegatingRenderer and Display.
Author  : danakj
Commit description:
  
Wherein we rewrite a few thousand cc unit_tests to not use a
DirectRenderer in LayerTreeHostImpl so that we can remove that
possibility from the codebase.

Currently some LayerTreeTests use a DirectRenderer in the
LayerTreeHostImpl, which is the last case of this occuring. Instead
give all LayerTreeTests a TestDelegatingOutputSurface (which means
a DelegatingRenderer in LayerTreeHostImpl). TestDelegatingOutputSurface
delegates drawing to a DirectRenderer via Display, which matches how
things "really work" now.

This means SwapBuffers hooks become async from operations on
LayerTreeHostImpl, so we introduce 3 new hooks for tests:
- DisplayReceivedCompositorFrameOnThread.
- DisplayWillDrawAndSwapOnThread.
- DisplayDidDrawAndSwapOnThread.
None of these receive a LayerTreeHostImpl* since they are async
from it and so using its state from the hook would be racey. These
hooks come from the TestDelegatingOutputSurface instead of from the
LayerTreeHostImpl.

LayerTreeTest gets two methods that can be overridden to control the
output surface:
- CreateDelegatingOutputSurface makes a TestDelegatingOutputSurface
so tests can control the arguments passed to its constructor, or the
ContextProviders given to it.
- CreateDisplayOutputSurface makes an OutputSurface subclass that
will be owned by the Display and used by the DirectRenderer. This
allows tests to control which ContextProvider it uses, and what
subclass of OutputSurface to use, to allow mocking out things as
desired.

R=enne
BUG= 606056 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2193293004
Cr-Commit-Position: refs/heads/master@{#409270}
Commit  : cae1058e62b7960796709512c4e1e650b6389c0f
Date    : Tue Aug 02 19:28:36 2016


===== TESTED REVISIONS =====
Revision         Exit Code  Std Dev  N  Good?
chromium@409228  0          N/A      5  good
chromium@409260  0          N/A      5  good
chromium@409268  0          N/A      5  good
chromium@409269  0          N/A      5  good
chromium@409270  1          N/A      5  bad    <--
chromium@409272  1          N/A      5  bad
chromium@409276  1          N/A      5  bad
chromium@409291  1          N/A      5  bad

Bisect job ran on: linux_perf_bisect
Bug ID: 634013

Test Command: ./src/out/Release/cc_perftests --test-launcher-print-test-stdio=always
Test Metric: prepare_tiles/10_100
Relative Change: Zero to non-zero
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bisect/builds/6617
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005331653485854208


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5801390245412864

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!
Labels: -Pri-1 Pri-2
The revert (https://codereview.chromium.org/2208693003/) has landed, so I'm lowering the priority of this bug.

danakj: Please fix the issue and reland your patch.
Status: Assigned (was: Untriaged)
Status: Fixed (was: Assigned)

Sign in to add a comment