[FIXr]Less QIing in nsXBLWindowHandler::WalkHandlersInternal

RESOLVED FIXED in mozilla1.8alpha1

Status

()

P2
normal
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
mozilla1.8alpha1
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

15 years ago
I see the DOM event QI showing up at several percent of profiles of user
interaction (like keyboard scrolling).  Simply hoisting it out of the loop makes
things a bit better...  The elt change is also to avoid a possibly-unnecessary QI.
(Assignee)

Updated

15 years ago
Priority: -- → P2
Summary: Less QIing in nsXBLWindowHandler::WalkHandlersInternal → [FIX]Less QIing in nsXBLWindowHandler::WalkHandlersInternal
Target Milestone: --- → mozilla1.8alpha
(Assignee)

Updated

15 years ago
Attachment #143894 - Flags: superreview?(peterv)
Attachment #143894 - Flags: review?(bugmail)
Comment on attachment 143894 [details] [diff] [review]
Like so

r=me

Though it seems like if this is performance critical code there is a lot more
that can be done.
Attachment #143894 - Flags: review?(bugmail) → review+
(Assignee)

Comment 3

15 years ago
I don't quite understand the setup here well enough to know what assumptions I
can make (but at a guess, this code is all semi-bogus, like most of XBL).  This
patch was just an easy win.
Comment on attachment 143894 [details] [diff] [review]
Like so

Yeah, this isn't the first time we've solved silly code like this in XBL :-(.
Attachment #143894 - Flags: superreview?(peterv) → superreview+
(Assignee)

Updated

15 years ago
Summary: [FIX]Less QIing in nsXBLWindowHandler::WalkHandlersInternal → [FIXr]Less QIing in nsXBLWindowHandler::WalkHandlersInternal

Comment 5

15 years ago
(In reply to comment #2)
>Though it seems like if this is performance critical code there is a lot more
>that can be done.
Such as
if (commandElt)
  commandElt->GetAttribute(NS_LITERAL_STRING("disabled"), disabled);
else
  elt->GetAttr(kNameSpaceID_None, nsHTMLAtoms::disabled, disabled);
perhaps?
(Assignee)

Comment 6

15 years ago
hmmm.. maybe.   I'm not sure how big the savings are there...

In any case, this is checked in for 1.8a.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.