Bug 77982

Summary:

EventListenerMap: Use Vector instead of HashMap as backend.

Product:

WebKit

Reporter:

Andreas Kling <kling>

Component:

UI Events

Assignee:

Andreas Kling <kling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, dglazkov, ggaren, koivisto, laszlo.gombos, rniwa, tonikitoo, webkit.review.bot

Priority:

Keywords:

Performance

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
30-day trial version for EWS	webkit.review.bot: commit-queue-
Proposed patch	ggaren: review+

Description Andreas Kling 2012-02-07 06:33:05 PST

As noted by Ryosuke in bug 77906, our code review pages are putting quite some strain on WebKit.

For the specific example <https://bugs.webkit.org/attachment.cgi?id=125284&action=review> we are allocating ~40MB below EventTarget::addEventListener().

We should experiment with making EventListenerMap use a Vector rather than a HashMap to store the listener vectors. The hash map solution may very well be overkill, it's quite unusual to have listeners for more than 1 or 2 event types on a given Node.

A quick hack-up yields a ~26MB reduction in memory usage without making EventListenerMap show up in profiles on our PerformanceTests/. I'll sleep on it, but I have a good feeling about this.

Comment 1 Ryosuke Niwa 2012-02-07 08:29:35 PST

Maybe we can use Vector and do the binary search for lookups?

Comment 2 Andreas Kling 2012-09-07 09:45:35 PDT

Created attachment 162792 [details]
30-day trial version for EWS

Comment 3 WebKit Review Bot 2012-09-08 02:16:49 PDT

Comment on attachment 162792 [details]
30-day trial version for EWS

Attachment 162792 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13793242

New failing tests:
inspector/console/command-line-api-getEventListeners.html

Comment 4 Andreas Kling 2012-09-09 07:53:37 PDT

Created attachment 162996 [details]
Proposed patch

Comment 5 Geoffrey Garen 2012-09-09 14:07:14 PDT

The memory savings here is awesome, so I'm going to say r+.

But are we missing the larger lesson here? Why are sparse hash tables so huge? Could we save even more memory by fixing that as well?

Comment 6 Geoffrey Garen 2012-09-09 14:08:06 PDT

Comment on attachment 162996 [details]
Proposed patch

r=me

Comment 7 Andreas Kling 2012-09-09 15:26:56 PDT

Committed r128002: <http://trac.webkit.org/changeset/128002>