Bug 6219 – Perf regression: -[NSImage initWithData:] called repeatedly while moving the cursor over an image

• Status: VERIFIED
• Resolution: FIXED
• Component: WebKit Misc.

Each call to -[WebView elementAtPoint:] for a point inside an image entails a call to -[NSImage 
initWithData:] to fill the WebElementImageKey. This results in pretty high CPU usage when simply moving 
the mouse cursor across a big image. See the testcase for example.

I think this started with the changes to QPixmap when bug 5689 was fixed.

Created attachment 5250 [details]
Testcase

[Eric Seidel (no email)]

Great catch.

Created attachment 5395 [details]
Lame patch

With this patch, initWithData: will still be called once per image, which is
almost always a waste of time. The non-lame approach, I think, is to use an
NSImage subclass or a proxy that does lazy instantiation as the value of
WebElementImageKey in the element dictionary.

[Darin Adler]

Comment on attachment 5395 [details]
Lame patch

Better to just change the pixmap() function to return a const QPixmap& -- that
would do the trick.

Created attachment 5406 [details]
Change pixmap() to return a const QPixmap&

I really ought to learn that "programming" thing.

Comment on attachment 5406 [details]
Change pixmap() to return a const QPixmap&

Maybe there should be a FIXME in WebHTMLView where it creates the NSImage
saying that it would be better to delay the actual NSImage instantiation? It
seems that in an overwhelming majority of cases, the WebElementImageKey isn't
used.

Another option, instead of using a proxy or an NSImage subclass, is to use an
NSMutableDictionary subclass for the result of -[WebHTMLView elementAtPoint:]
that knows how to generate the value of the WebElementImageKey from the
WebCoreElementImageRendererKey.

Okay, the last suggestion was really stupid.

[Darin Adler]

Comment on attachment 5406 [details]
Change pixmap() to return a const QPixmap&

Looks fine, r=me.

Good suggestions about implementing our own Objective-C dictionary so we can
compute this lazily.

[Darin Adler]

Mitz's last suggestion was *not* stupid. We can't create an NSMutableDictionary subclass, but we can 
make our own dictionary class that's part of the NSDictionary "class cluster". And that is indeed the best 
way to fix this "for real".

And yes, I think the the FIXME is a good idea.

[Maciej Stachowiak]

Agreed, in fact some APIs in AppKit return custom NSDictionary subclasses.

[Darin Adler]

To be even more specific, we should use an NSDictionary subclass. It only has to implement the three 
methods count, objectForKey:, and keyEnumerator. And the keyEnumerator will be an NSEnumerator that 
only has to implement nextObject.

We should make sure there's a method on DOMNode or DOMElement for each of the properties and then 
we can just map the dictionary key to a selector and call it on the DOMElement.

See bug 7450