Closed Bug 536092 Opened 15 years ago Closed 15 years ago

Micro optimizations for event handling

Categories

(Core :: DOM: Events, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch WIP (obsolete) — Splinter Review
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 on attachment 418576 [details] [diff] [review] WIP Most of this has landed elsewhere. A new patch with different optimizations and cleanups coming.
Attachment #418576 - Attachment is obsolete: true
Attached patch patchSplinter Review
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.)
Attachment #435919 - Flags: review?
Attachment #435919 - Flags: review? → review?(jst)
Attachment #435919 - Flags: review?(jst) → review+
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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;
Oops, I'll update that
And note, since SetTarget QIs now to nsPIDOMEventTarget, things should be safe. nsEventDispatcher uses nsEvent.*target for its own purposes.
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: