Don't even bother trying to unpackComponents if the APK hasn't changed

RESOLVED FIXED in Firefox 8

Status

()

Core
Widget: Android
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

({mobile, perf})

Trunk
mozilla8
ARM
Android
mobile, perf
Points:
---

Firefox Tracking Flags

(firefox8 fixed, firefox9 fixed)

Details

(Whiteboard: [mobilestartupshrink][ts][qa-])

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 557759 [details] [diff] [review]
Patch

During the Android startup, we attempt to unpack files from the APK. The current code does tests on the files (date and size) to make sure we only write out files when they are new.

Still, all the checks add up and on devices with slow file systems, all the .exists() and .lastModified() and .length() checks can take up a non-trivial amount of time.

From my tests on the nexus one, galaxy tab, and a Try server run on the Tegras, it's around 200ms.

This patch sets the last modified timestamp to the APK last modified timestamp when creating the "components" folder. If the folder exists and the timestamps are the same, we just leave.
Attachment #557759 - Flags: review?(doug.turner)

Comment 1

6 years ago
Comment on attachment 557759 [details] [diff] [review]
Patch

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

looks good, small tweaks needed.

::: embedding/android/GeckoApp.java
@@ +461,5 @@
>          throws IOException, FileNotFoundException
>      {
> +        File applicationPackage = new File(getApplication().getPackageResourcePath());
> +        File componentsDir = new File(sGREDir, "components");
> +        if (componentsDir.exists() && componentsDir.lastModified() == applicationPackage.lastModified())

You don't need the first call to .exists().

If componentsDir doesn't exists, then the call to lastModified() will return zero and that will not match the applicationPackage.lastModified().  so, just:

if (componentsDir.lastModified() == applicationPackage.lastModified())

@@ +468,2 @@
>          componentsDir.mkdir();
> +        componentsDir.setLastModified(applicationPackage.lastModified());

If this line fails, which could happen if utime fails, you won't unpack.  Please test for failure.  (it returns a bool.  false == failure)
Attachment #557759 - Flags: review?(doug.turner) → review-

Comment 2

6 years ago
ignore the comment about the change I wanted against setLastModified.  If the system doesn't support utime for some reason, worse case, we will unpack everytime.
Created attachment 557766 [details] [diff] [review]
patch 2

Makes the first change, but skips the second. The worst thing that should happen from a failed setLastModified call is that we unpack the next time around.
Assignee: nobody → mark.finkle
Attachment #557759 - Attachment is obsolete: true
Attachment #557766 - Flags: review?(doug.turner)
Keywords: mobile, perf
OS: Linux → Android
Hardware: x86 → ARM
Whiteboard: [mobilestartupshrink][ts]
Version: unspecified → Trunk

Updated

6 years ago
Attachment #557766 - Flags: review?(doug.turner) → review+

Updated

6 years ago
Attachment #557766 - Flags: approval-mozilla-beta?
Attachment #557766 - Flags: approval-mozilla-aurora?
http://hg.mozilla.org/mozilla-central/rev/ad61ee019ba8
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 557766 [details] [diff] [review]
patch 2

Too much for Beta. Will try for Aurora.
Attachment #557766 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

Comment 6

6 years ago
Comment on attachment 557766 [details] [diff] [review]
patch 2

Approved for releases/mozilla-aurora. Please land asap.
Attachment #557766 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/6be19f1139e0
status-firefox8: --- → fixed
status-firefox9: --- → fixed
Target Milestone: --- → mozilla8
Whiteboard: [mobilestartupshrink][ts] → [mobilestartupshrink][ts][qa-]
You need to log in before you can comment on or make changes to this bug.