New issue
Advanced search Search tips

Issue 922861 link

Starred by 1 user
Status: Assigned
Owner:
erikc...@chromium.org
Cc:
jegray@google.com
thomasanderson@chromium.org
sullivan@chromium.org
dsinclair@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

0%-0.1% regression in sizes at 623460:623461

Project Member Reported by sullivan@chromium.org, Jan 17 (6 days ago) 
This is a 2MiB increase in binary size. There are only 2 CLs in the range:

dsinclair: looks like you're the sheriff for this one, any way it could have caused such a size increase?

Roll src/third_party/SPIRV-Tools/src 3109ca16b0f9..49b5b0abc655 (1 commits)

https://chromium.googlesource.com/external/github.com/KhronosGroup/SPIRV-Tools.git/+log/3109ca16b0f9..49b5b0abc655


git log 3109ca16b0f9..49b5b0abc655 --date=short --no-merges --format='%ad %ae %s'
2019-01-16 stevenperron@google.com Fix up bit shifts by 32. (#2292)

jegray, the other one is:
Integrate HintCacheStore into Previews

Is the HintCacheStore in the binary?
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jan 17 (6 days ago) 

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

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


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

Google Chrome Linux x64
Google Chrome Win

Comment 2 by jegray@google.com, Jan 17 (5 days ago) 

HintCacheStore is in the binary. The CL bring in a new leveldb_proto::ProtoDatabase type, which appears to be where the bulk of the increase is coming from.

If I build Chrome on Linux with a non-debug build with no debug symbols, then the size of the chrome binary goes from 220,857,328 bytes without the CL to 220,925,920 bytes with the CL (68,592 bytes increase, which amounts to 0.03%).

On Android, this CL caused the APK size to increase from 72,770,491 to 72,778,938 bytes (8,447 bytes, which amounts to 0.01%).
https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-binary-size/136652

This amount seems pretty reasonable to me. The change itself uses the LevelDB database to move storage of DataSaver user resource loading hints from memory to disk, which should reduce their memory usage by close to 1MB (a couple orders of magnitude more than the 8KB binary size increase on Android).

Comment 3 by sullivan@chromium.org, Jan 18 (5 days ago)

Owner: thomasanderson@chromium.org
thomasanderson: since you work on Chrome Linux, do you think you could weigh in on whether this is a reasonable size regression? If it is, feel free to mark WontFix.

Comment 4 by jegray@google.com, Jan 18 (5 days ago) 

For context, here are the two HintCacheStore CLs:

Implement HintCacheStore and HintCacheLevelDBStore
https://chromium-review.googlesource.com/c/chromium/src/+/1403335

Integrate HintCacheStore into Previews
https://chromium-review.googlesource.com/c/chromium/src/+/1402283

Comment 5 by thomasanderson@chromium.org, Jan 18 (5 days ago)

Cc: thomasanderson@chromium.org
Owner: erikc...@chromium.org
Status: Assigned (was: Unconfirmed)
over to erikchen@ for memory/binarysize tradeoff
►

Sign in to add a comment