New issue
Advanced search Search tips

Issue 753819 link

Starred by 1 user
Status: Fixed
Owner:
jkummerow@chromium.org
Closed: Sep 2017
Cc:
estevenson@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

32 kb regression in resource_sizes (MonochromePublic.apk) at 492162:492162

Project Member Reported by estevenson@chromium.org, Aug 9 2017 
Caused by "Cache StoreIC-Transition handlers"

V8 Commit 05e862f78398d8bbeacc7f9e19bbb41c408e709b

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

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

Growth comes from 4 kb of native code, 28 kb due to snapshot size.

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

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

Thanks!
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Aug 9 2017 

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

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


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

Android Builder

Comment 2 by estevenson@chromium.org, Aug 9 2017

Description: Show this description

Comment 3 by estevenson@chromium.org, Aug 9 2017

Labels: OS-Android
Owner: jkummerow@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 18 2017 

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

commit a3ef2489f299aef5a5393985c9351aace0602390
Author: Jakob Kummerow <jkummerow@chromium.org>
Date: Fri Aug 18 16:06:44 2017

Cache fewer StoreIC-Transition handlers

Many handlers are not used again, so we can improve the cache hit rate
by caching fewer handlers. Specifically, in this CL, when a StoreIC
miss causes a new map transition to be created, then the handler is not
cached right away yet (it will be cached next time, when the transition
exists already).

Also, fix an embarrassing bug where growing a TransitionArray dropped
cached handlers. That further improves the cache hit rate. ;-)

Bug:  chromium:752867 ,  chromium:753819 
Change-Id: Id8db5ca1e780a5fe8fc61db7f20996e61c65a90e
Reviewed-on: https://chromium-review.googlesource.com/619851
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47433}
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/ic/ic.cc
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/ic/ic.h
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/lookup.cc
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/lookup.h
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/objects.cc
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/objects/map.h
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/src/transitions.cc
[modify] https://crrev.com/a3ef2489f299aef5a5393985c9351aace0602390/test/cctest/test-field-type-tracking.cc

Comment 5 by agrieve@chromium.org, Sep 28 2017

Status: Fixed (was: Assigned)
Assuming this is fixed. Please reopen if it's not.
►

Sign in to add a comment