Currently, ImageLoadingContent gets the imgILoader each time it loads an image. it would be better to cache it in a class static and release it when the module is unloaded. similar for nsIIOService.
Comment on attachment 122220 [details] [diff] [review] patch images still load fine mozilla also works fine if imglib is not present
Comment on attachment 122220 [details] [diff] [review] patch Excellent. I'd actually ask for approval, as this may help pageload some....
Comment on attachment 122220 [details] [diff] [review] patch this patch just caches two services that would otherwise be gotten on each image load. as this will probably improve Tp a bit, it would be nice to have it in 1.4b.
bz / christian are you sure this is a perf win? the reason I ask is that under the covers, getService() does some caching. I think repeating callings are just going to be lookups in a hash table. what's the risk-to-reward for this?
// Get the image loader... - nsresult rv; - nsCOMPtr<imgILoader> loader = do_GetService("@mozilla.org/image/loader;1", &rv); Remove the comment ;-) Seth: you're asking whether calling a function which then calls a function to do a hash lookup could be compared to accessing a static variable? Yes hashtable lookups are relatively cheap when compared to constructing objects but two function calls are much more expensive than the cost of a single static variable.
There's basically no risk, Seth. Static var access is _much_ faster than hashing the contractid string every time... I'm still looking into what may have caused a slowdown when I landed loading images from content.... and one of the things image frames did was cache the IOService.
Comment on attachment 122220 [details] [diff] [review] patch timless, the claim was that this fix will improve Tp. I'm just pointing out that it may not make a difference. does someone have some actual Tp numbers, or would we need to land and check tinderbox? based on bz's comment, a=sspitzer but let's keep an eye out for possible shutdown problems. also, once this lands, can someone report back with the Tp improvements? that will help me a= faster next time I see something like this.
checked in; I do not (yet) have actual Tp numbers, partly because Tp tests can only be run from inside the firewall I will watch tinderbox and paste Tp numbers here when I have them. Even if this does not help Tp much, it shouldn't hurt it, so I believe that this patch was worth getting checked in. marking fixed.
No effect on Linux btek and luna ~10 ms (~1.65%) Tp improvement on monkey which surprises me, not even the other OSX box (silverstone) shows a change. No change on WINNT Creature.
ah. the monkey improvement looks like it was just some noise