Last Comment Bug 756383 - make nsHTMLAnchorElement::IsHTMLFocusable faster
: make nsHTMLAnchorElement::IsHTMLFocusable faster
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks: 732872
  Show dependency treegraph
 
Reported: 2012-05-18 00:44 PDT by alexander :surkov
Modified: 2012-05-23 04:50 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.50 KB, patch)
2012-05-18 00:44 PDT, alexander :surkov
bzbarsky: review+
Details | Diff | Review

Description alexander :surkov 2012-05-18 00:44:40 PDT
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.
Comment 1 Olli Pettay [:smaug] 2012-05-18 02:07:32 PDT
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) 2012-05-18 15:37:01 PDT
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 [:bz] 2012-05-21 23:43:42 PDT
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 2012-05-22 02:41:13 PDT
(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 2012-05-22 02:51:41 PDT
(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 [:bz] 2012-05-22 06:46:39 PDT
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 2012-05-22 06:54:55 PDT
(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 9 Ed Morley [:emorley] 2012-05-23 04:50:11 PDT
https://hg.mozilla.org/mozilla-central/rev/41d7c9506e27

Note You need to log in before you can comment on or make changes to this bug.