Closed Bug 767320 Opened 9 years ago Closed 9 years ago

Addon.hasResource() should cache results to try avoid unnecessary IO in the future

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: Unfocused, Assigned: darktrojan)

Details

Attachments

(1 file, 1 obsolete file)

Addon.hasResource() does a fstat (or looks up the zip entry) to see if a file exists every time it's called. However, that result shouldn't change (we actively discourage addons from modifying themselves, as that can change the file modification time on their directory on some OSes). As such, we can potentially avoid some IO by caching the results.
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #641806 - Flags: review?(bmcbride)
Comment on attachment 641806 [details] [diff] [review]
v1

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

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +3843,5 @@
>      if (aAddon._installLocation.locked)
>        throw new Error("Cannot uninstall addons from locked install locations");
>  
> +    if ("_hasResourceCache" in aAddon)
> +      aAddon._hasResourceCache = new Dict();

Would like to use start using Map, instead of Dict.jsm (all the functions are exactly the same anyway).

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Map
(Ignore the scary warning - the only thing not done yet is iterating over Maps, which we don't need here.)
Attachment #641806 - Flags: review?(bmcbride) → review-
Attached patch v2Splinter Review
Attachment #641806 - Attachment is obsolete: true
Attachment #642212 - Flags: review?(bmcbride)
Comment on attachment 642212 [details] [diff] [review]
v2

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

\o/
Attachment #642212 - Flags: review?(bmcbride) → review+
https://hg.mozilla.org/mozilla-central/rev/abe7818a3f52
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.