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

Issue 773077 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

v8.runtimestats.browsing_mobile browser:chrome:newtab failing on webview

Project Member Reported by oysteine@chromium.org, Oct 9 2017

Issue description

v8.runtimestats.browsing_mobile failing on 2 builders

Builders failed on: 
- Android Nexus5X WebView Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Android%20Nexus5X%20WebView%20Perf
- Android Nexus6 WebView Perf: 
  https://build.chromium.org/p/chromium.perf/builders/Android%20Nexus6%20WebView%20Perf



 
Failing as far back as bot history goes; disabling.
Project Member

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

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

commit 4d071c63f0d7bc6c3277dca6fee3a165e77a4795
Author: Oystein Eftevaag <oysteine@chromium.org>
Date: Tue Oct 10 17:54:07 2017

Disable failing newtab story of v8.runtimestats.browsing_mobile benchmark on webview

TBR=nednguyen@chromium.org

Bug:  773077 
Change-Id: Idb235cd1d09e9dd35f15e74d07094bab29528e3d
Reviewed-on: https://chromium-review.googlesource.com/707778
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507710}
[modify] https://crrev.com/4d071c63f0d7bc6c3277dca6fee3a165e77a4795/tools/perf/benchmarks/v8_browsing.py

Cc: -mythria@chromium.org
Labels: -Pri-1 Pri-2
Owner: mythria@chromium.org
Cc: mythria@chromium.org ashleymarie@chromium.org
 Issue 773405  has been merged into this issue.
Cc: nedngu...@google.com rnep...@chromium.org
I think newtab and omnibox should not run on webview perf bots. These are disabled here: https://cs.chromium.org/chromium/src/tools/perf/page_sets/system_health/expectations.py?q=system_health/exp&sq=package:chromium&l=146

I guess this is not working because we have a GetExpectations function in v8.runtimestats.browsing_mobile benchmark. 
https://cs.chromium.org/chromium/src/tools/perf/benchmarks/v8_browsing.py?q=v8_browsi&sq=package:chromium&l=195

In my understanding we should not have GetExpectations inside the benchmark and it should be controlled from system_health/expectations.py instead.

@ned, @rnephew, any suggestions on if we can have GetExpectations in v8.runtimestats.browsing_mobile benchmark?

Yeah, the system_health page sets/benchmarks are the odd man out for this because of how they are structured. There is no reason they couldn't have their own expectations kept separate and be removed from the system_health expectations file. The v8 benchmarks using the system health page set already have their own classes, so it doesn't really matter where it is located. Do we want it to match the weirdness of system health since its using its page set, or do we want it to match the other benchmarks.
Oh ok. I am fine with either approaches. It will be nice to have a consistent behaviour for mobile and desktop versions. So its better we either let the desktop variants to have their own class or move the mobile to use system_health expectations file. I will wait for feedback from ned. 
Making a separate GetExpectations for  v8.runtimestats.browsing_mobile benchmark SGTM
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 13 2017

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

commit 2aff6db2e54003949511454f195acf42c3ae357d
Author: Mythri Alle <mythria@chromium.org>
Date: Fri Oct 13 13:42:58 2017

Add GetExpectations class to v8.browsing* benchmarks

v8.browsing_desktop, v8.browsing_mobile, v8.runtimestats.browsing_desktop
were using the expectations from system_health/expectations.py file. This
cl moves them to their own class.

Bug:  chromium:773077 
Change-Id: I049361516407455016ee11558d1d2eb9d8e57abc
Reviewed-on: https://chromium-review.googlesource.com/716678
Reviewed-by: rnephew <rnephew@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508690}
[modify] https://crrev.com/2aff6db2e54003949511454f195acf42c3ae357d/tools/perf/benchmarks/v8_browsing.py
[modify] https://crrev.com/2aff6db2e54003949511454f195acf42c3ae357d/tools/perf/page_sets/system_health/expectations.py

Status: Fixed (was: Available)

Sign in to add a comment