Issue metadata
Sign in to add a comment
|
69kb regression in resource_sizes (MonochromePublic.apk) at 516183:516183 |
||||||||||||||||||||
Issue descriptionCaused by “[parser] Greatly simplify Array spread rewriting code” V8 Commit: 18cac20c50f2fd9d3238f12c89f1e7be56539b1b Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=516183 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 CL description, I'd guess that this increase was probably unexpected or might be avoidable. Please have a look and either: Close as “Won't Fix” with a short justification, or Land a revert / fix-up.
,
Nov 14 2017
Supersize output (via: tools/diagnose_bloat.py --subrepo v8 18cac20c5)
1 symbols added (+), 33 changed (~), 66 removed (-), 836417 unchanged (not shown)
Of changed symbols, 34 grew, 66 shrank
Number of unique symbols 512399 -> 512366 (-33)
0 paths added, 1 removed, 2 changed
Showing 100 symbols (aliases not grouped for diffs) with total pss: 70014 bytes
Histogram of symbols based on PSS:
(-2048,-1024]: 1 (-128,-64]: 11 (-16,-8]: 1 [8,16): 1 [4096,8192): 3
(-512,-256]: 3 (-64,-32]: 11 (-4,-2]: 1 [1024,2048): 21
(-256,-128]: 7 (-32,-16]: 16 (-1,0): 15 [2048,4096): 9
.text=68.6kb .rodata=0 bytes .data.rel.ro=-256 bytes .data=0 bytes .bss=0 bytes total=68.4kb
Number of unique paths: 3
Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
~ 0) 7328 (10.5%) t@0x11fa044 7328 (2764->10092) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseLeftHandSideExpression
~ 1) 13036 (18.6%) t@0x11fe894 5708 (1484->7192) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseMemberExpressionContinuation
~ 2) 17484 (25.0%) t@0x120eb98 4448 (1240->5688) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseConditionalExpression
~ 3) 20568 (29.4%) t@0x1203a64 3084 (2152->5236) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseForAwaitStatement
~ 4) 23620 (33.7%) t@0x11f5860 3052 (2616->5668) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseClassLiteral
~ 5) 26644 (38.1%) t@0x1207dfc 3024 (1628->4652) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseSwitchStatement
~ 6) 29668 (42.4%) t@0x12113ac 3024 (1440->4464) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseUnaryExpression
~ 7) 32620 (46.6%) t@0x1210380 2952 (1188->4140) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseBinaryExpression
~ 8) 35568 (50.8%) t@0x120d0e4 2948 (1004->3952) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseStandardForLoop
~ 9) 38504 (55.0%) t@0x120c0ec 2936 (1152->4088) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseForEachStatementWithoutDeclarations
~ 10) 41400 (59.1%) t@0x120ada0 2896 (1692->4588) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseForEachStatementWithDeclarations
~ 11) 44268 (63.2%) t@0x11fd4b8 2868 (1552->4420) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseArguments
- 12) 42636 (60.9%) t@0x0 -1632 (1632->0) v8/src/parsing/parser.cc
v8::internal::NonPatternRewriter::RewriteExpression
~ 13) 44224 (63.2%) t@0x11f7f8c 1588 (2428->4016) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseObjectPropertyDefinition
~ 14) 45764 (65.4%) t@0x1201484 1540 (844->2384) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseSingleExpressionFunctionBody
~ 15) 47296 (67.6%) t@0x120e258 1532 (836->2368) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseYieldExpression
~ 16) 48820 (69.7%) t@0x11fcc18 1524 (684->2208) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseMemberWithNewPrefixesExpression
~ 17) 50336 (71.9%) t@0x11eb360 1516 (4128->5644) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseAssignmentExpression
~ 18) 51844 (74.1%) t@0x11e7c1c 1508 (1356->2864) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseFormalParameter
~ 19) 53348 (76.2%) t@0x12008e8 1504 (776->2280) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseClassFieldInitializer
~ 20) 54852 (78.4%) t@0x11f6e84 1504 (1344->2848) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseTemplateLiteral
~ 21) 56348 (80.5%) t@0x1209e94 1496 (2012->3508) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseVariableDeclarations
~ 22) 57832 (82.6%) t@0x1204ee0 1484 (1844->3328) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseForStatement
~ 23) 59316 (84.7%) t@0x12068e4 1484 (600->2084) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseThrowStatement
~ 24) 60800 (86.8%) t@0x12075d4 1484 (604->2088) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseWithStatement
~ 25) 62276 (89.0%) t@0x1209028 1476 (1152->2628) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseExpressionOrLabelledStatement
~ 26) 63744 (91.1%) t@0x11ea6a8 1468 (1020->2488) v8/src/parsing/parser.cc
v8::internal::Parser::ParseExportDefault
~ 27) 65212 (93.1%) t@0x1203260 1468 (584->2052) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseWhileStatement
~ 28) 66676 (95.2%) t@0x12021a4 1464 (692->2156) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseIfStatement
~ 29) 68128 (97.3%) t@0x1202a10 1452 (676->2128) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseDoWhileStatement
~ 30) 69580 (99.4%) t@0x11f8f3c 1452 (2204->3656) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParsePropertyName
~ 31) 71016 (101.4%) t@0x121251c 1436 (496->1932) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParsePostfixExpression
~ 32) 72436 (103.5%) t@0x120604c 1420 (780->2200) v8/src/parsing/parser.cc
v8::internal::ParserBase::ParseReturnStatement
,
Nov 14 2017
Based on all of these parser symbols getting bigger, I'd guess there's some over-inlining happening?
,
Nov 14 2017
Thanks for the data, this does look like over-inlining. We were already asking for forced inlining here, but my cleanup seems to have enabled a bunch more than was happening before. This should be a relatively easy matter of removing some always-inline attributes, but I'm not sure the best way to experiment here. Will diagnose_bloat help me bisect, or is there a better tool?
,
Nov 14 2017
diagnose_bloat is what I use. It's just a wrapper building + running supersize, but does at least do incremental builds (leaves not room for manual error). You can also just build libchrome manually with target_os="android" is_official_build=true and readelf -S (or "size") to see the new section sizes.
,
Nov 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/1abeb0caa21301f5ace7177711c4f09f2d6447d9 commit 1abeb0caa21301f5ace7177711c4f09f2d6447d9 Author: Adam Klein <adamk@chromium.org> Date: Tue Nov 14 23:06:25 2017 [parser] Remove inlining of Parser::RewriteSpreads Despite the V8_INLINE annotation, it was never actually inlined until 18cac20c50f2fd9d3238f12c89f1e7be56539b1b removed the NonPatternRewriter, causing all calls to Parser::RewriteNonPattern() to inline RewriteSpreads. This patch should recover the binary bloat in the attached bug while retaining the inlining of the rest of RewriteNonPattern, which in the common case does very little work (and doesn't call out to RewriteSpreads). Bug: chromium:784924 Change-Id: I1c2062b41ceb51a9522d49bdb9353e1840393ca1 Reviewed-on: https://chromium-review.googlesource.com/769442 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#49370} [modify] https://crrev.com/1abeb0caa21301f5ace7177711c4f09f2d6447d9/src/parsing/parser.h
,
Nov 15 2017
Saw a 73kb reduction when this rolled. Thanks! https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=516581 |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Nov 14 2017