New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 721922 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

[Cronet] Cronet Dynamic framework has considerable startup latency impact on iOS devices

Project Member Reported by mef@chromium.org, May 12 2017

Issue description

Packaging Cronet as embedded dynamic framework provides a lot of benefits.
Unfortunately it comes with considerable impact on cold app startup latency.

Use of Cronet as static library resolves the startup latency issue.

We can try to use 'ld -r' to hide private symbols inside of the static library and avoid conflicts with other dependencies used by application.
 

Comment 1 by mef@chromium.org, May 12 2017

Labels: -Pri-3 M-60 Pri-1
I've updated prototype https://codereview.chromium.org/2807283002/ 

There are 2 open issues remain.

1. Debug build for 32 bit x86 simulator fails with cryptic error during link phase, presumably due to size of .o file exceeding some size limit. This may be acceptable as other configuration, including x64 simulator build fine.

2. The hide_symbols script doesn't play well with fat binaries built by specifying additional_cpus. This is worked around in package_ios.py script, but needs to be fixed for real to avoid breakage of official build bots.

This issue is pretty urgent as it blocks multiple consumers from public deployment.

Hiroshi, would you be able to help with item 2?

Comment 2 Deleted

Comment 3 by ichikawa@google.com, May 15 2017

> Hiroshi, would you be able to help with item 2?

Sure, but what would be a problem by workarounding it in package_ios.py? Why it breaks official build bots?

Comment 4 by mef@chromium.org, May 15 2017

Official build bots build one configuration per bot, so currently we have 4 - Debug ARM, Debug Intel, Release ARM, Release Intel. 

Each bot uses additional_cpus = [] gn setting to build both 32 and 64 bit together into a fat binary.

From previous conversations with smut@ it doesn't look like we could use package_ios script directly on the bot (we've been using it for local builds on dev machines).

Ideally we should be able to craft combined hide_symbols.py and corresponding gn rule, which will work with additional_cpus setting. I think that requires clear separation of 32 and 64 bit .o and .a files during build, and applying lipo to final static library with hidden symbols. 

Does this make sense?

Comment 5 by ichikawa@google.com, May 16 2017

Makes sense, thanks!

But I may not be the best person to do the work because I'm not very familiar with GN. I will try to write the GN rule if you want, but it's likely that I need some help from a GN expert. Do you know of any GN expert who would help this?

Comment 6 by mef@chromium.org, May 16 2017

Cc: jbudorick@chromium.org sdefresne@chromium.org
I see, I was hoping that you were the GN expert. :)

Adding jbudorick@ and sdefresne@ who have helped Cronet with GN in the past and maybe interested in the new challenge.

Comment 7 by mef@chromium.org, May 16 2017

This issue is tracked internally as b/36763054.

Comment 8 by mef@chromium.org, May 22 2017

Owner: mef@chromium.org
Status: Started (was: Available)
The CL https://codereview.chromium.org/2807283002/  has second issue resolved now.

The x86 support is deemed not critical and could also be remedied by packaging static library without hiding its dependencies, which should be ok for testing.
Project Member

Comment 9 by bugdroid1@chromium.org, May 26 2017

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

commit 3412c3c60b69798430b3a3cc9d2834f5d44e6042
Author: mef <mef@chromium.org>
Date: Fri May 26 22:15:30 2017

[Cronet] Build static libcronet.a for iOS with complete dependencies.

- Hide dependencies inside of the library to avoid symbol conflicts.
- Add cronet_consumer_static target that builds consumer app with static library.

BUG= 721922 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2807283002
Cr-Commit-Position: refs/heads/master@{#475158}

[modify] https://crrev.com/3412c3c60b69798430b3a3cc9d2834f5d44e6042/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/3412c3c60b69798430b3a3cc9d2834f5d44e6042/components/cronet/ios/cronet_consumer/BUILD.gn
[add] https://crrev.com/3412c3c60b69798430b3a3cc9d2834f5d44e6042/components/cronet/tools/dummy.py
[add] https://crrev.com/3412c3c60b69798430b3a3cc9d2834f5d44e6042/components/cronet/tools/hide_symbols.py
[modify] https://crrev.com/3412c3c60b69798430b3a3cc9d2834f5d44e6042/components/cronet/tools/package_ios.py

Project Member

Comment 10 by bugdroid1@chromium.org, May 27 2017

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

commit 14d16a8ffa51069ce25796ed7fa64df795b07d91
Author: mef <mef@chromium.org>
Date: Sat May 27 14:55:11 2017

Revert of [Cronet] Build static libcronet.a for iOS with complete dependencies. (patchset #20 id:380001 of https://codereview.chromium.org/2807283002/ )

Reason for revert:
Broke official Cronet builders.

Original issue's description:
> [Cronet] Build static libcronet.a for iOS with complete dependencies.
>
> - Hide dependencies inside of the library to avoid symbol conflicts.
> - Add cronet_consumer_static target that builds consumer app with static library.
>
> BUG= 721922 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
>
> Review-Url: https://codereview.chromium.org/2807283002
> Cr-Commit-Position: refs/heads/master@{#475158}
> Committed: https://chromium.googlesource.com/chromium/src/+/3412c3c60b69798430b3a3cc9d2834f5d44e6042

TBR=ichikawa@chromium.org,kapishnikov@chromium.org,lilyhoughton@chromium.org,rsesek@chromium.org,jzw@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 721922 

Review-Url: https://codereview.chromium.org/2907023002
Cr-Commit-Position: refs/heads/master@{#475247}

[modify] https://crrev.com/14d16a8ffa51069ce25796ed7fa64df795b07d91/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/14d16a8ffa51069ce25796ed7fa64df795b07d91/components/cronet/ios/cronet_consumer/BUILD.gn
[delete] https://crrev.com/d932d66b1f1d0d873d935a6c9e2df0807843c908/components/cronet/tools/dummy.py
[delete] https://crrev.com/d932d66b1f1d0d873d935a6c9e2df0807843c908/components/cronet/tools/hide_symbols.py
[modify] https://crrev.com/14d16a8ffa51069ce25796ed7fa64df795b07d91/components/cronet/tools/package_ios.py

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 5 2017

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

commit f035c8c0053e7bcec27f968bf7fbbfc062369e0f
Author: mef <mef@chromium.org>
Date: Mon Jun 05 15:22:31 2017

[Cronet] Build static libcronet.a for iOS with complete dependencies.

- Hide dependencies inside of the library to avoid symbol conflicts.
- Add cronet_consumer_static target that builds consumer app with static library.

BUG= 721922 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2807283002
Cr-Original-Commit-Position: refs/heads/master@{#475158}
Committed: https://chromium.googlesource.com/chromium/src/+/3412c3c60b69798430b3a3cc9d2834f5d44e6042
Review-Url: https://codereview.chromium.org/2807283002
Cr-Commit-Position: refs/heads/master@{#476986}

[modify] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/ios/cronet_consumer/BUILD.gn
[add] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/tools/dummy.py
[add] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/tools/hide_symbols.py
[modify] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/tools/package_ios.py

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 5 2017

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

commit f035c8c0053e7bcec27f968bf7fbbfc062369e0f
Author: mef <mef@chromium.org>
Date: Mon Jun 05 15:22:31 2017

[Cronet] Build static libcronet.a for iOS with complete dependencies.

- Hide dependencies inside of the library to avoid symbol conflicts.
- Add cronet_consumer_static target that builds consumer app with static library.

BUG= 721922 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2807283002
Cr-Original-Commit-Position: refs/heads/master@{#475158}
Committed: https://chromium.googlesource.com/chromium/src/+/3412c3c60b69798430b3a3cc9d2834f5d44e6042
Review-Url: https://codereview.chromium.org/2807283002
Cr-Commit-Position: refs/heads/master@{#476986}

[modify] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/ios/BUILD.gn
[modify] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/ios/cronet_consumer/BUILD.gn
[add] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/tools/dummy.py
[add] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/tools/hide_symbols.py
[modify] https://crrev.com/f035c8c0053e7bcec27f968bf7fbbfc062369e0f/components/cronet/tools/package_ios.py

Comment 13 by mef@chromium.org, Jun 6 2017

Labels: -M-60 M-61
Cronet Build 61.0.3122.0 includes both static and dynamic framework.

For the record there are several remaining open items:

1. The x86 debug size of libcronet_deps_complete.a is over 700mb and it causes
problems with ld, so we use ar to build cronet_static_complete.a without hiding
symbols for this configuration. This causes problems later on, when static
Cronet.framework becomes too big to get into internal source repository.

Is there a good way to strip some of the debug info prior to ld? 

2. Depending on lipo_binary target to produce fat static library results in errors
with additional_target_cpus where lipo tries to merge 2 libraries with the same
architecture. We need to do something similar to ios_framework_bundle to properly handle
additional cpus.

3. cronet_consumer_static is currently excluded from fat builds. This needs to be fixed.

4. The dummy.py script is used to put static framework output into "Static/Cronet.framework" . There should be some way to avoid having dymmy.py script.

5. hide_symbols.py and ios_static_framework template should move to location, where they could be used by ios webview.

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 13 2017

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

commit 325d2faef60c2a00ea953f943f5287666188476f
Author: mef <mef@chromium.org>
Date: Tue Jun 13 18:41:06 2017

[Cronet] Strip debug symbols from i386 static framework for iOS.

The size of Cronet library with complete dependencies for i386 exceeds 700mb.
This causes problems with internal toolchains.
Stripping debug symbols reduces binary size to reasonable 65mb.

BUG= 721922 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2935103002
Cr-Commit-Position: refs/heads/master@{#479077}

[modify] https://crrev.com/325d2faef60c2a00ea953f943f5287666188476f/components/cronet/tools/hide_symbols.py

Cc: linds...@chromium.org
Can an update be posted to this bug please? It is a P1 that was targeted for M61. Is this fixed?

Comment 16 by mef@chromium.org, Sep 12 2017

Status: Fixed (was: Started)
It is fixed in a sense that Cronet builds include BOTH Dynamic and Static framework.

Ideally we will choose one or another, but I'm not sure whether we have a clear winner. I've asked for update on internal tracker b/36763054.

Sign in to add a comment