756383 - make nsHTMLAnchorElement::IsHTMLFocusable faster

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

in example of bug 732872 it takes up to 11% because of URI computation. Cached URI usage should help.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 625024 [details] [diff] [review]
patch

I'm not familiar with Link. Jonas and Boris should know it better, 
based on blame.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 625024 [details] [diff] [review]
patch

I won't have time to get to this anytime soon. Checking with bz.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 625024 [details] [diff] [review]
patch

Wouldn't it make more sense to move the cache check into IsLink()?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to Boris Zbarsky (:bz) from comment #3)
> Wouldn't it make more sense to move the cache check into IsLink()?

IsLink is a virtual method and takes an argument we don't need what makes us a little bit slower than we could be. Good example how minor stuffs can make a difference I've got is bug 756381 where we lost 3% perf on nsCOMPtr things.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to Boris Zbarsky (:bz) from comment #3)
> Comment on attachment 625024 [details] [diff] [review]
> patch
> 
> Wouldn't it make more sense to move the cache check into IsLink()?

in case if I misread you: I can fix IsLink() while I'm here but I would avoid to use it for IsHTMLFocusable implementation.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 625024 [details] [diff] [review]
patch

OK, r=me.  I'm still planning to rip out all this stuff anyway...

Comment 7

[alexander :surkov (:asurkov)]

(In reply to Boris Zbarsky (:bz) from comment #6)

> OK, r=me.  I'm still planning to rip out all this stuff anyway...

ok, cool since these stuffs are crazy a little bit (I guess because old and new code share the same room).

Then I'm leaving IsLink untouched for now.

Comment 8

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41d7c9506e27

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/41d7c9506e27