make nsHTMLAnchorElement::IsHTMLFocusable faster

RESOLVED FIXED in mozilla15

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: surkov)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla15
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 625024 [details] [diff] [review]
patch

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 1

5 years ago
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
(Assignee)

Updated

5 years ago
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-
(Assignee)

Comment 4

5 years ago
(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.
(Assignee)

Comment 5

5 years ago
(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+
(Assignee)

Comment 7

5 years ago
(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.
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/41d7c9506e27
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla15

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/41d7c9506e27
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.