New issue
Advanced search Search tips

Issue 613167 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

51.1%-52.9% regression in blink_perf.layout at 393873:393961

Project Member Reported by majidvp@chromium.org, May 19 2016

Issue description

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

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


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

chromium-rel-win10
chromium-rel-win7-x64-dual
Project Member

Comment 2 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: jinsuk...@chromium.org
Owner: jinsuk...@chromium.org

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

Hi jinsukkim@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 : Revert "Reland "UTF-8 detector for pages missing encoding info""
Author  : jinsukkim
Commit description:
  
UTF-8 detector introduced in crrev.com/1890103002 is suspected to
cause a couple of bugs due to UTF-8 encoding detection being applied
to wider ranges of web documents. It may have caused overall page
loading time regression by 5% reported via Omaha
PageLoad.Timing2.NavigationToFirstContentfulPaint.
(Update: the regression was caused by something else)

The other bug was confirmed to have been caused by the CL since
the detection changed (albeit correctly) the encoding of the documents
that had not been affected before.

An alternative to address them would be narrow the scope of
the encoding detector but it defeats one of the the goals of the UTF-8
detection expected to set the previously misinterpreted encodings
right for more documents. crrev.com/1960943002

I'm reverting the CL in the hope that the bugs will go away before
branching to M52 scheduled this week. The plan is bring CED to light,
substitute ICU and turn on the automatic encoding detection by
default. CED is much more efficient than ICU, and can potentially
eliminate the need for UTF-8 detection. I'll evaluate the usefulness
of UTF-8 encoding detection afterwards.

This reverts commit 57139d64c5b98142ca9305792f39ae23a4950375.

BUG= 609053 

Review-Url: https://codereview.chromium.org/1979103003
Cr-Commit-Position: refs/heads/master@{#393957}
Commit  : 1673e2f6087df0f802117110dda2bf6b42b56ef4
Date    : Mon May 16 22:44:33 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@393921  198.296  0.539729  5  good
chromium@393947  200.512  0.448916  5  good
chromium@393954  200.831  0.284943  5  good
chromium@393956  199.775  1.13632   5  good
chromium@393957  101.529  0.127726  5  bad    <--
chromium@393960  100.572  0.743006  5  bad
chromium@393973  101.837  0.353706  5  bad

Bisect job ran on: mac_10_11_perf_bisect
Bug ID: 613167

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.layout
Test Metric: hindi-line-layout/hindi-line-layout
Relative Change: 48.64%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_11_perf_bisect/builds/709
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9008139070085978496


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

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

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


===== SUSPECTED CL(s) =====
Subject : Revert "Reland "UTF-8 detector for pages missing encoding info""
Author  : jinsukkim
Commit description:
  
UTF-8 detector introduced in crrev.com/1890103002 is suspected to
cause a couple of bugs due to UTF-8 encoding detection being applied
to wider ranges of web documents. It may have caused overall page
loading time regression by 5% reported via Omaha
PageLoad.Timing2.NavigationToFirstContentfulPaint.
(Update: the regression was caused by something else)

The other bug was confirmed to have been caused by the CL since
the detection changed (albeit correctly) the encoding of the documents
that had not been affected before.

An alternative to address them would be narrow the scope of
the encoding detector but it defeats one of the the goals of the UTF-8
detection expected to set the previously misinterpreted encodings
right for more documents. crrev.com/1960943002

I'm reverting the CL in the hope that the bugs will go away before
branching to M52 scheduled this week. The plan is bring CED to light,
substitute ICU and turn on the automatic encoding detection by
default. CED is much more efficient than ICU, and can potentially
eliminate the need for UTF-8 detection. I'll evaluate the usefulness
of UTF-8 encoding detection afterwards.

This reverts commit 57139d64c5b98142ca9305792f39ae23a4950375.

BUG= 609053 

Review-Url: https://codereview.chromium.org/1979103003
Cr-Commit-Position: refs/heads/master@{#393957}
Commit  : 1673e2f6087df0f802117110dda2bf6b42b56ef4
Date    : Mon May 16 22:44:33 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@393908  310.729  3.35663   5  good
chromium@393933  311.426  1.09855   5  good
chromium@393946  305.686  5.38848   5  good
chromium@393952  305.389  4.67558   5  good
chromium@393955  299.919  7.80211   5  good
chromium@393956  302.156  6.53116   5  good
chromium@393957  135.75   1.70698   5  bad    <--
chromium@393958  139.279  0.720049  5  bad

Bisect job ran on: linux_perf_bisect
Bug ID: 613167

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.layout
Test Metric: hindi-line-layout/hindi-line-layout
Relative Change: 55.18%
Score: 99.9

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


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

| 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!
Cc: e...@chromium.org yukishiino@chromium.org majidvp@chromium.org haraken@chromium.org bashi@chromium.org
Status: WontFix (was: Assigned)
There is a improvement in these metrics that coincide with landing of the original patch [388894:388942] and a regression that coincide with reverting the patch [393873:393961]. So although I don't understand how but the patch improves these metrics (blink_perf.{dom, layout}). Hopefully jinsukkim@  relands it :).

/cc blink_perf.{dom, layout} owners to be aware of potential improvement of this patch.

Sign in to add a comment