User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050812 Firefox/1.0+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050812 Firefox/1.0+ Firefox does not appear to cache the resized image used as the favicon displayed in the address bar and tab of tabs showing high-resolution images. The CPU overhead of performing the scaling on every redraw is very noticeable, and makes Firefox impractical for viewing galleries of photographs taken from digital cameras by opening them in tabs, for example. Reproducible: Always Steps to Reproduce: 1. Open the image from the URL on multiple tabs, say about 10. 2. Switch between the tabs, open and close menus, resize the browser window, etc. 3. Actual Results: Browser practically unusable on my dual processor Athlon MP 2800+ with 1GB RAM and a Radeon 9800XT running latest Catalyst drivers under Windows XP SP2. It's no longer a top-of-the-range system, but it's faster than most users' machines. Expected Results: No browser slowdown. The 16x16 image could be cached. Experiments show that this seems to occur for some images in different formats or with different colour depths. I understand that the resized-image-caching thing is a core issue rather than a Firefox problem per se, as it also affects scrolling on pages that dynamically resize images. However, using images as thumbnail icons on tabs and in the address bar is a change for Firefox 1.5 which makes the browser unusable for viewing high-res images from a Photo CD, digital camera or online gallery, and there's currently no way to control it. I don't believe those situations are atypical. In raising the bug on this component I wish to encourage consideration of whether a general fix is possible for Fx1.5, whether a work-around for the specific case of favicons is conceivable, or whether it's worth disabling the preview icons by default until the problems can be addressed (by the Cairo stuff?) given that the feature wasn't present in 1.0.

This one is evil evil evil. Looks like we need to find the magic size cap for this feature. Normal images don't leak, these do, even one at a time. http://www.nasa.gov/images/content/125061main_image_feature_385_ys_full.jpg http://www.nasa.gov/images/content/124923main_image_feature_383_ys_full.jpg http://www.nasa.gov/images/content/123817main_image_feature_376_ys_full.jpg http://www.nasa.gov/images/content/124995main_image_feature_384_ys_full.jpg http://www.nasa.gov/images/content/123688main_image_feature_375_ys_full.jpg Vlad says this is known, we really just need to clamp the size of the image (to what, we haven't tracked down yet. My guess is that its only on images that are resized in content as well, but that's anecdotally based.

related to or dupe of bug 302858 ?

I can't reproduce this, but it may be something particular to ATI's GDI implementation. With a nvidia 6800GT, on a 2GHz Sempron, 1GB RAM, I can load up all of the above images in tabs, and quickly switch between them. Interesting to note is that when I first switch to another tab, memory usage jumps drastically, then falls back down (e.g. 23mb -> 50+mb on a switch, then falls back down to 23mb). Something strange is going on, though.. I opened up the images in tabs, and clicked on the 375 image's tab (it's one of the bigger ones). - Memory usage is 23.5MB. I just let firefox sit there, not touching the mouse or anything. - At exactly the 1 minute mark, memory usage started climbing rapidly to 101MB, and it's staying there now. An additional minute or two didn't cause an increase in memory usage. - If I move the mouse at this point, memory usage immediately drops to 62MB. - If I just let it sit, again at the 1 minute mark, memory usage jumps to 101MB. - Move the mouse again, drops down to 44mb (I moused over some of the other image tabs). - If I click on another tab, especially an about:blank one, memory usage drops back down to 23.5MB. So, there's no memory leak here per se, but we do have some very bizzare allocations. While loading I can understand the memory spike; we have to keep the entire image in RAM until we can convert it to a windows device bitmap (which won't show up in our memory usage, I don't think). However, the 60 second memory usage spikes are very strange. We may want a pref to disable image favicon-ing, if we don't have one already.

Note that something happens at the 60 second mark even if the large-image tab isn't at the front. Is windows evicting the image data back into application space after 60 seconds?

Note that it's entirely possible that ATI's drivers aren't loading these images into video ram, so that drawing them involves transferring the image across the bus every single time.. which is why it could be sloooooooow.

mconnor: were you able to reproduce this yourself? if so on which machine and which build? I'm not having any luck, the most I see is what Vlad sees, but I never hit a slowdown. Can we get more exact details about how the browser is unusable? Does it hang before it resizes? Do you hit a CPU spike, or experience a memory leak?

What I see is consistent with Vlad's hypothesis in comment 5: the full image being transferred to the video card each time it's rendered (only if it's being scaled?). Nearly 50% of the CPU time (a single processor in the dual-CPU system) is spent in kernel time for over a second (with six or more large image tabs), even when doing something as trivial as causing a tab to be redrawn (by hovering over it with the mouse to get the orange highlight, for example). As for memory usage, loading the image causes a jump from 43 to 58MB, but most of that disappears afterwards. It's not the memory usage that bothers me. I can confirm that I don't have a problem on some nVidia-based systems that I tried: a dual Xeon at work, or even a laptop with a Geforce mobile chipset. Due to the bitmap cache, it doesn't even affect terminal services. I've been running Catalyst 5.7 on the ATI machine (and the problem has existed in previous versions), so I'll upgrade now that 5.8 is out. As I mentioned, the slow resizing problem also affects images that are resized in-page, and Internet Explorer also suffers from that problem, but favicons make the problem worse on Firefox. A pref would help me, but it's not an ideal solution for other ATI users, if that's the root cause. Of course, I'm willing to provide any specific information or perform any tests/experiments that you ask of me. For what it's worth, there's some evidence that other users are having this problem, notably on other platforms (Mac): http://forums.mozillazine.org/viewtopic.php?p=1565017

Odd, I can't reproduce this now, at least not the massive leaks I was seeing. There's some spiking when switching tabs, but I'm not sure whether that's only the tab icon or showing the image itself. Its intermittent and subtle, but this is a dual Xeon system.

I can no longer reproduce anything except a bit of background CPU stuff, but I don't have a good machine to test perf stuff with.

James - so you are saying you don't have a memory leak, but the browser is considerably slower? These would be two separate issues. Can you give us specific steps to reproduce, like which images you have open, if they're scaled or full size, how many of them are open when you see this, etc. Basically give us the step by step so that we can do exactly what you're doing. Also, what version of ff are you using? I couldn't reproduce any slowdown on 1.0.6 or on the latest nightly.

majken, Peter's bug 302858 covers the memory leak, I think. This bug relates to performance, and seems to only affect machines with ATI graphics cards. My problems are described in the opening comment. To reproduce, open the link from the URL in ten background tabs. After the tabs have loaded, move the mouse cursor horizontally from left to right over the tab bar. Simply redrawing the tabs to show the orange mouseover highlight causes the CPU utilisation shown in the attached picture. If the browser is minimised and restored, otherwise resized, or a window is dragged over the top, etc, similar CPU usage occurs and the window will take about five seconds to redraw. The tabs themselves aren't even showing and there is only one image involved. This has been problematic since the images-as-favicon change was made, but I'm currently using a build taken from the latest-mozilla1.8 directory earlier today: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050821 Firefox/1.0+ Since Vlad's suggestion that it could be ATI drivers to blame, I have tested a number of machines, including a Pentium M laptop, a dual P4 Xeon, an Athlon 64 X2 (dual core), an Athlon XP, and a dual Athlon MP. The X2 is PCI Express and has been tested with an ATI X700 and an nVidia 7800GTX. The others were tested with whatever they had: a 420 Go, some low-end Quadro, an ATI 8500 and an ATI 9800XT. With ATI cards the problem was evident, and with nVidia it wasn't. Conspicuously absent from this matrix is an Intel+ATI combination, or any platform other than Windows XP SP2, but I may be able to them later, and see the thread I referenced earlier. I'll see what else I can do to diagnose what's going on. Guidance would be welcome. It seems that there's a problem with the ATI drivers and I'm bothered that drawing an unscaled image is an order of magnitude faster than drawing a scaled image. I'm equally bothered that some large greyscale images aren't affected, but colour images are, even if I change the colour depth of the display.

sysKin on IRC broke into firefox during the period of 100% CPU activity, and got a stack trace.. bryner was nice enough to do the munging necessary to convert the manual stacktrace into symbols.. the partial trace is: nsImageWin::Draw() [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/gfx/src/windows/nsImageWin.cpp:635] nsRenderingContextImpl::DrawImage() [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/gfx/src/shared/nsRenderingContextImpl.cpp:378] nsImageBoxFrame::PaintImage() [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsImageBoxFrame.cpp:465] nsImageBoxFrame::Paint() [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsImageBoxFrame.cpp:406] Let me know if you need more... it looks like the GDI function being called in that trace is SelectObject(). ===== Not sure what SelectObject does; pav? (It's also the call right after the actual rendering operation, so it could be that he cought the stack right after rendering finished or something.) But this is certainly something that windows is doing and not us, directly.

This is probably our bug that's being made worse by drivers that aren't optimizing things as much as they could be. There's no reason we should be drawing the full image each time for a thumbnail; unfortunately we don't have any services in place for caching a thumbnail. It's entirely possible that ATI's drivers (rightfully, in most cases) assume that any images past a certain size won't be used frequently, so they don't bother to put them into video ram. A pref with max image size (width*height) beyond which we wouldn't set the thumbnail should take care of this for now. I'd reccommend a value of 768*1024..

Patch that adds a pref called "browser.chrome.image_icons.max_size" with a default of 1024; if either image dimension exceeds this value, then that image gets no tab icon if its loaded in a tab. I think 1024 should fix things; however, if people still see the slowdown with this once it goes in while viewing images that are close to that limit (say, looking at a 1024x1024 image), we should drop it lower until the problem goes away. I'm trying to get a machine with an ATI in it here to test this locally as well.

Comment on attachment 193506 [details] [diff] [review] 304561-limit-image-icon-size.patch > if (browser.contentDocument instanceof ImageDocument) { > if (this.mPrefs.getBoolPref("browser.chrome.site_icons")) >- this.setIcon(aTab, browser.currentURI.spec); >+ { >+ try { nit: bracket on previous line, note that the line directly above uses different style... >+ if (!req) >+ return; >+ if (req.image.width > sz || >+ req.image.height > sz) >+ return; if (!req || req.image.width > sz || req.image.height > sz) return; r=me on the toolkit/pref bits, hopefully bz can r+sr the content bits

Is there any reason we can't do what you suggested above, vlad, and "simply" generate a resized version of the image and set that as the favicon instead of resizing the big one each time? We could also use that image as the favicon when you bookmark an image, if we did that, without putting multi-megabyte images into the bookmarks file.

(In reply to comment #16) > Is there any reason we can't do what you suggested above, vlad, and "simply" > generate a resized version of the image and set that as the favicon instead of > resizing the big one each time? We could also use that image as the favicon when > you bookmark an image, if we did that, without putting multi-megabyte images > into the bookmarks file. That sounds great; let me know how I generate a resized version of the image and set it as the src for an <img> :)

Well, one inefficient way would be to draw the image to a canvas and then grab a data: URI out of the canvas... But I'm not suggesting doing it that way. Can't imagelib provide some sort of convenient API to do something like this?

(In reply to comment #18) > Well, one inefficient way would be to draw the image to a canvas and then grab a > data: URI out of the canvas... But I'm not suggesting doing it that way. Can't > imagelib provide some sort of convenient API to do something like this? Actually, that would be a pretty efficient way; the problem is that we have no way of encoding an image back into data for creating that data URI. We'll have something in place for gecko1.9/future, but not for ffox 1.5.

Okie dokie. Should we file a bug on this, or leave this one open on the matter?

Comment on attachment 193506 [details] [diff] [review] 304561-limit-image-icon-size.patch Why not just expose the imgIRequest for the image instead of the nsIImageLoadingContent? I see no reason someone would need the DOM node itself, and the fact that there even is one is really an implementation detail...

Updated patch that just exposes the imgIRequest.. good idea! :)

Comment on attachment 193735 [details] [diff] [review] new patch, with bz's comments >Index: content/html/document/src/nsImageDocument.cpp >+nsImageDocument::GetImageRequest(imgIRequest** aImageRequest) >+ if (mImageContent) { do_QI is null-safe; no need for this null-check. >Index: toolkit/content/widgets/tabbrowser.xml >+ if (!req || >+ req.image.width > sz || >+ req.image.height > sz) Fix the indent here? And you probably want a "!req.image ||" in there too -- req could be non-null but could have a null image... sr=bzbarsky with those nits.

Checked in on branch and trunk with nits fixed

can this one get "fixed1.8" ? The problem appears to be solved

(In reply to comment #20) > Okie dokie. Should we file a bug on this, or leave this one open on the matter? Was there a follow-up bug filed?

Filed bug 305986.

I don't understand how the patch here would fix a memory leak (bug 302858). Doesn't it just avoid the case that causes the leak to show up?

*** Bug 325219 has been marked as a duplicate of this bug. ***