Closed Bug 756383 Opened 9 years ago Closed 9 years ago

make nsHTMLAnchorElement::IsHTMLFocusable faster

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

Attached patch patchSplinter Review
in example of bug 732872 it takes up to 11% because of URI computation. Cached URI usage should help.
Attachment #625024 - Flags: review?(bugs)
Comment on attachment 625024 [details] [diff] [review]
patch

I'm not familiar with Link. Jonas and Boris should know it better, 
based on blame.
Attachment #625024 - Flags: review?(bugs) → review?(jonas)
Comment on attachment 625024 [details] [diff] [review]
patch

I won't have time to get to this anytime soon. Checking with bz.
Attachment #625024 - Flags: review?(jonas) → review?(bzbarsky)
No longer blocks: 732872
Blocks: 732872
Comment on attachment 625024 [details] [diff] [review]
patch

Wouldn't it make more sense to move the cache check into IsLink()?
Attachment #625024 - Flags: review?(bzbarsky) → review-
(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.
(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 on attachment 625024 [details] [diff] [review]
patch

OK, r=me.  I'm still planning to rip out all this stuff anyway...
Attachment #625024 - Flags: review- → review+
(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.
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/41d7c9506e27
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.