New issue
Advanced search Search tips

Issue 865076 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

28KB regression in resource_sizes (MonochromePublic.apk) at 575968:575968

Project Member Reported by mheikal@google.com, Jul 18

Issue description

Caused by “Create a skeleton App Service and App Registry.”

Commit: 2d1e7d102ae50dbe5f2e36e76c3c1c519478891c

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

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: 25kb of native code.


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

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

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


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

Android Builder Perf
Assigning to dominickn@chromium.org because this is the only CL in range:
Create a skeleton App Service and App Registry.

This CL creates the App Service as a in-process per-profile service, and
attaches the App Registry to it.

App Service is currently located in chrome/browser as many of its direct
dependencies (which are not yet connected to the service) live there.
Over time, we will refactor the dependencies such that:

1. data which can be made independent of chrome/browser is moved within
   the App Service
2. components which rely on that data access it asynchronously via the
   App Service

This will allow us to move the App Service to chrome/services in the
future.

BUG=826982

Change-Id: Ia7608fbb8c8b235d2b3e69ffa447963b32f2d5f0
Reviewed-on: https://chromium-review.googlesource.com/1127513
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575968}
Supersize output of which symbols increased in size:

Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss, x=.dex, m=.dex.method, p=.pak.translations, P=.pak.nontranslated, o=.other
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
~ 0)       1356 (4.6%)  o@0x0        1356 (2024314->2025670) base/trace_event/cfi_backtrace_android.cc
               assets/unwind_cfi_32
+ 1)       2186 (7.4%)  t@0x83d3a0   830 (0->828)       services/preferences/public/cpp/persistent_pref_store_client.cc
               prefs::PersistentPrefStoreClient::FlushPendingWrites
+ 2)       2996 (10.2%) t@0x83ecdc   810 (0->808)       services/preferences/public/cpp/pref_service_factory.cc
               prefs::OnConnect
+ 3)       3618 (12.3%) t@0x83fa20   622 (0->620)       services/preferences/public/cpp/pref_store_client_mixin.cc
               prefs::PrefStoreClientMixin<>::OnPrefChanged
+ 4)       4240 (14.4%) t@0x83fee8   622 (0->620)       services/preferences/public/cpp/pref_store_client_mixin.cc
               prefs::PrefStoreClientMixin<>::OnPrefChanged
+ 5)       4642 (15.8%) t@0x83cf64   402 (0->400)       services/preferences/public/cpp/persistent_pref_store_client.cc
               prefs::PersistentPrefStoreClient::PersistentPrefStoreClient
+ 6)       4974 (16.9%) t@0x83eb3c   332 (0->332)       services/preferences/public/cpp/pref_service_factory.cc
               prefs::ConnectToPrefService
+ 7)       5294 (18.0%) t@0x84245c   320 (0->320)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom.cc
               mojo::internal::Serializer<>::Serialize
~ 8)       5580 (19.0%) P@0x2cab     286 (4149->4435)   chrome/browser/chrome_content_browser_client.cc
               ../../chrome/browser/browser_resources.grd: IDR_CHROME_CONTENT_BROWSER_MANIFEST_OVERLAY
+ 9)       5864 (19.9%) t@0x840ae0   284 (0->284)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom.cc
               prefs::mojom::PrefStoreConnector_Connect_ForwardToCallback::Accept
+ 10)      6142 (20.9%) t@0x8407f4   278 (0->276)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom.cc
               prefs::mojom::PrefStoreObserverStubDispatch::Accept
+ 11)      6390 (21.7%) t@0x8410e8   248 (0->248)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom.cc
               prefs::mojom::PersistentPrefStoreProxy::RequestValue
+ 12)      6636 (22.6%) t@0x843e0c   246 (0->244)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom-shared.cc
               prefs::mojom::internal::PrefStoreConnector_Connect_ResponseParams_Data::Validate
+ 13)      6880 (23.4%) t@0x112e8b4  244 (0->244)       chrome/browser/apps/foundation/app_service/app_registry/app_registry.cc
               apps::AppRegistry::GetApps
+ 14)      7122 (24.2%) t@0x83de46   242 (0->242)       services/preferences/public/cpp/persistent_pref_store_client.cc
               std::__ndk1::__tree<>::__find_equal<>
+ 15)      7354 (25.0%) t@0x83fcf0   232 (0->232)       services/preferences/public/cpp/pref_store_client_mixin.cc
               std::__ndk1::vector<>::insert<>
+ 16)      7576 (25.8%) t@0x83d254   222 (0->220)       services/preferences/public/cpp/persistent_pref_store_client.cc
               prefs::PersistentPrefStoreClient::QueueWrite
+ 17)      7790 (26.5%) t@0x83e8b0   214 (0->212)       services/preferences/public/cpp/pref_registry_serializer.cc
               prefs::SerializePrefRegistry
+ 18)      7998 (27.2%) t@0x840a10   208 (0->208)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom.cc
               prefs::mojom::PrefStoreConnectorProxy::Connect
+ 19)      8198 (27.9%) t@0x112f084  200 (0->200)       $root_gen_dir/chrome/browser/apps/foundation/app_service/public/mojom/app_registry.mojom.cc
               apps::mojom::AppRegistry_GetApps_ProxyToResponder::Run
+ 20)      8392 (28.5%) t@0x84092c   194 (0->192)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom.cc
               prefs::mojom::PrefStoreObserverRequestValidator::Accept
+ 21)      8576 (29.2%) t@0x8446d8   184 (0->184)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom-shared.cc
               mojo::internal::Array_Data<>::Validate
+ 22)      8754 (29.8%) t@0x112ee50  178 (0->176)       chrome/browser/apps/foundation/app_service/app_service.cc
               apps::AppService::OnStart
+ 23)      8930 (30.4%) t@0x83f77c   176 (0->176)       services/preferences/public/cpp/pref_store_client.cc
               prefs::PrefStoreClient::PrefStoreClient
+ 24)      9102 (31.0%) t@0x8444a2   172 (0->172)       $root_gen_dir/services/preferences/public/mojom/preferences.mojom-shared.cc
               mojo::internal::Map_Data<>::Validate
+ 25)      9270 (31.5%) t@0x112f2a8  168 (0->168)       $root_gen_dir/chrome/browser/apps/foundation/app_service/public/mojom/app_registry.mojom.cc
               apps::mojom::AppRegistryRequestValidator::Accept

Components: Platform>Apps>Foundation
Labels: OS-Android
Sigh. Mojo bloat strikes again.

For now I'll disable the App Service on Android (we don't have any imminent plans to use it there), and we can follow up later if we need to enable it.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 19

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

commit b7f01aa35ef682fb4ad9ac4350ae0484913a9631
Author: Dominick Ng <dominickn@chromium.org>
Date: Thu Jul 19 23:37:41 2018

Do not include the App Service on Android.

Due to Mojo bloat, the App Service causes a 28KiB size regression on
Android. Since we don't have imminent plans to use the App Service
there, stop building the service into the Android binary. We can revisit
including the App Service if there is a future need for it.

BUG= 865076 
TBR=rockot@chromium.org

Change-Id: Ifeaad395de9d8e4cee0ef95ac53cd67ea6439d53
Reviewed-on: https://chromium-review.googlesource.com/1142708
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576691}
[modify] https://crrev.com/b7f01aa35ef682fb4ad9ac4350ae0484913a9631/chrome/app/BUILD.gn
[modify] https://crrev.com/b7f01aa35ef682fb4ad9ac4350ae0484913a9631/chrome/browser/BUILD.gn
[modify] https://crrev.com/b7f01aa35ef682fb4ad9ac4350ae0484913a9631/chrome/browser/profiles/profile_impl.cc
[modify] https://crrev.com/b7f01aa35ef682fb4ad9ac4350ae0484913a9631/chrome/test/BUILD.gn

Status: Fixed (was: Assigned)
This should be fixed now.
 Issue 866167  has been merged into this issue.

Sign in to add a comment