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

Issue 714894 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

16KB regression (again) in resource_sizes (MonochromePublic.apk) at 466040:466040

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=714894

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


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

Android Builder
Labels: OS-Android
Owner: jkummerow@chromium.org
15kb of this was caused by: 
https://codereview.chromium.org/2810363003
[builtins] Introduce DeleteProperty builtin

Symbol diff via //tools/binary_size attached below.

Two interesting things:
1. Looks like there is a 92 byte symbol being inlined into many new spots
2. 13.5 kb come from a single symbol: v8::internal::Builtins::CallableFor

Showing 147 symbols with total size: 15720 bytes
.text=15.4kb     .rodata=0 bytes    other=-16 bytes  total=15.4kb
Number of object files: 15

First columns are: running total, type, size
+    13516 t@0xef29ec   13516   v8/v8_base/builtins.o
               v8::internal::Builtins::CallableFor
+    14160 t@0x11f0844  644     v8/v8_base/runtime-object.o
               v8::internal::Stats_Runtime_ShrinkPropertyDictionary
+    14596 t@0x11f474c  436     v8/v8_base/runtime-object.o
               v8::internal::Stats_Runtime_DeleteProperty
+    15000 t@0x11f5c74  404     v8/v8_base/runtime-object.o
               v8::internal::Runtime_ShrinkPropertyDictionary
-    14604 t@0x11f09a4  -396    v8/v8_base/runtime-object.o
               v8::internal::Stats_Runtime_DeleteProperty_Sloppy
-    14208 t@0x11f0b30  -396    v8/v8_base/runtime-object.o
               v8::internal::Stats_Runtime_DeleteProperty_Strict
+    14392 t@0x11f5bbc  184     v8/v8_base/runtime-object.o
               v8::internal::Runtime_DeleteProperty
+    14560 t@0x10071e8  168     v8/v8_base/hydrogen.o
               v8::internal::HOptimizedGraphBuilder::GenerateJSCollectionGetTable
-    14420 t@0x11f2004  -140    v8/v8_base/runtime-object.o
               v8::internal::Runtime_DeleteProperty_Strict
-    14284 t@0x11f1f7c  -136    v8/v8_base/runtime-object.o
               v8::internal::Runtime_DeleteProperty_Sloppy
-    14184 t@0xf6df2c   -100    v8/v8_base/js-operator.o
               v8::internal::compiler::Operator1<v8::internal::LanguageMode, v8::internal::compiler::OpEqualTo<v8::internal::LanguageMode>, v8::internal::compiler::OpHash<v8::internal::LanguageMode> >::PrintParameter const
+    14276 t@0xef2898   92      v8/v8_base/builtins.o
               v8::internal::CreateIterResultObjectDescriptor::CreateIterResultObjectDescriptor
+    14368 t@0xef283c   92      v8/v8_base/builtins.o
               v8::internal::DeletePropertyDescriptor::DeletePropertyDescriptor
+    14460 t@0xef2448   92      v8/v8_base/builtins.o
               v8::internal::LoadDescriptor::LoadDescriptor
+    14552 t@0xef24a4   92      v8/v8_base/builtins.o
               v8::internal::LoadFieldDescriptor::LoadFieldDescriptor
+    14644 t@0xef2614   92      v8/v8_base/builtins.o
               v8::internal::LoadGlobalWithVectorDescriptor::LoadGlobalWithVectorDescriptor
+    14736 t@0xef25b8   92      v8/v8_base/builtins.o
               v8::internal::LoadICProtoArrayDescriptor::LoadICProtoArrayDescriptor
+    14828 t@0xef255c   92      v8/v8_base/builtins.o
               v8::internal::LoadWithVectorDescriptor::LoadWithVectorDescriptor
+    14736 t@0xef25b8   92      v8/v8_base/builtins.o
               v8::internal::LoadICProtoArrayDescriptor::LoadICProtoArrayDescriptor
+    14828 t@0xef255c   92      v8/v8_base/builtins.o
               v8::internal::LoadWithVectorDescriptor::LoadWithVectorDescriptor
+    14920 t@0xef27e0   92      v8/v8_base/builtins.o
               v8::internal::PerformNativePromiseThenDescriptor::PerformNativePromiseThenDescriptor
+    15012 t@0xef2784   92      v8/v8_base/builtins.o
               v8::internal::RejectNativePromiseDescriptor::RejectNativePromiseDescriptor
+    15104 t@0xef2728   92      v8/v8_base/builtins.o
               v8::internal::ResolveNativePromiseDescriptor::ResolveNativePromiseDescriptor
+    15196 t@0xef2500   92      v8/v8_base/builtins.o
               v8::internal::StoreWithVectorDescriptor::StoreWithVectorDescriptor
+    15288 t@0xef2670   92      v8/v8_base/builtins.o
               v8::internal::TypeConversionDescriptor::TypeConversionDescriptor
+    15380 t@0xef26cc   92      v8/v8_base/builtins.o
               v8::internal::TypeofDescriptor::TypeofDescriptor
~    15460 t@0x10198b0  80      v8/v8_base/hydrogen.o
               v8::internal::HOptimizedGraphBuilder::GenerateArrayBufferViewGetByteOffset
-    15388 t@0xf6d07c   -72     v8/v8_base/js-operator.o
               v8::internal::compiler::JSOperatorBuilder::DeleteProperty
+    15448 t@0xf70a64   60      v8/v8_base/js-operator.o
               v8::internal::compiler::JSOperatorBuilder::DeleteProperty
~    15496 t@0xf64fa0   48      v8/v8_base/js-generic-lowering.o
               v8::internal::compiler::JSGenericLowering::LowerJSDeleteProperty
Project Member

Comment 4 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% | 73559213.0 -> 73575597.0

Suspected Commit Range
  11 commits in range
  https://chromium.googlesource.com/chromium/src/+log/aab6cc307527e5c86c674d78ae8e857f2c6d0e37..01b2bb5f5d1c602f15fda07051ea2fbe49bd2845


Revision                           Result               N
chromium@466039                    73559213 +- 0.0      6        good
chromium@466039,v8@64bb6e6c90      ---                  ---      build failure
---                                ---                  ---      too many build failures to list
chromium@466039,v8@131c117674      ---                  ---      build failure
chromium@466040                    73575597 +- 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/8981385648056239200

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


| 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!
Labels: -Restrict-View-Google
ping
Project Member

Comment 7 by bugdroid1@chromium.org, May 3 2017

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

commit 133ef93afa74f1d08919c89b358b8e0da5323cdb
Author: jkummerow <jkummerow@chromium.org>
Date: Wed May 03 15:02:14 2017

Reduce binary size of Builtins::CallableFor

by pulling parameterizable things out of the case-blocks.
No change in functionality.

BUG= chromium:714894 

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

[modify] https://crrev.com/133ef93afa74f1d08919c89b358b8e0da5323cdb/src/builtins/builtins.cc
[modify] https://crrev.com/133ef93afa74f1d08919c89b358b8e0da5323cdb/src/interface-descriptors.h

Status: Fixed (was: Assigned)
Should be fixed by #7; in fact Builtins::CallableFor should be smaller than before the size increase now.
Labels: -binary-size Performance-Size

Sign in to add a comment