New issue
Advanced search Search tips

Issue 918735 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner:
Closed: Jan 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

36.1 kb regression in resource_sizes (MonochromePublic.apk) at 619147:619147

Project Member Reported by estevenson@chromium.org, Jan 3

Issue description

Caused by "[wasm] Inline Pop methods for performance"

Commit: 65da8bf06d0d3d3a46b1505b550ff9012b5f6f26

Link to size graph: https://chromeperf.appspot.com/report?sid=bb23072657e2d7ca892a1c3fa4643b1ee29b3a0a44d0732adda87168e89c0380&num_points=10&rev=619147
Link to trybot result: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-binary-size/125726

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

The CL above was part of a v8 roll (https://chromium-review.googlesource.com/c/chromium/src/+/1391885) but when I diff'ed the size of the CL (via tools/binary_size/diagnose_bloat.py --subrepo v8 --reference-rev 4b2036ee^ 01257620 --all) the regression was only 28 kb.

I saw in the commit message that a 12 kb regression is expected so I just wanted to call attention to it since it was larger.

Please have a look and either:

1. Close as “Won't Fix” with a short justification, or
2. Land a revert / fix-up.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=918735

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


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.3.254.

Summary of changes available at:
https://chromium.googlesource.com/v8/v8/+log/4b2036ee..01257620

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=luci.chromium.try:linux-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

Change-Id: I1659744613521029b0bb279d13ff7da223b8967e
Reviewed-on: https://chromium-review.googlesource.com/c/1391885
Reviewed-by: v8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Commit-Queue: v8-ci-autoroll-builder <v8-ci-autoroll-builder@chops-service-accounts.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#619147}
Cc: titzer@chromium.org jarin@chromium.org
Components: Blink>JavaScript>WebAssembly
I can confirm the numbers in our v8-local measurements:
https://chromeperf.appspot.com/report?sid=a3e0b14a61c98491d97cd33f2f868c5b5d881808c1cf3c9b630c039c6a695f00&start_rev=58476&end_rev=58496

Size changes are as follows:
- ia32: +12.4% (+0.07%)
- x64: -11.9kB (-0.07%) (sic!)
- arm32: +28.7kB (+0.34%)
- arm64: +37.1kB (+0.30%)

The performance change or epic validation can be seen here:
https://chromeperf.appspot.com/report?sid=3cde349a5b706198b56bb6e395ebd4018f0dd10a549dcf93afdd62179df2cdca&start_rev=58476&end_rev=58496

It's as follows:

- ia32: -7.7%
- x64: -7.9%
- arm32: -5.7%

Hard to say whether this performance improvement justifies the binary size increase. I tend to think it does.
+titzer (wasm TLM) and +jarin (performance sheriff) for their opinion.
I think a 0.3% space improvement for a 7-8% performance gain is acceptable.
By improvement on space, I mean regression, of course :)
Status: WontFix (was: Assigned)
Ack, closing as WontFix according to #3 and #4.

Comment 7 by clemensh@chromium.org, Today (21 hours ago)

Cc: clemensh@chromium.org chiniforooshan@chromium.org
 Issue 918529  has been merged into this issue.

Sign in to add a comment