Closed Bug 395260 Opened 16 years ago Closed 3 years ago

Firefox caches pixmaps to X11, need feature to disable


(Core :: Graphics: ImageLib, defect, P3)






(Reporter: jim, Unassigned)


(Depends on 1 open bug, )



(4 files, 7 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061025 Firefox/2.0 (Swiftfox)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061025 Firefox/2.0 (Swiftfox)

Firefox caches pixmaps to X11. On a full workstation with plenty of RAM and tons of hard drive space, this probably wouldn't be seen as a bug. But when running thin clients or any type of diskless workstation this is a HUGE problem. For an example of what happens and to provide repeatability for testing here is some information.  

Install Edubuntu 7.04. Boot a thin client. Install xrestop on the server. Log into client and open xrestop in a terminal. Then open Firefox while leaving xrestop visable. Browse standard less graphic intense websites and watch the cache usage as displayed by xrestop....not bad. Now hit this webpage:

You will see the X cache climb drastically until Firefox completely consumes all available RAM and Swap space at which point the client completely freezes and needs to be hard rebooted.

Now the example site above is a certain exaggeration of the problem. But it is a site that will certainly crash about every thin client system you try it on. As a workaround one can add massive amounts of RAM to the thin client (512MB+) or increase nbdswap to a very high amount (1GB+). However if a user hits the right site, the terminal will still crash. If a browser such as Opera or Konquerer are used and monitored with xrestop, you can hit the same site above and see very little increase, and a client with 128MB RAM and 32MB swap will run perfectly.

This is a bug that has apparently plagued mozilla based browsers for years and has not yet been resolved. Please remove this "feature" or at least provide a way to disable pixmap caching to X from within about:config such as "browser.cache.pixmap.enable user set boolean false". I have tried every other config option and none of them disable the pixmap caching.

Again this isn't the standard report of "Firefox is a memory hog" or "Firefox has memory leaks", etc.  The problem is that Firefox caches the pixmaps at all.  This simply is not able to be done on thin clients without problems.  The ability to disable this feature is key to the future of thin client usage (Edubuntu, K12LTSP, LTSP in general, etc).  Otherwise users of thin client systems will be forced to use alternative browsers.

Jim Kronebusch

Reproducible: Always

Steps to Reproduce:
1. install xrestop
2. log into client and run xrestop
3. with xrestop running browse the listed website, be sure you can see xrestop
4. watch usage of memory climb until client freezes
Actual Results:  
Firefox caches all pixmaps from the currently viewed page to X until X eats all available system memory and freezes the client.

Expected Results:  
Loaded the webpage with minimal system memory use.
see bug 259672 and the patch in bug 296818
I referenced both the bug and the patch above.  I have the mozilla source downloaded and the diff and have been trying to get this to work with patch, but get many errors.  I would love to test if this patch would fix the pixmap problem.  If anyone could post some quick steps I'll give it a whirl.

Jim Kronebusch
Closed: 16 years ago
Resolution: --- → DUPLICATE
I think this would be better addressed by a different solution from bug 296818.  See bug 296818 comment 27.
Resolution: DUPLICATE → ---
To summarize that comment, I think it makes sense to have a preference for maximal size of the pixmap cache we end up with.  Once we get there, we stop decoding new images that would take us over the cache size.  The default can be either infinite or something large enough that not rendering images is better than trying to render them at that point.  Deployments in low-resource situations (e.g. mobiles with no virtual memory, thin clients, etc) can set the preference as needed.
Component: General → ImageLib
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → imagelib
Version: unspecified → Trunk
Flags: blocking1.9?
there is already a feature to disable caching image data as pixmaps (we don't cache pixmaps to X11).

Flags: blocking1.9?
not sure what people are hoping to get out of this bug.  if it is "need feature to disable" then it is already there (dup of bug 336191).

If you want to be smarter about the way we allocate pixmaps, that could be done separately and in addition to what we already have.
Stuart, where is the MOZ_DISABLE_IMAGE_OPTIMIZE=1 firefox to be set?  Do we just add that to about:config?  I am not sure if I am misreading your statement in Comment 8 or not.  If I monitor resources with xrestop, I can see system memory usage spike as a result of Firefox hitting an image intense website (see the example website in Comment 1).  I looked at bug 336191, but still don't understand where to set the parameter and am not clear on if this patch is already present in current release firefox.  

Boris, I think your suggestion for a user settable limit is a good idea.  Is it possible to set so that once the limit is reached, older data is simply overwritten still allowing new data to be cached but not exceeding the limit?

> Stuart, where is the MOZ_DISABLE_IMAGE_OPTIMIZE=1 firefox to be set? 

In the environment.  Shell-agnostic command-line:


> not clear on if this patch is already present

It's present in the alphas of Firefox 3.
I have a build of firefox-granparadiso to test Federico's patch.  I launched it at the command line with env MOZ_DISABLE_IMAGE_OPTIMIZE=1 firefox-granparadiso.  Then I loaded the site to see what happened and monitored with xrestop.  Pixmap storage would hit very close to 25MB then be cleared and start over.  This seemed to work very well.  If I simply launched without the environment variable, Pixmap storage would climb to over 500MB+ by the time the page finished loading.  This seems acceptable.  Is this possible to do in Firefox and is it possible to set the environment variable permanently?

I could still see a use for a user settable limit on storage, and the ability to set this environment variable in about:config to make it more user friendly.  A settable discard timer would also be helpful.  All of these would make Firefox more usable in low memory situations and still allow defaults to be set for performance in standard higher memory situations.

> Is this possible to do in Firefox

No.  The support for this is in the new Thebes rendering code.  I suppose you could try to backport that fix to branch, but...

> is it possible to set the environment variable permanently

In your own account?  Of course.  See your shell's documentation.
(In reply to comment #11)
>  Then I loaded the site

Something strikes me about the way firefox (or is it Gecko?) deals with this page.   The real cause of this issue is that the page contains a number of massive images, which are embedded at small resolution in the page.  We all know this is down to bad web page authoring, but obviously firefox has to deal with it.  My laptop slows to a crawl loading this page.

Arguably the reason for the problems in that webpage is that firefox uses the video card to do the resizing -- therefore caching a huge quantity of image data which it will never actually display (unless the person chooses to view the image which is probably quite rare).

I realise the video card will do the resize more quickly, but if firefox resized the image in software, the X server's RAM usage would be fairly minimal as it would be caching the data which was actually needed.  There is often a balance between cpu and ram usage, this seems to be one of them. 

I would therefore be inclined to suggest an optional feature (particularly for remote X servers or low ram situations) where firefox would resize the image in software and send the resized pixmap to the X server. Perhaps this would only be used where the original image was above a certain size -- or where the display size was under some fraction of the image size (eg less than half the width and height).

The X server seems often to do a simple resample, rather than a resize, so I would expect firefox to do the same.
Gavin, maybe this is how non-gecko based browsers are handling this and possibly why they are not affected in the same fashion.

Boris, Stuart, is this possible?
Storing images at the original size is a pretty long-standing imagelib design decision.  You might want to read the various existing bugs that discuss that behavior.
Patch prescales images overflowing their Thebes destination rectangle because they've been resized with the "width" or "height" html tags. The patch uses the GDK library to perform downscaling using a bilinear interpolation algorithm. We also optimized the consumed X memory by only transferring the visible portion of a pixmap between the X application and server. This also reduces bandwith consumption and reduces the delay before an image is painted by X. Pixmaps are now kept in the application's memory instead of X's.

This patch only affects the behavior of pixmaps storage on UNIX systems. It could easily be ported to other platforms if similar issues are discovered.

Complete blog posts :
Attachment #328690 - Attachment is obsolete: true
Given that the same nsThebesImage can be drawn to multiple different destination rectangle sizes how does this page behave?  Do we end up thrashing the pixmap on such draws?
The complete pixmap is always kept in Firefox's memory while one subpixmap is generated and sent to X for each destination rectangle. These subpixmaps do not trash the original pixmap and are only used for painting in X.
Yes, I understand all that.  But the point is that before this patch we send the data to the server _once_ and then use it to paint whenever we need, right?  With this patch, we send the data on every sub-rectangle paint?  And multiple times if the image appears on the page multiple times?

How is scrolling performance with a non-local X server with this approach?
Data needs to be retransfered to the server for images that need resizing or
those that are partially visible. When it's transfered once, image
manimuplation is done by X which leads to poor quality in the case of image

I've just tested scrolling and I confirm that the feeling is as good as with
the FF 3.0 official build on a local machine (for both regular and smooth

There is a very slight difference when scrolling with a thin client caused by
the transfer of images between the server and client. I will limit image
manipulations to find the best balance between memory consumption and bandwith
usage (e.g. only pre-scale under a certain ratio).
After testing the patch in a few environments, I discovered that the scrolling quality could be sluggish when Firefox was used on a thin client with an application server having low resources. Since we can assume that a thin client’s backend should have the minimum ressources to support mallocs without using the swap area, there should be no visible difference of usage on a thin client. 

To give users control over image manipulation, I added the “browser.gdk_interpolation_threshold_percent” preference variable that allows values between 1-100 and has for effect to either use or bypass GDK image manipulations. The default value is 50% which only affects images being scaled by a factor of 50% or 200% and images with a visible portion under 50%. Lowering this value to it’s minimum (1%) would turn the feature off while using 100% will premanipulate any image overflowing it’s container. 

Finally, as Joe from #gfx did mention, I added support for images upscaling with GDK to increase the image quality by also using the bilinear interpolation algorithm.
Attachment #328694 - Attachment is obsolete: true
So you tested the patch with X over a WAN link (not just thin client, but appreciable latency and low bandwidth) and there was no impact?
Boris, I tested X over WAN and there is no difference between the original FF 3.0 and my patched version.
Attached patch update - missing all.js file (obsolete) — Splinter Review
Attachment #329705 - Attachment is obsolete: true
needs reviewal

intensive testing has been done with that patch on both thin clients and desktops and no issue was reported.
Attachment #329724 - Attachment is obsolete: true
Comment on attachment 330932 [details] [diff] [review]
optimized/increased quality + fixed interlacing issue

+/*#ifdef XP_UNIX
+    mTinySurf = nsnull;
should be removed. can this be done without reuploading a new patch ?
Attachment #330932 - Flags: review?(joe)
Attachment #330932 - Flags: superreview?(vladimir)
Comment on attachment 330932 [details] [diff] [review]
optimized/increased quality + fixed interlacing issue

Some comments on the patch, and then overall comments at the end...

>Index: ./gfx/src/thebes/nsThebesImage.cpp
>RCS file: /cvsroot/mozilla/gfx/src/thebes/nsThebesImage.cpp,v
>retrieving revision 1.86
>diff -u -8 -p -B -b -r1.86 nsThebesImage.cpp
>--- ./gfx/src/thebes/nsThebesImage.cpp	28 Apr 2008 21:27:05 -0000	1.86
>+++ ./gfx/src/thebes/nsThebesImage.cpp	22 Jul 2008 20:36:07 -0000
>@@ -43,48 +44,68 @@
> #include "gfxPattern.h"
> #include "gfxPlatform.h"
> #include "prenv.h"
> static PRBool gDisableOptimize = PR_FALSE;
>+#ifdef XP_UNIX
>+static gfxFloat gImageThreshold = 0.5;

This variable is oddly named; I'm not sure what it means, especially
not from the name.  gImagePreScaleFactor?  Though that's not quite how
it's used, see below.

> nsresult
> nsThebesImage::Init(PRInt32 aWidth, PRInt32 aHeight, PRInt32 aDepth, nsMaskRequirements aMaskRequirements)
> {
>     mWidth = aWidth;
>     mHeight = aHeight;
>@@ -206,29 +227,28 @@ PRInt32
> nsThebesImage::GetAlphaLineStride()
> {
>     return (mAlphaDepth > 0) ? mStride : 0;
> }
> void
> nsThebesImage::ImageUpdated(nsIDeviceContext *aContext, PRUint8 aFlags, nsRect *aUpdateRect)
> {
>+    mImageIncomplete = aFlags & nsImageUpdateFlags_ImageIncomplete;

You only modified the PNG decoder to pass down ImageIncomplete; why is
this info needed at all?  You should be able to tell whether the image
is complete or not via the same mechanism used in GetIsImageComplete.

> nsresult
> nsThebesImage::Optimize(nsIDeviceContext* aContext)
> {
>     if (gDisableOptimize)
>         return NS_OK;
>@@ -326,21 +346,26 @@ nsThebesImage::Optimize(nsIDeviceContext
> #ifdef XP_MACOSX
>     if (mQuartzSurface) {
>         mQuartzSurface->Flush();
>         mOptSurface = mQuartzSurface;
>     }
> #endif
>+#ifndef XP_UNIX
>     if (mOptSurface == nsnull)
>         mOptSurface = gfxPlatform::GetPlatform()->OptimizeImage(mImageSurface, mFormat);

If you want to do this, we should just early-return from Optimize
after the 1x1 check, instead of adding the extra ifdefs.  But I'm not
sure it's a good idea, see below.

>@@ -482,24 +507,108 @@ nsThebesImage::Draw(nsIRenderingContext 
>         return NS_ERROR_FAILURE;
>     // Expand the subimageRect to place its edges on integer coordinates.
>     // Basically, if we're allowed to sample part of a pixel we can
>     // sample the whole pixel.
>     subimageRect.RoundOut();
>     nsRefPtr<gfxPattern> pat;
>+#ifdef XP_UNIX
>+    // We optimize the image surface to it's visible size and use scaling algorithms
>+    // to reduce X memory consumption, bug 395260. We bypass 1x1 rectangles to avoid useless operations.
>+    if (destRect.size != gfxSize(1.0, 1.0) &&
>+        (mWidth > (destRect.size.width / gImageThreshold) ||
>+         mHeight > (destRect.size.height / gImageThreshold) ||
>+         xscale < (1.0 * gImageThreshold) || yscale < (1.0 * gImageThreshold) ||
>+         xscale > (1.0 / gImageThreshold) || yscale > (1.0 / gImageThreshold))) {

I don't understand this check at all; why isn't this just checking if
the xscale is less then gImageThreshold?  Upscaling shouldn't go
through this path, you'll just end up creating a bigger temporary
surface than the original...

>+        gfxIntSize tinySize(NS_lroundf(destRect.size.width), NS_lroundf(destRect.size.height));
>+        GdkPixbuf* pixbuf = gdk_pixbuf_new_from_data(mImageSurface->Data(), GDK_COLORSPACE_RGB,
>+                                                     PR_TRUE, 8, mWidth, mHeight,
>+                                                     mStride, nsnull, nsnull);

All this scaling stuff should be done using thebes, not gdk_pixbuf;
there's no reason why all this code can't be shared on other platforms
as well if it helps.

>+        if (!pixbuf)
>+            return NS_ERROR_FAILURE;
>+        GdkPixbuf* tinyPix = pixbuf;
>+        nsRefPtr<gfxASurface> tmpSurf = gfxPlatform::GetPlatform()->CreateOffscreenSurface(tinySize, mFormat);

There's no reason to do this -- if you're just creating a new scaled
surface each time, just let cairo take care of getting it in whatever
format's useful for the platform.  Optimizing to an offscreen surface
is only useful when you keep that image around.

>+    } else if (!mOptSurface) {
>+        // Sends pixmaps to X
>+        if (GetIsImageComplete()) {
>+            mOptSurface = gfxPlatform::GetPlatform()->OptimizeImage(mImageSurface, mFormat);
>+        } else {
>+            gfxPlatform::GetPlatform()->OptimizeImage(mImageSurface, mFormat);
>+        }

That second call does a bunch of work and then leaks the return.  Why does it need to be called at all?

>--- ./modules/libpref/src/init/all.js	2 May 2008 06:27:27 -0000	3.758
>+++ ./modules/libpref/src/init/all.js	22 Jul 2008 20:36:11 -0000
>@@ -97,16 +97,19 @@ pref("browser.display.show_image_placeho
>+#ifdef XP_UNIX
>+pref("browser.gdk_image_interpolation_threshold", 50);

This shouldn't be present to 50%; it should be set to whatever the
value is for "don't touch anything", and then it can be tweaked from

So overall, I think the idea is a good one, but I think the
implementation is flawed.  Doing a pre-scale on each Draw invocation
might help with X memory consumption, but it'll be horrible for

A better approach would be to keep track of the maximum image size
that was requested in Draw.  If it's bigger than the last max size,
then re-decode if needed and optimize to that size (up to the actual
image size).  If a smaller size is requested, just draw whatever size
you currenly have scaled down.  That would be generally useful on all
platforms, and shouldn't need any heuristics or significant pre-scaling.
Attachment #330932 - Flags: superreview?(vladimir) → superreview-
Attached patch changed GDK for cairo scaling (obsolete) — Splinter Review
- We don't use offscreen surfaces when we're drawing temp surfaces
- We use cairo transforms only to create visible image portions or to scale instead of GDK
- It is now multiplatform (compiled and tested on Windows)
- The modifications for the ImageUpdate flag is required for interlaced images. It is an issue with the GetIsImageComplete() which would return true even if we only had decoded the first pass on an interlaced image.
Attachment #330932 - Attachment is obsolete: true
Attachment #331837 - Flags: superreview?(vladimir)
Attachment #331837 - Flags: review?(joe)
Attachment #330932 - Flags: review?(joe)
Flags: wanted1.9.1?
Depends on: 449655
Flags: wanted1.9.1? → wanted1.9.1+
Priority: -- → P3
Patch was separated in two bugs, PNGDecoding for interlaced image is now referred to bug 449655.
I added some comments and removed the all.js variable. Users must now create the browser.image_prescaling_threshold variable of type integer in the about:config panel.
Attachment #331837 - Attachment is obsolete: true
Attachment #332858 - Flags: superreview?(vladimir)
Attachment #332858 - Flags: review?(joe)
Attachment #331837 - Flags: superreview?(vladimir)
Attachment #331837 - Flags: review?(joe)
Comment on attachment 332858 [details] [diff] [review]
updated patch from reviewers comments

Couple of problems with this patch: 
1. Not having a browser.image_prescaling_threshold pref doesn't disable prescaling.

2. (much more important) It breaks several reftests, including the apng reftest.
Attachment #332858 - Flags: superreview?(vladimir)
Attachment #332858 - Flags: review?(joe)
Attachment #332858 - Flags: review-
Fixed the aPNG tests.
text/zwnj-02.html, bugs/214077-1b.html and bugs/214077-1a.html are still failing but I'm out of ideas.
Some other SVG tests are also failing and this is due to gfxImageFrame::SetMutable where I changed mImage->Optimize() for mImage-SetMutable(). We can't restore mImage->Optimize() or we won't be able to  reduce memory usage.
Attachment #332858 - Attachment is obsolete: true
Are there any news about this bug? We will need this patch into firefox 3.1 to prevent crash of linux thin-clients, caused by large pictures loaded in X server memory. Where is it stuck?
I haven't looked into the other reftest failures, but until they're fixed, there's no way this can be applied.
bugs/214077-1b.html and bugs/214077-1a.html failed on a pristine 3.1b2 on ubuntu intrepid, so it's not related to this. 

I do have some problems, since nsThebesImage.cpp changed a lot and the patch doesn't apply anymore. But I confirm the problem is still there.
are any of these preferences “browser.gdk_interpolation_threshold_percent” or browser.image_prescaling_threshold implemented in a build with firefox? I tried using this in FF3 and FF3.1Beta4 but doesn't seem to impact the image quality
no, attachment 351844 [details] [diff] [review] isn't checked in (it's not even ready to be checked in, see comment 34)
Any progress on this?

While opening a 10 megapixel picture (very common on the web nowadays), Firefox itself is using +40 MB, but it's also forcing Xorg to temporarily use an additional +80 MB of memory.

Those +120 MB are not only crashing thin clients but low-RAM PCs too.

With `MOZ_DISABLE_IMAGE_OPTIMIZE=1 firefox` indeed Xorg doesn't use any additional RAM at all and the clients do not crash.
(btw, shouldn't Firefox be terminated instead of X crashing? Maybe some return value check is missing?)
As mentioned above, chrome doesn't have this issue.

I uploaded a test case in bug 575523 before realizing it was the same problem.
This bug is KILLING me. I have an LTSP server with terminals with 128mb RAM and any time they go to sites with large images scaled down it completely crashes their Firefox.
(In reply to comment #40)
> This bug is KILLING me. I have an LTSP server with terminals with 128mb RAM and
> any time they go to sites with large images scaled down it completely crashes
> their Firefox.

Did you try running:


and see if that helps?  If it does, you could set that environment variable for everyone by putting it in /etc/profile.

I did a small blog entry to explain and compare the effect of MOZ_DISABLE_IMAGE_OPTIMIZE=1

I also join to this bug the script I used for memory monitoring. 

The conclusion is that the option MOZ_DISABLE_IMAGE_OPTIMIZE=1 gets the right behavior: the allocations occurs in firefox itself, not in the Xserver, and that it compares to other browsers. This option should be the default.
Usage example: python /usr/bin/X /usr/lib/firefox-3.6.6/firefox-bin
Usage example: gnuplot-nox mem.plt
This testcase is made from a problem I think is related to this bug. I encounter this bug in a real application on a desktop with lots of memory (basically, I try to load an image like 10 or 20 times a second).

Load the testcase, monitor your memory using e.g. top with memory sort (capital M to enable it), start loading images (first button) and look at the Xorg process taking more and more memory, making your computer swap and sweat.

When you finally stop the loading (second button), the Xork memory stays at the same value. But, at one moment, it is released. It's sometimes done when I switch the tabs back and forth, and it's always done when I close the tab with the testcase.

And sorry to Renault for their wasted bandwidth. ;-)
Attachment #579037 - Attachment mime type: text/plain → text/html
Other informations I forgot in my first comment:
- I tried this on the latest nightly (11.0a1 (2011-12-05)) on a new profile without any extension.
- I couldn't make it happen with data URIs (that's why I had to find an external image).
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111201 Firefox/10.0a2 - Build ID: 20111201042025

Cant confirm Xorg leak. Just FF bloats.
total       used       free     shared    buffers     cached
Mem:       3940852    2472188    1468664          0     143588    1028684
-/+ buffers/cache:    1299916    2640936
Swap:      4192252          0    4192252

23664 XXXX    20   0 1267m 567m  30m S    1 14.7  36:24.63 firefox 
1671 root      28   8  236m  32m 6536 S    0  0.8  97:14.93 Xorg
For me :

after some time downloading images :

             total       used       free     shared    buffers     cached
Mem:          4042       3916        125          0          2         81
Low:           845        726        119
High:         3196       3189          6
-/+ buffers/cache:       3833        209
Swap:         3812       1442       2370

root      2563  1.2 30.2 1302800 1250608 tty7  Ss+  Nov18 303:50 /usr/bin/X ...
jyyw8271 29819  9.1  1.9 390000 80404 pts/9    Sl   15:50   0:38 firefox-nightly/firefox...

Then, I create a new tab, go to this tab, and go back to the tab with the testcase; the memory is freed (it is only when I come back to the tab, not before):

             total       used       free     shared    buffers     cached
Mem:          4042       2796       1245          0          2         93
Low:           845        304        541
High:         3196       2492        703
-/+ buffers/cache:       2700       1341
Swap:         3812       1472       2340

root      2563  1.2  3.4 196456 141780 tty7    Ss+  Nov18 303:55 /usr/bin/X
jyyw8271 29819  7.6  1.3 370540 53932 pts/9    Sl   15:50   0:39 firefox-nightly/firefox..

This is more or less the same state as before launching the testcase.
I wonder if this is somewhat fixed with the recent work about pages with a lot of images.
We'll probably cache fewer pixmaps to X11 now, yes (the ones that are visible or close to visible). I'm not sure if that is the exact goal of the bug or not though.
Closed: 16 years ago3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.