686637 - Stop extracting hyphenation files from the APK

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Extracting the hyphenation files kills startup performance on initial launch after install or update. Until bug 655337 creates a way to read these files from the APK/JAR, we should just stop trying to use them. Yes, we would lose hyphenation support, but right now the cost is too high.

Perhaps we should go even further and stop _shipping_ hyphenation files period. We don't ship spellcheck dictionaries for the same reason. There are 2.5MB of hyphenation files in our APK. I don't have the compressed size impact on the APK, but not shipping would also save APK size.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We should definitely do this.

Hyphenation's nice on mobile but given it's rarely used by Web authors so far, it's totally not worth it for now.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 1

* Remove hyphenation files from APK
* Remove code that tries to unpack files from APK

Sent to Try server. We might need to disable some tests.

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 560211 [details] [diff] [review]
patch 1

Review of attachment 560211 [details] [diff] [review]:
-----------------------------------------------------------------

looks fine.  follow up with any test failures you have to disable.

Comment 4

[Jonathan Kew [:jfkthame]]

Note that there are now patches in bug 655337 that allow the patterns to be loaded directly from the APK, without extracting them to separate files.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/718c6db12804

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 560211 [details] [diff] [review]
patch 1

This is an extremely cheap, simple patch that just removes some files we were shipping and extracting during first launch. The result is a dramatic startup improvement during first launch after an install or update.

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to Mark Finkle (:mfinkle) from comment #6)
> Comment on attachment 560211 [details] [diff] [review]
> patch 1
> 
> This is an extremely cheap, simple patch that just removes some files we
> were shipping and extracting during first launch. The result is a dramatic
> startup improvement during first launch after an install or update.

And the other result is the removal of a feature that we shipped in FF6. I realize it's not (yet) widely used on the web, but it is a long-standing request from web authors that we finally began to address; removing it would be regrettable.

(It's important to remember that this is only about behavior on *first* launch, not subsequent startup times, so users on the release channel would typically see the impact just once every 6 weeks.)

If we're going to do something like this for Aurora, I think we should consider leaving the en-US dictionary (and the code to extract it) in place, so that we don't actually regress the feature as already shipped. That would still provide almost all of the first-launch improvement, as we'd only be extracting one file instead of 30-plus.

Also, if we can get bug 655337 reviewed and landed, we'll have a solution that fixes the first-launch startup issue without removing any of the functionality.

Comment 8

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/718c6db12804

Comment 9

[[:Cww]]

Comment on attachment 560211 [details] [diff] [review]
patch 1

Drivers agree this can land in aurora given risk assessment.

Comment 10

[Jonathan Kew [:jfkthame]]

While I sympathise with the desire to avoid the cost of unpacking all those files, I think it's a mistake to pull *all* hyphenation support including the en-US dictionary, which has been in the shipping product since FF6. This will regress behavior for sites that have begun to adopt the auto-hyphenation feature (which is indeed in use on live sites).

So I have filed bug 688779 on restoring the en-US dictionary (only) to the Android package, as an interim measure until bug 655337 (which will allow us to use the dictionaries without the unpacking step) gets done.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

Startup performance is one of the critical metrics we have and not extracting the files improves performance, even if for firstrun. Removing the bulk of the hyphenation files from the APK reduces our footprint on the device and reduces the size of the APK, which also helps startup time.

Adding back en-US wouldn't be too bad, but I'd rather focus on getting bug 655337 landed and see the affect on performance. We still might want to consider a subset of hyphenation files, if the affect on APK size/speed is measurably negative.

I landed this on Aurora. It's possible we could reconsider and talk about bug 688779, but I wanted to be sure the baseline patch was landed.

https://hg.mozilla.org/releases/mozilla-aurora/rev/9b32a50de895