Closed
Bug 684152
Opened 13 years ago
Closed 13 years ago
Don't even bother trying to unpackComponents if the APK hasn't changed
Categories
(Core Graveyard :: Widget: Android, defect)
Tracking
(firefox8 fixed, firefox9 fixed)
RESOLVED
FIXED
mozilla8
People
(Reporter: mfinkle, Assigned: mfinkle)
Details
(Keywords: mobile, perf, Whiteboard: [mobilestartupshrink][ts][qa-])
Attachments
(1 file, 1 obsolete file)
2.33 KB,
patch
|
dougt
:
review+
christian
:
approval-mozilla-aurora+
mfinkle
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
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•13 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•13 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.
Assignee | ||
Comment 3•13 years ago
|
||
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)
Updated•13 years ago
|
Updated•13 years ago
|
Attachment #557766 -
Flags: review?(doug.turner) → review+
Updated•13 years ago
|
Attachment #557766 -
Flags: approval-mozilla-beta?
Attachment #557766 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 4•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ad61ee019ba8
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•13 years ago
|
||
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 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+
Assignee | ||
Comment 7•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/6be19f1139e0
Whiteboard: [mobilestartupshrink][ts] → [mobilestartupshrink][ts][qa-]
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•