147938 - Don't Invalidate nsImageFrame area if it's hidden

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, enhancement)

Description

[ArronM (:paper)]

When nsImageFrame::FrameChanged is called, and the image frame is visibility:
hidden, we don't need to invalidate.  This saves a lot of calls and reduces CPU
usage.

Test Case:
http://www.animecity.nu/mozilla/IMGTest/hiddenStorms.html

There are 7 hidden animated gifs in the test case.  Without this patch, CPU is
about 25% on my system (PIII-933 on Win2k).  With the patch, CPU is 0%

Comment 1

[ArronM (:paper)]

Attachment: Patch to check if invisible

Comment 2

[ArronM (:paper)]

I wouldn't mind someone confirming CPU usage on
http://www.animecity.nu/mozilla/IMGTest/hiddenStorms.html
Include CPU Speed, CPU Usage, OS, Build.

Comment 3

[Terri Preston]

I do see about 25% CPU usage on win XP trunk build 2002052908

Comment 4

[Sam Emrick]

os/2 mozilla rc2 plus privates, 700mhz, 
w/o fix: idle: 56.62%  busy: 41.99%  intr:  1.39%
w/ fix: idle: 78.91%  busy: 20.51%  intr:  0.58%

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 85476 [details] [diff] [review]
Patch to check if invisible

It might be nice to solve this problem more generally.	However, this seems OK
if you think it will help more than it hurts.  (How often are images hidden? 
How long does it take to check the style data?	It probably does...)

>+  const nsStyleVisibility* vis = 
>+        (const nsStyleVisibility*)((nsIStyleContext*)mStyleContext)->GetStyleData(eStyleStruct_Visibility);

Could you use

const nsStyleVisibility* vis;
::GetStyleData(this, &vis);

instead?  You certainly don't need that second cast.

Comment 6

[ArronM (:paper)]

Attachment: Patch to check if invisible

David Baron:  Done.

The whole style casting thing is also done in about 6 other places in
nsImageFrame, and that's why I had it like that originally.  Is there enough
performace inprovements using your method to create another bug to convert
those 6 over?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 88265 [details] [diff] [review]
Patch to check if invisible

r=dbaron.  It's not a performance issue -- it's a typesafety one.

Comment 8

[Chris Waterson]

Comment on attachment 88265 [details] [diff] [review]
Patch to check if invisible

sr=waterson, nice!

Comment 9

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

fixed