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

Issue 714893 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

12KB regression in resource_sizes (MonochromePublic.apk) at 466172:466172

Project Member Reported by rsch...@chromium.org, Apr 25 2017

Issue description

Assigning to agrieve@ since this is a roll and bisect is currently not working.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=714893

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


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

Android Builder
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Apr 25 2017


=== BISECT JOB RESULTS ===
Perf regression found but unable to narrow commit range

Build failures prevented the bisect from narrowing the range further.


Bisect Details
  Configuration: android_nexus7_perf_bisect
  Benchmark    : resource_sizes
  Metric       : MonochromePublic.apk_Specifics/normalized apk size
  Change       : 0.02% | 73581091.0 -> 73597475.0

Suspected Commit Range
  5 commits in range
  https://chromium.googlesource.com/chromium/src/+log/49736bfaaee2f7f1c641daf66516ce540eddcbd2..62eefa5666392672359a92bade4d89bc0dc7de34


Revision                           Result               N
chromium@466171                    73581091 +- 0.0      6        good
chromium@466171,v8@d089276e03      ---                  ---      build failure
chromium@466171,v8@f28e487858      ---                  ---      build failure
chromium@466171,v8@fa1de6145f      ---                  ---      build failure
chromium@466171,v8@fe77234eb0      ---                  ---      build failure
chromium@466172                    73597475 +- 0.0      6        bad

To Run This Test
  src/build/android/resource_sizes.py --chromium-output-directory {CHROMIUM_OUTPUT_DIR} --chartjson {CHROMIUM_OUTPUT_DIR}/apks/MonochromePublic.apk

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8981385680889989232

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5838626799222784


| 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 Speed>Bisection.  Thank you!
Cc: jkummerow@chromium.org
Labels: -Restrict-View-Google OS-Android
Owner: kozyatinskiy@chromium.org
Summary: 12KB regression in resource_sizes (MonochromePublic.apk) at 466172:466172 (was: 16KB regression in resource_sizes (MonochromePublic.apk) at 466172:466172)
Biggest jump (of 12kb) is from: 
https://codereview.chromium.org/2828933002
[console] fast console.assert(true)


While 12kb on its own is below our size alert threshold, the commit description makes it sound like maybe this shouldn't have such an impact.

Symbol diff from //tools/binary_size attached below.
Looks like the entire increase is from:
https://cs.chromium.org/chromium/src/v8/src/builtins/builtins.cc?q=CallableFor&sq=package:chromium&l=117

The same happened in recent change  bug 714894 . Not sure if there's anything to be done here.


7 symbols added (+), 21 changed (~), 16 removed (-), 317530 unchanged (not shown)
0 object files added, 0 removed
~    13428 t@0xef26b0   13428   v8/v8_base/builtins.o
               v8::internal::Builtins::CallableFor
+    13520 t@0xef2500   92      v8/v8_base/builtins.o
               v8::internal::BinaryOpWithVectorDescriptor::BinaryOpWithVectorDescriptor
+    13612 t@0xef2448   92      v8/v8_base/builtins.o
               v8::internal::BuiltinDescriptor::BuiltinDescriptor
+    13704 t@0xef24a4   92      v8/v8_base/builtins.o
               v8::internal::CompareDescriptor::CompareDescriptor
-    13612 t@0xef2898   -92     v8/v8_base/builtins.o
               v8::internal::CreateIterResultObjectDescriptor::CreateIterResultObjectDescriptor
-    13520 t@0xef283c   -92     v8/v8_base/builtins.o
               v8::internal::DeletePropertyDescriptor::DeletePropertyDescriptor
-    13428 t@0xef2448   -92     v8/v8_base/builtins.o
               v8::internal::LoadDescriptor::LoadDescriptor
-    13336 t@0xef24a4   -92     v8/v8_base/builtins.o
               v8::internal::LoadFieldDescriptor::LoadFieldDescriptor
-    13244 t@0xef2614   -92     v8/v8_base/builtins.o
               v8::internal::LoadGlobalWithVectorDescriptor::LoadGlobalWithVectorDescriptor
-    13152 t@0xef25b8   -92     v8/v8_base/builtins.o
               v8::internal::LoadICProtoArrayDescriptor::LoadICProtoArrayDescriptor
-    13060 t@0xef255c   -92     v8/v8_base/builtins.o
               v8::internal::LoadWithVectorDescriptor::LoadWithVectorDescriptor
-    12968 t@0xef27e0   -92     v8/v8_base/builtins.o
               v8::internal::PerformNativePromiseThenDescriptor::PerformNativePromiseThenDescriptor
-    12876 t@0xef2784   -92     v8/v8_base/builtins.o
               v8::internal::RejectNativePromiseDescriptor::RejectNativePromiseDescriptor
-    12784 t@0xef2728   -92     v8/v8_base/builtins.o
               v8::internal::ResolveNativePromiseDescriptor::ResolveNativePromiseDescriptor
-    12692 t@0xef2500   -92     v8/v8_base/builtins.o
               v8::internal::StoreWithVectorDescriptor::StoreWithVectorDescriptor
-    12600 t@0xef2670   -92     v8/v8_base/builtins.o
               v8::internal::TypeConversionDescriptor::TypeConversionDescriptor
-    12508 t@0xef26cc   -92     v8/v8_base/builtins.o
               v8::internal::TypeofDescriptor::TypeofDescriptor
+    12600 t@0xef255c   92      v8/v8_base/builtins.o
               v8::internal::WasmRuntimeCallDescriptor::WasmRuntimeCallDescriptor
~    12516 t@0x12ee548  -84     v8/src/inspector/inspector/v8-console.o
               v8_inspector::V8Console::Assert
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a1a3090479cf8da10a2376f58ef2f33cc045eedb

commit a1a3090479cf8da10a2376f58ef2f33cc045eedb
Author: kozyatinskiy <kozyatinskiy@chromium.org>
Date: Wed Apr 26 15:40:42 2017

[builtins] Builtins::CallableFor(): generate CPP case for ConsoleAssert only.

To reduce size of Builtins::CallableFor function we can add only case which we actually use.

BUG= chromium:714893 
R=ishell@chromium.org

Review-Url: https://codereview.chromium.org/2839933003
Cr-Commit-Position: refs/heads/master@{#44900}

[modify] https://crrev.com/a1a3090479cf8da10a2376f58ef2f33cc045eedb/src/builtins/builtins.cc

Status: Fixed (was: Assigned)
Assuming fixed.
Labels: -binary-size Performance-Size

Sign in to add a comment