New issue
Advanced search Search tips

Issue 631411 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 631396
Owner:
Closed: Jul 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

15.9% regression in blink_perf.parser at 407368:407372

Project Member Reported by primiano@chromium.org, Jul 26 2016

Issue description

Hmm this is weird, none of the CL in range seem related
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=631411

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


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

android-one
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Jul 26 2016

Mergedinto: 631396
Status: Duplicate (was: Assigned)

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


===== SUSPECTED CL(s) =====
Subject : Reland of Shrink gn's chrome.dll - now smaller than gyp's (patchset #1 id:1 of https://codereview.chromium.org/2173453004/ )
Author  : brucedawson
Commit description:
  
Reason for revert (reland):
Problem is understood, mostly. Needed to slightly update the patch to fix it. Blink split counts were copied from gyp.

Original issue's description (with indents removed):
More work to shrink gn's chrome.dll

The three largest globals that were present in gn's chrome.dll but not in gyp's chrome.dll were eliminated by using /verbose linker output to track the object files that pulled them in and then conditionally changing source_set targets to static_library targets. Specifically:

unigram_table, in compact_enc_det.obj
- Referenced by TextResourceDecoder.obj from //third_party/WebKit/Source/core:html - some other source_set targets in this file were also modified

gpu::ApplyFramebufferAttachmentCMAAINTELResourceManager::cmaa_frag_s1_ and cmaa_frag_s2_, in gles2_cmd_apply_framebuffer_attachment_cmaa_intel.obj from //gpu/command_buffer/service:service_sources
- Referenced by gpu_command_buffer_stub.obj from //gpu/ipc/service:ipc_service_sources
- Referenced by gpu_video_decode_accelerator.obj from //media/gpu/ipc/service:service
- Referenced by gpu_child_thread.obj from //content/gpu:gpu_sources
- Referenced by gpu_video_decode_accelerator_factory.obj from //content/public/gpu:gpu_sources

As of R406709 this shrinks gn's 32-bit official chrome.dll file size from 38,907,904 bytes to 37,571,584 bytes - an unexpected 1,336,320 byte savings, mostly from the .text section. There is also ~67,000 bytes of memory-only savings in the zero-init part of the .data section.

At the same revision gyp's 32-bit official chrome.dll file size is 37,843,456 bytes - 271,872 bytes *larger* than the gn version.

There are still globals that are present in gn's chrome.dll but not gyp's chrome.dll, so the optimization technique can still be applied some more, but the priority is much lower now that gn is winning.

This is a follow-on to crrev.com/2163823002.

TBR=brettw@chromium.org,achuith@chromium.org
BUG= 624274 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2172033002
Cr-Commit-Position: refs/heads/master@{#407372}
Commit  : 6e53a2e16dc26dec41494b02da83a1221b72e7c4
Date    : Sat Jul 23 19:37:25 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@407367  62.7889  0.379238  5  good
chromium@407370  62.9284  0.330804  5  good
chromium@407371  63.0311  1.33997   5  good
chromium@407372  51.8141  0.5341    5  bad    <--

Bisect job ran on: android_one_perf_bisect
Bug ID: 631411

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.parser
Test Metric: css-parser-yui/css-parser-yui
Relative Change: 17.48%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1434
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9006082790386919216


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

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

Comment 5 by 42576172...@developer.gserviceaccount.com, Jul 26 2016


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


===== SUSPECTED CL(s) =====
Subject : Reland of Shrink gn's chrome.dll - now smaller than gyp's (patchset #1 id:1 of https://codereview.chromium.org/2173453004/ )
Author  : brucedawson
Commit description:
  
Reason for revert (reland):
Problem is understood, mostly. Needed to slightly update the patch to fix it. Blink split counts were copied from gyp.

Original issue's description (with indents removed):
More work to shrink gn's chrome.dll

The three largest globals that were present in gn's chrome.dll but not in gyp's chrome.dll were eliminated by using /verbose linker output to track the object files that pulled them in and then conditionally changing source_set targets to static_library targets. Specifically:

unigram_table, in compact_enc_det.obj
- Referenced by TextResourceDecoder.obj from //third_party/WebKit/Source/core:html - some other source_set targets in this file were also modified

gpu::ApplyFramebufferAttachmentCMAAINTELResourceManager::cmaa_frag_s1_ and cmaa_frag_s2_, in gles2_cmd_apply_framebuffer_attachment_cmaa_intel.obj from //gpu/command_buffer/service:service_sources
- Referenced by gpu_command_buffer_stub.obj from //gpu/ipc/service:ipc_service_sources
- Referenced by gpu_video_decode_accelerator.obj from //media/gpu/ipc/service:service
- Referenced by gpu_child_thread.obj from //content/gpu:gpu_sources
- Referenced by gpu_video_decode_accelerator_factory.obj from //content/public/gpu:gpu_sources

As of R406709 this shrinks gn's 32-bit official chrome.dll file size from 38,907,904 bytes to 37,571,584 bytes - an unexpected 1,336,320 byte savings, mostly from the .text section. There is also ~67,000 bytes of memory-only savings in the zero-init part of the .data section.

At the same revision gyp's 32-bit official chrome.dll file size is 37,843,456 bytes - 271,872 bytes *larger* than the gn version.

There are still globals that are present in gn's chrome.dll but not gyp's chrome.dll, so the optimization technique can still be applied some more, but the priority is much lower now that gn is winning.

This is a follow-on to crrev.com/2163823002.

TBR=brettw@chromium.org,achuith@chromium.org
BUG= 624274 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2172033002
Cr-Commit-Position: refs/heads/master@{#407372}
Commit  : 6e53a2e16dc26dec41494b02da83a1221b72e7c4
Date    : Sat Jul 23 19:37:25 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@407367  62.4422  1.58751   5  good
chromium@407370  63.0567  0.769759  5  good
chromium@407371  62.6265  1.33564   5  good
chromium@407372  50.765   0.816     5  bad    <--

Bisect job ran on: android_one_perf_bisect
Bug ID: 631411

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.parser
Test Metric: css-parser-yui/css-parser-yui
Relative Change: 18.70%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1435
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9006082779979282496


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

| 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