536092 - Micro optimizations for event handling

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

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

Attachment: WIP

There are several small things which show up in event handling profiles.
The patch fixes few: some reduced QIing, less string handling, and not so many
getService calls.

Posted to TryServer

Comment 1

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

Comment on attachment 418576 [details] [diff] [review]
WIP

Most of this has landed elsewhere.
A new patch with different optimizations and cleanups coming.

Comment 2

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

Attachment: patch

The change to nsGenericElement::doPreHandleEvent reduces the calls to
ownerDoc->BindingManager()->GetInsertionParent, since elements don't
usually have insertion parents.

The change to dom/base/nsDOMClassInfo.cpp removes one extra Addref/Release for
each time a listener is called.

Rest of the patch reduces some QIing when accessing .target, .originalTarget or .currentTarget.

Depending on the case this speeds up event dispatching 2-5%.
(More if there are lots of event listeners.)

Comment 3

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

http://hg.mozilla.org/mozilla-central/rev/b6d7952c5c5c

Comment 4

[Jeff Muizelaar [:jrmuizel]]

It looks like the comment in this hunk is now inaccurate:

   // Event targets, needed by DOM Events
   // Using nsISupports, not nsIDOMEventTarget because in some cases
   // nsIDOMEventTarget is implemented as a tearoff.
-  nsCOMPtr<nsISupports> target;
-  nsCOMPtr<nsISupports> currentTarget;
-  nsCOMPtr<nsISupports> originalTarget;
+  nsCOMPtr<nsPIDOMEventTarget> target;
+  nsCOMPtr<nsPIDOMEventTarget> currentTarget;
+  nsCOMPtr<nsPIDOMEventTarget> originalTarget;

Comment 5

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

Oops, I'll update that

Comment 6

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

And note, since SetTarget QIs now to nsPIDOMEventTarget, things should be safe.
nsEventDispatcher uses nsEvent.*target for its own purposes.

Comment 7

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

Additional fix to remove the bogus comment and fix a real issue in the
patch. Fortunately a testcase did catch that one.
http://hg.mozilla.org/mozilla-central/rev/dd83b3b5418e