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

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: v1

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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.)

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: v2

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 642212 [details] [diff] [review]
v2

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

\o/

Comment 5

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/abe7818a3f52

(https://tbpl.mozilla.org/?tree=Try&rev=549a42fd271e)

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/abe7818a3f52