284978 - After loading many large images, Firefox stops repainting

Status: RESOLVED FIXED

(Core Graveyard :: GFX: Win32, enhancement)

Description

[Jesse Ruderman]

Steps to reproduce: Open about six galleries worth of porn images at once using
the "linked images" bookmarklet.

Result: Firefox stops repainting its interface and content area, with the
exception that it still paints parts of the content area when the content area
is scrolled.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050305
Firefox/1.0+.  I'm using Windows XP.  My video card is NVIDIA GeForce4 MX 4000.

This bug is similar to bug 184933, which was fixed by a patch in bug 205893. It
returned because the patch to that bug was effectively backed out for a
performance gain in bug 284716.

Comment 1

[ArronM (:paper)]

Thanks.  I have a patch for this.  When Moz runs out of GDI objects, we don't
check the results CreateCompatibleBitmap when we make a Drawing Surface.  It
fails, and Moz stops painting.  A check to see if it fails, and using
CreateDIBSection if so, fixes this problem.

Patch soon to come.  I'm looking for any other places we create a DDB without
first checking.

Comment 2

[ArronM (:paper)]

Attachment: Do DDB surface. Do DIB if fails.

Comment 3

[ArronM (:paper)]

Comment on attachment 176455 [details] [diff] [review]
Do DDB surface. Do DIB if fails.

Ere, you are familiar with this code, care to do a review?

Comment 4

[Stuart Parmenter]

The patch looks right to me.  If ere doesn't review shortly I will.

Comment 5

[Ere Maijala (slow)]

Comment on attachment 176455 [details] [diff] [review]
Do DDB surface. Do DIB if fails.

r=emaijala provided you fix the comment (Create a DDB -> Create a DIB) and
remove the prinf from nsImageWin.cpp. 

Are you sure this will let us get away with consuming all resources without
causing other problems? I'm not confident this is a good thing.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Er... what's with the printf in non-debug code?

Comment 7

[Ere Maijala (slow)]

Something I wanted to be removed to get r+ (although I made a typo).

Comment 8

[ArronM (:paper)]

Comment on attachment 176455 [details] [diff] [review]
Do DDB surface. Do DIB if fails.

Woah, nsImageWin wasn't supposed to be in the patch.  Consider it (and thus the
printf) removed.

Comment 9

[ArronM (:paper)]

Well, it's never a good idea to use up all resources.  But there is no way to
determine when we use them up.  It isn't linked to # of GDI objects.  I can make
Moz run out of GDI resources by opening 4 images from my digicam, which are 6MP
each (ie. large), so usage seems dependent on size.  But the limit may be set by
OS/BIOS/Video Card/Video Driver, or maybe it's hardcoded into windows.  Only the
goats know.

It also appears that in Win2k (and hopefully XP), each application gets its own
limit.  When Moz hits it's limit, other apps still painted for me.  But, I have
my doubts that that is true.  The other apps could just be using DIBs when DDBs
run out too!

Comment 10

[ArronM (:paper)]

Attachment: Create DDB surface, if failed, try DIB

Minor nits changed:
-Removal of printf/nsImageWin
-"Create a DDB if we need Pixel Access" changed to "Create a DIB.."
-Exit early if binfo is nsnull

Unless ere or bz objects, I'm assuming the r+ can be transfered from the
obsolete patch, so I'm only asking for a SR.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 176528 [details] [diff] [review]
Create DDB surface, if failed, try DIB

sr=bzbarsky

Comment 12

[ArronM (:paper)]

Checking in nsDrawingSurfaceWin.cpp;
/cvsroot/mozilla/gfx/src/windows/nsDrawingSurfaceWin.cpp,v  <-- 
nsDrawingSurfaceWin.cpp
new revision: 3.21; previous revision: 3.20
done

Comment 13

[Ashley Bischoff (blog at handcoding.com)]

(In reply to comment #12)
> Checking in nsDrawingSurfaceWin.cpp;

So, -> FIXED?

Comment 14

[ArronM (:paper)]

(In reply to comment #13)
> So, -> FIXED?

Won't know until people try the latest nightly.  Fixes it for me, but I may not
have tested it as aggressively as others.

Comment 15

[Alex Kapranoff]

BTW, something like this happens in StaticGray mode on X Window. Look here
https://bugzilla.mozilla.org/show_bug.cgi?id=117310

The misbehaviour is exactly the same -- some parts of UI and pages are not
redrawn after loading many images (but only in StaticGray mode).

Comment 16

[ArronM (:paper)]

Status?  Does FF/Moz stop repainting in the latest nightly builds?

Comment 17

[Ere Maijala (slow)]

I haven't tried a build that showed the problem, but at least I couldn't
reproduce on WinXP SP 2 and the latest SeaMonkey nightly.

Comment 18

[Bernard Alleysson]

(In reply to comment #16)
Windows 2000 SP4, firefox trunk 20050310, Radeon 7500 64MB:

1) Loading a lot of jpeg (about 10 windows x 10 tabs x 20 images)
2) GDI object count at 2865 for firefox

Results: 
* firefox has some major problems with fonts (just try "about:cache" for instance)
fonts on web sites (most of them) are wrong, ie you get bold fonts, fonts with
the wrong sizes, ...
* other applications suffer from rendering problem (bitmaps, fonts), for
instance try IE

Remarks:
Closing a few windows seems to fix the rendering bug, the critical GDI object
count value seems to be 2500 for me

Comment 19

[ArronM (:paper)]

darn, so it's the fonts now.  I was eventually able to reproduce #18, although
it took a long long long time.  Once I got it, Win2k went into spasms of "not
enough virtual memeory" (even though I had 300M of real memory free) and other
apps had a problem drawing.  Everything was horribly slow.

I'm considering writing a failsafe routine that dumps ALL Mozilla DDBs when we
run out of resources.  Considering that running out of resources is a pretty
major matter for a computer, I think such a bold reaction is acceptable.  Plus,
it wouldn't take much overhead (compared to maintaining a handle count). At
least it'll be something until someone works magic on Bug 218861.

Comment 20

[Aaron Kaluszka]

Accidental reassign

Comment 21

[Bernard Alleysson]

(In reply to comment #19)
From my testing (Windows 2000), I can see that applications affect each other
with regard to GDI resources. You can't prevent an application from taking all
availables resources. Firefox can run out of GDI handles no matter the effort
you put into limiting its use of GDI resources. So I think that it's ok to use
DDB and fallback to DIB after all (I had to open more than 2000 images to defeat
it !).

I can see that GDI resources are released by IE when windows are closed but GDI
handles are not released by Firefox. They are retained by the memory cache
instead (I get the same as IE when the Firefox memory cache is disabled). So
clearing the inactive memory cache content (to release GDI handles) when the
creation of any DDB fails could be considered (I guess that disabling the memory
cache is not an option).

Comment 22

[Boris Zbarsky [:bzbarsky]]

One interesting thing may be to have a "gdi pressure" observer (like our
memory-pressure observer) mechanism.  If ddb creation fails, fire the
notification (which should probably flush our image and font caches), and retry...

Comment 23

[ArronM (:paper)]

Attachment: Simple GDI Failsafe

This patch is very similar to Bug 205893, Attachment 127547 [details] [diff] ("patch to fix
problem").  Since that patch was r- by stuart, I'm going to ask for trouble and
ask stuart to review this one.	The difference is that my patch does less. 
It's not a rotating cache, rather it's a simple failsafe that flushes all image
DDBs if we are unable to create a new DDB.  I consider this a crisis state, as
the OS is not able to even properly paint its low-resources warning message.  A
retry to optimize the image is not done for the same reason (the OS is in a
crisis state), however, new images will try to optimize.

As much as I like the idea of flushing the cache, I think keeping it simple, at
least for the 1.8 release, is best.  If we were to go the 'flushing the cache'
route, I'd rather just un-optimize when no open pages are referencing the
image, and re-optimize when it's pulled from memory cache and used again.  Even
that plan, though, would require the failsafe presented in this patch (for
those who open a crazy amount of images), or a similar failsafe mechanism.

Comment 24

[Jesse Ruderman]

Following the steps in comment 0 no longer makes Firefox stop repainting.  This
bug is still open because of related problems first mentioned in comment 18.

Comment 25

[HopeSeekr of xMule]

Using 20050312 I *still* get this bug, or maybe a new one.  It happens after x
refreshes per minute of pages.  I can't figure it out but it is fairly low, say
25 refreshes in a minute?  Attached is a screenshot (gmail.jpg).  It happens to
every page in every window and tab and refreshing does not solve the problem,
although you can view the blocked images individually fine.

Comment 26

[HopeSeekr of xMule]

Attachment: Picture of blocked images

Comment 27

[Bernard Alleysson]

(In reply to comment #24)
> This bug is still open because of related problems first mentioned in comment
> 18.

I can confirm that it's certainly not fixed for me (Windows 2000). After some
browsing, suddenly other applications stop repainting (WMP, iTunes, ...). Even
the Start menu won't work (it doesn't expand, clicking Start does nothing)! The
solution: close all firefox windows. Each time I take a look at the GDI object
count and it is between (1500 - 2500) for firefox.exe, even if all windows are
blank. I have 1GB RAM and the memory cache is set to 32768 KB.

Comment 28

[Bernard Alleysson]

setting blocking flags:
Once the memory cache is filled up with images, Firefox takes almost all GDI
resources and other applications stop repainting.

Comment 29

[Steve England [:stevee]]

Bernard, try reducing your memory cache size. Once the cache is full, firefox
should start releasing its oldest cached (GDI) objects so new objects can be
cached. If you set your cache too big, then I think you're more likely to hit
the OS limit of GDI objects, which can cause problems.

Comment 30

[Bernard Alleysson]

(In reply to comment #29)
> Bernard, try reducing your memory cache size.
I tried and it works for me, indeed. I even tried to disable the memory cache.
Unfortunately some sites don't load when the memory cache is disabled (!). The
lowest working value for me is 1KB.
 
> Once the cache is full, firefox
> should start releasing its oldest cached (GDI) objects so new objects can be
> cached.
Once the cache is full, old objects are replaced by new ones but once the memory
cache is full, it stays full, and GDI resources don't go down.

> If you set your cache too big, then I think you're more likely to hit
> the OS limit of GDI objects, which can cause problems.
The default for my setup (1GB RAM) is 31744 KB, and I've set it up to 32768 KB
because I've tried to change the size, and it's easier to remember.

Comment 31

[Bernard Alleysson]

Just a thought, is it possible (about:config) not to cache any images (only css
+ html)? Given the performance improvement of using DDB, maybe caching images in
the memory cache is not necessary anymore.

Comment 32

[Steve England [:stevee]]

> Once the cache is full, old objects are replaced by new ones but once the memory
> cache is full, it stays full, and GDI resources don't go down.
But that's expected behaviour right? Once the memory cache is full, as new items
are cached, old items fall out the cache, so the number of GDI objects and
memory used should stay around constant. Even when you close all but one tab,
the cache remains full, handles to GDI Objects still remain.

If you choose to clear your cache then you will notice that GDI Objects are
released. As expected.

Comment 33

[Boris Zbarsky [:bzbarsky]]

> Given the performance improvement of using DDB, maybe caching images in
> the memory cache is not necessary anymore.

DDB isn't exactly universally used across the platforms we support....

Comment 34

[Bernard Alleysson]

fwiw, the problem I've run into with memory cache size set to 0 is bug 184304

Comment 35

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Bug 290510 has a testcase, assuming this is the same issue.

Comment 36

[ArronM (:paper)]

Attachment: [WIP] nsGdiFailsafe

Work-In-Progress, but I thought I'd attach it as it stands since it's in a
working state.	I'd like to throw in a GetFreeSystemResources32 in
nsGdiFailsafe::IsOkayToOptimize() check for Win9x, and browse through the patch
once more before asking for a review.

This patch requires Attachment #181941 [details] [diff] of Bug 292051

Quick Summary:
- Use GDI resources until there are no more

- Set a total image DDB memory limit to 1/2 of what we are currently using
(memory limit probably dependent upon graphic card), and clear all GDI
resources asynchronously (flushing all takes too long time to do synchronously)


- When memory limit is reached, and an image wants to be optimized, flush
enough DDBs to make it happen.

- If the surface creation routine runs out of memory, flush image DDBs and try
again.	Surface is more important than images.


Via Bug 292051, DDBs will also be flushed based on time, making it less likely
we will ever reach the limit.

In case I didn't mention it, GDI resources are limited in number on
Win9x/ME/NT, AND limited in total memory size on ALL platforms.

Comment 37

[ArronM (:paper)]

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

Comment 38

[Christian :Biesinger (don't email me, ping me on IRC)]

why not make the voidarray a concrete member, rather than a pointer member?

Comment 39

[Daniel Veditz [:dveditz]]

Checked attachment 176528 [details] [diff] [review] into the aviary and mozilla1.7 branches.

Comment 40

[Asa Dotzler [:asa]]

transferring nomination to 1.8b4. If we don't get it there, we don't get it for 1.1

Comment 41

[Asa Dotzler [:asa]]

It would be great if we could get a fix for the next release but I don't think
we'll hold if one isn't available in time.  I'm certainly not trying to
discourage further work here.

Comment 42

[Stefan Ring]

For me the regression happens in the changes made from nsImageWin.cpp, revision
3.139 to 3.140. On Win2K/XP, this effectively cancels out my patch from #205893
which has been in use for nearly 18 months. I just noticed a few days ago when I
finally upgraded from Mozilla 1.7.7 to 1.7.11. Note that it wasn't a self
fulfilling prophecy - I noticed the problems FIRST and looked into bonsai
BECAUSE OF the problems.

Comment 43

[Boris Zbarsky [:bzbarsky]]

Right.  From bug 284716 comment 0:

  Apparently DIBs use less GDI resources.  However, DDBs are much
  much faster than DIBs. 

So it sounds like we're more likely to run out of GDI resources now that we're
using DDBs.  We're also not taking the pretty severe perf hit we were taking
before (order of > 400% on a real-life site in bug 277762, for example).

Comment 44

[Stefan Ring]

Ok so you are right, I forgot about the performance aspect. I didn't notice any
difference back then, however. Now I remember having seen one page using lots of
transparent images that was indeed a lot slower. But it seemed to be the
transparency - didn't notice performance issues with opaque images.

However, I'm sorry about my previous comment which may have been a bit biased
because I thought of my patch as a one size fits all permanent cure which it
doesn't seem to be. I still prefer slower, reliable painting to faster graphics
operations producing display errors, though.

Comment 45

[HyperHacker]

(In reply to comment #18)
> fonts on web sites (most of them) are wrong, ie you get bold fonts, fonts with
> the wrong sizes, ...
> * other applications suffer from rendering problem (bitmaps, fonts), for
> instance try IE
This happens any time Windows runs out of graphic resources, whether it's caused by Firefox or not. The most likely reason is that it doesn't have enough free resources to load the correct font, so it simply chooses a 'fall-back' font which is always loaded (usually Fixedsys). Similarly, I've seen cases where list boxes will be drawn at position 0,0 instead of their real position. Unfortunately I don't think much, if anything, can be done to prevent this; loading a lot of images uses up a lot of resources, and using up a lot of resources makes Windows act funny.

Also, it's important to note that number of images or number of GDI objects have very little relation to the problem; the real issue is the amount of resources these objects take up. If you load 10000 1x1 black-and-white images, you've allocated several thousand GDI objects, but only a few K of video memory. On the other hand, if you load a single 5000x5000 24-bit colour image, you've only allocated a few GDI objects, but nearly 100MB of video memory.

Comment 46

[Craig]

(In reply to comment #45)

> Unfortunately I don't think much, if anything, can be done to prevent this;
> loading a lot of images uses up a lot of resources, and using up a lot of
> resources makes Windows act funny.

When other browsers use 1/5 the GDI resources that Mozilla does, it is clear that Mozilla is being extremely badly behaved and needs to be fixed. Clearly something can be done, if everyone else except Mozilla is managing to do it better.

Comment 47

[Justin Kerk]

Fixed by bug 366548?

Comment 48

[Steve Chapel]

(In reply to comment #46)
> When other browsers use 1/5 the GDI resources that Mozilla does...

Can you provide steps to reproduce with a recent nightly builds of Firefox 3 (after October 3, 2007)?

Comment 49

[Steve Chapel]

Opening <http://a2pg.com/testcase/images.php?images=2000> causes Firefox 3 beta 1 to use more than 4000 GDI objects (temporarily). In comparison, opening the same page in IE 7 makes it use no more than 700 GDI objects at any time. Even with Firefox's use of many times more GDI objects, I can't reproduce any repainting problem, though.

Comment 50

[Wayne Mery (:wsmwk)]

Last patch is comment 39.
closing FIXED per comment 49 and WFM testing Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008021504