New issue
Advanced search Search tips

Issue 894507 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 16
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

28KB regression in resource_sizes (MonochromePublic.apk) at 598299:598299

Project Member Reported by mheikal@chromium.org, Oct 11

Issue description

Caused by “Reland "[parser] Restructure Binary expression parsing"++”

Commit: e7b1908dcabb0c6276e5beefbc04cd50ccfdb62c

Link to size graph: https://chromeperf.appspot.com/report?sid=bb23072657e2d7ca892a1c3fa4643b1ee29b3a0a44d0732adda87168e89c0380&num_points=10&rev=598299

Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase

Based on the graph: 28kb of native code.

It's not clear to me whether or not this increase was expected.
Please have a look and either:

 - Close as “Won't Fix” with a short justification, or
 - Land a revert / fix-up.


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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=e70594d5f0193a03e7ffcd1ce44c28a7f14b3a2429e2ad83c6fb3d131c452b81


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

Android Builder Perf
Assigning to v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com because this is the only CL in range:
Update V8 to version 7.1.310.

Summary of changes available at:
https://chromium.googlesource.com/v8/v8/+log/668b4046..f47ffd75

Please follow these instructions for assigning/CC'ing issues:
https://github.com/v8/v8/wiki/Triaging%20issues

Please close rolling in case of a roll revert:
https://v8-roll.appspot.com/
This only works with a Google account.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:android_optional_gpu_tests_rel

TBR=hablich@chromium.org,v8-waterfall-sheriff@grotations.appspotmail.com

Binary-Size: autoroller
Change-Id: I8e327fb0c988bc44209aff7b43b1b66ef017690c
Reviewed-on: https://chromium-review.googlesource.com/c/1271791
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: v8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#598299}
Looking at your commit it seams you marked some functions as inline and that bloated a bunch of other functions in the v8 parser. Here is the supersize output:

Showing 54 symbols (26 -> 39 unique) with total pss: 28672 bytes
Histogram of symbols based on PSS:
    (-16384,-8192]: 1   (-128,-64]: 1   (-1,0): 12     [8,16): 1     [128,256): 8   [2048,4096): 5
     (-8192,-4096]: 2      (-8,-4]: 1    [1,2): 1     [32,64): 2     [256,512): 4   [4096,8192): 5
     (-4096,-2048]: 2      (-4,-2]: 1    [2,4): 1    [64,128): 1   [1024,2048): 6
Sizes: .text=28.0kb     .rodata=0 bytes    .data.rel.ro=0 bytes    .data=0 bytes    .bss=0 bytes    .dex=0 bytes    .dex.method=0 bytes    .pak.translations=0 bytes    .pak.nontranslated=0 bytes    .other=1 bytes    total=28.0kb
Counts: .text=50 .rodata=0 .data.rel.ro=0 .data=0 .bss=0 .dex=0 .dex.method=0 .pak.translations=0 .pak.nontranslated=0 .other=5
Number of unique paths: 3

Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss, x=.dex, m=.dex.method, p=.pak.translations, P=.pak.nontranslated, o=.other
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
- 0)      -8596 (-30.0%) t@0x0        -8596 (8592->0)    v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseBinaryExpression
- 1)     -16350 (-57.0%) t@0x0        -7754 (7752->0)    v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseBinaryExpression
+ 2)      -9718 (-33.9%) t@0x1002094  6632 (0->6632)     v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseForAwaitStatement
~ 3)      -3274 (-11.4%) t@0xff6368   6444 (3364->9808)  v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseAssignmentExpression
+ 4)       3026 (10.6%) t@0x100a228  6300 (0->6300)     v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseBinaryContinuation
- 5)      -3058 (-10.7%) t@0x0        -6084 (6084->0)    v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParsePrimaryExpression
~ 6)       1290 (4.5%)  t@0x1014ee0  4348 (2920->7268)  v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseAssignmentExpression
+ 7)       5630 (19.6%) t@0x10182ac  4340 (0->4340)     v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseBinaryContinuation
~ 8)       2058 (7.2%)  t@0xfff67c   -3572 (13760->10188) v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseStatement
- 9)      -1398 (-4.9%) t@0x0        -3456 (3456->0)    v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParsePrimaryExpression
~ 10)      1962 (6.8%)  t@0x100eca0  3360 (2492->5852)  v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseAwaitExpression
~ 11)      5226 (18.2%) t@0x100d5bc  3264 (2596->5860)  v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParsePrefixExpression
~ 12)      8442 (29.4%) t@0x100518c  3216 (3092->6308)  v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseMemberWithPresentNewPrefixesExpression
~ 13)     11590 (40.4%) t@0x1006f9c  3148 (3540->6688)  v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseClassLiteral
~ 14)     14210 (49.6%) t@0x100bac4  2620 (4284->6904)  v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseUnaryOpExpression
~ 15)     16244 (56.7%) t@0x101d424  2034 (1952->3986)  v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseMemberWithPresentNewPrefixesExpression
~ 16)     18248 (63.6%) t@0x101a97c  2004 (2196->4200)  v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParsePrefixExpression
~ 17)     20230 (70.6%) t@0x101b9e4  1982 (2088->4070)  v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseAwaitExpression
~ 18)     22046 (76.9%) t@0x101ea2c  1816 (2796->4612)  v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseClassLiteral
~ 19)     23834 (83.1%) t@0x10208c8  1788 (10224->12012) v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseStatement
~ 20)     25486 (88.9%) t@0x10193a0  1652 (3944->5596)  v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ParseUnaryOpExpression
+ 21)     25954 (90.5%) t@0x10089bc  468 (0->468)       v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseV8Intrinsic
+ 22)     26400 (92.1%) t@0xff1870   446 (0->440)       v8/src/parsing/parser.cc
               v8::internal::Parser::ShortcutNumericLiteralBinaryExpression
+ 23)     26680 (93.0%) t@0x1017e44  280 (0->280)       v8/src/parsing/preparser.cc
               v8::internal::ParserBase<>::ValidateArrowFormalParameters
~ 24)     26956 (94.0%) t@Group      276 (104224->104500) {no path}
               ** thunk (count=2)
+ 25)     27200 (94.9%) t@0x1006c0e  244 (0->244)       v8/src/parsing/parser.cc
               v8::internal::ParserBase<>::ParseFunctionExpression

Status: Fixed (was: Assigned)
That regression was detected by our own bots and has already been fixed. I've outlined ParsePrimaryExpression again and also restructured ParseLeftHandSideExpression to inline less.

Sign in to add a comment