New issue
Advanced search Search tips

Issue 896685 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

15.8 kb regression in resource_sizes (MonochromePublic.apk) at 600637:600637

Project Member Reported by estevenson@chromium.org, Oct 18

Issue description

Caused by “sqlite: Upgrade from 3.24.0 to 3.25.2”

Commit: 56d26cfc1461f9cf84c0b861c181c5dec06a0bf0

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

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: native code growth.

It seems likely that this size growth is unavoidable here but I thought I'd file a bug to see if there's any room for optimization.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=896685

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


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

Android Builder Perf
Assigning to pwnall@chromium.org because this is the only CL in range:
sqlite: Upgrade from 3.24.0 to 3.25.2

The new release changes how ALTER TABLE RENAME is implemented, and
requires an update to WebSQL's SQLite authorizer. This CL includes the
update, and extends the LayoutTest for the authorizer to cover the
newly implemented column renaming functionality.

Bug:  892852 
Change-Id: I58fdc2927cacb6ccc84a741d7fc519fb2e5b8721
Reviewed-on: https://chromium-review.googlesource.com/c/1266881
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600637}
I put together https://crrev.com/c/1290510 -- that should give us the space back.

Note: This is a one-time thing, please don't expect future magic optimizations :) 

Details: At this point, the SQLite build is fairly tight. We'll be able to slim it down after we remove WebSQL, which is expected to take ~years.
Great, thank you!
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 19

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

commit c6b2d9d15de0afdb3a2f46e14008eab7e708db5a
Author: Victor Costan <pwnall@chromium.org>
Date: Fri Oct 19 23:40:20 2018

sqlite: Disable two unused features.

This CL disables window functions, which were introduced in SQLite 3.25
[1], and PostgreSQL-style UPSERT, which was introduced in SQLite 3.24
[2]. This reduces the binary size by 57,376 bytes on a Linux official
build.

The sqlite3.c diff was entirely generated by re-running the updated
generated_amalgamation.sh script.

[1] https://www.sqlite.org/windowfunctions.html#history
[2] https://www.sqlite.org/lang_UPSERT.html

Bug:  896685 ,  892852 
Change-Id: Ia501d35993c382445e6a70348dd1643037969245
Reviewed-on: https://chromium-review.googlesource.com/c/1290510
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601355}
[modify] https://crrev.com/c6b2d9d15de0afdb3a2f46e14008eab7e708db5a/third_party/sqlite/BUILD.gn
[modify] https://crrev.com/c6b2d9d15de0afdb3a2f46e14008eab7e708db5a/third_party/sqlite/amalgamation/sqlite3.c
[modify] https://crrev.com/c6b2d9d15de0afdb3a2f46e14008eab7e708db5a/third_party/sqlite/scripts/generate_amalgamation.sh

Status: Fixed (was: Assigned)
resource_sizes went down by 16 kb after the CL above -- https://chromeperf.appspot.com/report?sid=bb23072657e2d7ca892a1c3fa4643b1ee29b3a0a44d0732adda87168e89c0380&num_points=10&rev=601355

Sign in to add a comment