Issue metadata
Sign in to add a comment
|
v8.runtimestats.browsing_desktop failing on 5 builders |
||||||||||||||||||||||
Issue descriptionv8.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.
,
Sep 7 2017
,
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
,
Sep 8 2017
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.
,
Sep 11 2017
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?
,
Sep 11 2017
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.
,
Sep 11 2017
,
Sep 11 2017
,
Sep 11 2017
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.
,
Sep 11 2017
SystemHealthStory doesn't used SharedPageState but this one https://cs.chromium.org/chromium/src/tools/perf/page_sets/system_health/system_health_story.py?rcl=f508048a1ba16a303adf5338491d9e92edffcda9&l=19
,
Sep 11 2017
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?
,
Sep 11 2017
That is what issue 763854 about
,
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
,
Sep 11 2017
With that landed, I would expect CanRunOnBrowser to behave as expected now. If thats not the case let me know.
,
Sep 12 2017
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)
,
Sep 12 2017
,
Sep 13 2017
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.
,
Sep 13 2017
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
,
Sep 14 2017
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.
,
Sep 14 2017
,
Sep 14 2017
#19: I prefer the second as well. We might just be able to reuse WEBGL tag (https://cs.chromium.org/chromium/src/tools/perf/page_sets/system_health/story_tags.py?rcl=ee63a5ba7429ec1acffb4031c75937afff628068&l=45)
,
Sep 19 2017
+1, yeah checking for the presence of the WEBGL tag before running the story sgtm.
,
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
,
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
,
Oct 5 2017
Both these pages are stable since a week. Marking it as fixed. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by charliea@chromium.org
, Sep 7 2017