New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 763029 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----

Blocked on:
issue 763853
issue 763854



Sign in to add a comment

v8.runtimestats.browsing_desktop failing on 5 builders

Project Member Reported by charliea@chromium.org, Sep 7 2017

Issue description

v8.runtimestats.browsing_desktop failing on 5 builders

Builders failed on: 
- Mac Retina Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Mac%20Retina%20Perf
- Win 10 Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Win%2010%20Perf
- Win 7 Intel GPU Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Win%207%20Intel%20GPU%20Perf
- Win 7 x64 Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Win%207%20x64%20Perf
- Win 8 Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf

Win8 perf seems to be one of the consistently failing platforms, so I'm going to kick off a bisect.
 
Never mind: not necessary. They were reenabled yesterday in crrev.com/c/635587. Landing the disable now in https://chromium-review.googlesource.com/c/chromium/src/+/655808
Owner: mythria@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7c2478a9d32c09251bd2ec0e2a2cd92134d9c0d

commit c7c2478a9d32c09251bd2ec0e2a2cd92134d9c0d
Author: Charlie Andrews <charliea@chromium.org>
Date: Thu Sep 07 20:41:20 2017

Disable v8.browsing_desktop browse:tools:earth and maps

They were reenabled yesterday in crrev.com/c/635587, but are still
failing across desktop platforms.

TBR=nednguyen@google.com, mythria@chromium.org

Bug: 708590,  712694 ,  763029 
Change-Id: Ie897089f88118f043a20279e7a63f42c30a86735
Reviewed-on: https://chromium-review.googlesource.com/655808
Reviewed-by: Charlie Andrews <charliea@chromium.org>
Commit-Queue: Charlie Andrews <charliea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500372}
[modify] https://crrev.com/c7c2478a9d32c09251bd2ec0e2a2cd92134d9c0d/tools/perf/page_sets/system_health/expectations.py

Thanks for disabling the stories. Both of them seem to be failing when waiting for a zoom in button. They were working fine locally for me. I will take a look at them.
Cc: rnep...@chromium.org nednguyen@chromium.org
I think there are two different problems here. On Mac Retina Perf, earth story seems to work fine but maps times out when waiting for an event after scrolling. I will create another bug to track this. I will keep this bug to track failures on rest of them.

On the rest of the builders, both maps and earth fail right at the beginning. 
In my understanding these pages should be skipped on these bots because they
do not have a GPU enabled. On all of these builders smoothness.maps page gets skipped which also checks for a GPU. For example: 

https://luci-logdog.appspot.com/v/?s=chrome%2Fbb%2Fchromium.perf%2FWin_8_Perf%2F1218%2F%2B%2Frecipes%2Fsteps%2Fsmoothness.maps_on__102b__GPU_on_Windows_on_Windows-2012ServerR2-SP0%2F0%2Fstdout

Somehow for both maps and earth the following check I added to skip these stories is not working:

https://cs.chromium.org/chromium/src/tools/perf/page_sets/system_health/browsing_stories.py?q=browsing_stor&sq=package:chromium&l=678

@rnephew, any ideas on why this check isn't working?
I did another quick check locally. The story does not get disabled even when I return False unconditionally from CanRunOnBrowser. CanRunOnBrowser method on these pages does not get invoked.
Blockedon: 763854
Blockedon: 763853
Hmm.. It looks like CanRunOnBrowser is called in CanRunStory in the default shared_page_state.py:
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/page/shared_page_state.py?q=CanRunOnBrowser&sq=package:chromium&dr=CSs&l=284

CanRunStory is used in story_runner here:
https://cs.chromium.org/chromium/src/third_party/android_tools/sdk/platform-tools/systrace/catapult/telemetry/telemetry/internal/story_runner.py?q=CanRunStory&sq=package:chromium&dr=C&l=83

Is it possible that whatever shared_state is being used has CanRunStory overridden to not check CanRunOnBrowser? I'm not sure where the shared state for v8 browsing tests is set.


It looks like that shared state was only used to disable stories for system health which story expectations now does. If thats true, could we switch to the default shared_page_state?
That is what  issue 763854  about
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c829ec04abeb4bccb72a8bf142ef52252876e1b0

commit c829ec04abeb4bccb72a8bf142ef52252876e1b0
Author: rnephew <rnephew@chromium.org>
Date: Mon Sep 11 23:31:30 2017

[Telemetry] Get rid of system health specific shared page state.

Before it was used to be able to disable specific system health stories. This
feature is now done via StoryExpectations for all benchmark types so this
special shared page state is no longer required.

Bug:  763854 ,  763029 
Change-Id: Ibf77fe68c2ce964eeaf9f8f6ebdb4c7ee41f1029
Reviewed-on: https://chromium-review.googlesource.com/661326
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: rnephew <rnephew@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501095}
[modify] https://crrev.com/c829ec04abeb4bccb72a8bf142ef52252876e1b0/tools/perf/page_sets/system_health/platforms.py
[modify] https://crrev.com/c829ec04abeb4bccb72a8bf142ef52252876e1b0/tools/perf/page_sets/system_health/system_health_story.py

With that landed, I would expect CanRunOnBrowser to behave as expected now. If thats not the case let me know.
I tried it locally, it still does not seem to work. I updated my checkout and CanRunOnBrowser still does not get invoked on the earth page. It still invokes the CanRunOnBrowser in the shared_page_state.py. (https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/page/shared_page_state.py?type=cs&q=SharedPageS&sq=package:chromium&l=288)
Cc: -nednguyen@chromium.org nedngu...@google.com
Which CanRunOnBrowser is the correct one. Its now using the CanRunStory that runs CanRunOnBrowser, but until we override that to be the correct CanRunOnBrowser it will always return True.
Or, now I remember that CanRunOnBrowser is specified at SharedPageState level, and not at page level. So there are two ways moving forward here:
1) Make all the page support CanRunOnBrowser. Which means we rewrite SharedPageState.CanRunStory to do:

def CanRunStory(self, browser_info, page):
  return page.CanRunOnBrowser

or we can override shared_page_state.CanRunOnBrowser of all system health stories to this:

def CanRunOnBrowser(self, browser_info, page):
   if page.NEED_WEBGL_SUPPORT:
      return browser_info.HasWebGLSupport()
   return True

With this, you would tag your pages with NEED_WEBGL_SUPPORT = True
I am happy to provide a patch for either of the approaches. What would be the preferred method? I prefer the second, since all the stories that don't need any additional restrictions need not implement CanRunOnBrowser. 
Cc: perezju@chromium.org
+1, yeah checking for the presence of the WEBGL tag before running the story sgtm.
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8e75578b095d0242d6fdaaf361dd0f299cbea75

commit c8e75578b095d0242d6fdaaf361dd0f299cbea75
Author: Mythri Alle <mythria@chromium.org>
Date: Wed Sep 27 14:57:37 2017

Add a shared page state for system health stories

Add a shared page state for system health stories to disable
some of the stories on certain platforms. Currently this is used
to disable stories that needs webgl on platforms that does not
have webgl support.

Bug:  chromium:763029 
Change-Id: Id78875ec6db6c2c2db51da91a91127562bf1c98b
Reviewed-on: https://chromium-review.googlesource.com/684898
Commit-Queue: Mythri Alle <mythria@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#504672}
[modify] https://crrev.com/c8e75578b095d0242d6fdaaf361dd0f299cbea75/tools/perf/page_sets/system_health/browsing_stories.py
[modify] https://crrev.com/c8e75578b095d0242d6fdaaf361dd0f299cbea75/tools/perf/page_sets/system_health/system_health_story.py

Project Member

Comment 24 by bugdroid1@chromium.org, Oct 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/079ad9866b4daa6ad36ce74b3487ac69561734d4

commit 079ad9866b4daa6ad36ce74b3487ac69561734d4
Author: Mythri Alle <mythria@chromium.org>
Date: Mon Oct 02 16:29:38 2017

Enable maps and earth stories in system health and v8 browse benchmarks


Bug:  chromium:763029 
Change-Id: I2f547bd04663c567468572d20107181e3afb8806
Reviewed-on: https://chromium-review.googlesource.com/687077
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505639}
[modify] https://crrev.com/079ad9866b4daa6ad36ce74b3487ac69561734d4/tools/perf/page_sets/system_health/expectations.py

Status: Fixed (was: Assigned)
Both these pages are stable since a week. Marking it as fixed. 

Sign in to add a comment