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

Issue 627087 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Jul 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

10.5% regression in dromaeo.domcoretraverse at 404546:404561

Project Member Reported by tdres...@chromium.org, Jul 11 2016

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=627087

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgste9swoM


Bot(s) for this bug's original alert(s):

android-galaxy-s5
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Jul 11 2016

Cc: piman@chromium.org
Owner: piman@chromium.org

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

Hi piman@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 : Make dynamic mock bindings initialization consistent with other GL implementations
Author  : piman
Commit description:
  
Various cleanup:
- Every context type except GLContextStub calls InitializeDynamicBindings in
  MakeCurrent. Make it consistent.
- In turn, it means we don't need an explicit InitializeDynamicMockBindings, so
  remove it.
- In some tests, we were creating a context just to be able to call
  InitializeDynamicMockBindings. This is not necessary any more - dynamic
  bindings will be initalized as the first useful context is made current
- Realizing that InitializeDynamicGLBindings is always called with a real
  context, remove the special cases for mock bindings. This makes it consistent
  with other GL implementations
- In turn InitializeDynamicGLBindings always does exactly
  InitializeDynamicGLBindingsGL for valid GLImplementations, and is only called
  with a valid GLImplementation, simplify. It also means it can't fail.

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2129043003
Cr-Commit-Position: refs/heads/master@{#404548}
Commit  : 608429d9968aa471d128c74628d345c10fa8cf08
Date    : Sat Jul 09 00:44:53 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@404545  135.334  2.55047  8  good
chromium@404547  136.095  2.83445  5  good
chromium@404548  129.33   1.67036  5  bad    <--
chromium@404549  127.748  3.20271  5  bad
chromium@404553  131.302  1.31915  5  bad
chromium@404561  129.844  2.92097  8  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 627087

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests dromaeo.domcoretraverse
Test Metric: dom/dom
Relative Change: 4.48%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/783
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007428931607163120


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

| 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!

Comment 3 by piman@chromium.org, Jul 11 2016

Cc: -piman@chromium.org
Owner: littledan@chromium.org
I really don't think this is the right change. My CL virtually doesn't impact production - only at best affects "mock" GL bindings that are used for unit tests.
The v8 roll https://chromium.googlesource.com/v8/v8/+log/252926e7..75f10f91 seems a bit more likely - speculatively assigining to littledan for https://chromium.googlesource.com/v8/v8/+/97e8046e444da3e7e4729aa1ee5a4f86d56f69d0
Status: WontFix (was: Assigned)
The benchmark appears to have recovered. My patch is very unlikely to have affected performance, as it basically inlined one function inside another one and removed part of the code. Marking as WontFix due to the recovery.

Sign in to add a comment