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

Issue 594053 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

26.2% regression in blink_perf.dom at 380376:380387

Project Member Reported by kouhei@chromium.org, Mar 11 2016

Issue description

See the link to graphs below.
 

Comment 1 by kouhei@chromium.org, Mar 11 2016

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=594053

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


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

chromium-rel-win10
Timed out waiting for a build in the v8 roll. Launching again.
Cc: ashej...@chromium.org
Labels: Needs-Feedback
Bisect re-kicked with wider revision range.

Details on the below link:
-------------------------
https://chromeperf.appspot.com/group_report?bug_id=594053

Thank you!

Comment 5 Deleted

Comment 6 Deleted

Cc: tkent@chromium.org
Owner: tkent@chromium.org

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

Hi tkent@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 : TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in the previous line.
Author  : tkent
Commit description:
  
Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra trailing
one so that we can ignore it safely on generating TEXTAREA value.
Also, we should make sure editing code doesn't add \n for placeholder newline.

 - HTMLTextAreaElement simply ignore the last <BR> on generating value.

 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.

 - InsertLineBreakCommand always inserts \n for plain text editing field, however
  it produces <BR> as an extra line break for TEXTAREA.

 - ReplaceSelectionCommand appends a <BR> for an InterchangeNewline at the end.

 - InsertTextCommand should not produce TabSpans for plain text editing.

BUG= 522144 , 593184 

Review URL: https://codereview.chromium.org/1785603002

Cr-Commit-Position: refs/heads/master@{#380385}
Commit  : 37f57d892222dc0ed816ae004074d7c80600ad46
Date    : Thu Mar 10 10:54:04 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@380320         0.701506    0.021221    5           good
chromium@380375         0.691197    0.02133     5           good
chromium@380382         0.684803    0.024374    5           good
chromium@380384         0.654152    0.018258    5           good
chromium@380385         0.537369    0.019831    5           bad         <-
chromium@380386         0.54572     0.01324     5           bad
chromium@380389         0.553621    0.014898    5           bad
chromium@380403         0.560446    0.013574    5           bad
chromium@380430         0.544261    0.015154    5           bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 594053

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests blink_perf.dom
Test Metric: textarea-edit/textarea-edit
Relative Change: 22.42%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/1704
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9016332853490204080


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

| 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 8 by tkent@chromium.org, Apr 4 2016

Status: WontFix (was: Assigned)
Basically this is unavoidable, and fixing  issue 109587  will recover this.

I'm the author of blink_perf.dom/textarea-edit, and it's ok to have this performance regression for a while.


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


===== SUSPECTED CL(s) =====
Subject : TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in the previous line.
Author  : tkent
Commit description:
  
Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra trailing
one so that we can ignore it safely on generating TEXTAREA value.
Also, we should make sure editing code doesn't add \n for placeholder newline.

 - HTMLTextAreaElement simply ignore the last <BR> on generating value.

 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.

 - InsertLineBreakCommand always inserts \n for plain text editing field, however
  it produces <BR> as an extra line break for TEXTAREA.

 - ReplaceSelectionCommand appends a <BR> for an InterchangeNewline at the end.

 - InsertTextCommand should not produce TabSpans for plain text editing.

BUG= 522144 , 593184 

Review URL: https://codereview.chromium.org/1785603002

Cr-Commit-Position: refs/heads/master@{#380385}
Commit  : 37f57d892222dc0ed816ae004074d7c80600ad46
Date    : Thu Mar 10 10:54:04 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@380320         0.673256    0.019992    5           good
chromium@380375         0.699156    0.022742    5           good
chromium@380382         0.655968    0.024032    5           good
chromium@380384         0.672923    0.034417    5           good
chromium@380385         0.536574    0.014831    5           bad         <-
chromium@380386         0.540741    0.018042    5           bad
chromium@380389         0.554387    0.012838    5           bad
chromium@380403         0.561227    0.022499    5           bad
chromium@380430         0.543567    0.015298    5           bad

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 594053

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests blink_perf.dom
Test Metric: textarea-edit/textarea-edit
Relative Change: 19.26%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/1705
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9016329272585091600


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

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

Sign in to add a comment