Bug 77982

Summary: EventListenerMap: Use Vector instead of HashMap as backend.
Product: WebKit Reporter: Andreas Kling <kling>
Component: UI EventsAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, dglazkov, ggaren, koivisto, laszlo.gombos, rniwa, tonikitoo, webkit.review.bot
Priority: P2 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+

Andreas Kling
Reported 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.
Attachments
30-day trial version for EWS (12.72 KB, patch)
2012-09-07 09:45 PDT, Andreas Kling 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Proposed patch (14.52 KB, patch)
2012-09-09 07:53 PDT, Andreas Kling 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-02-07 08:29:35 PST
Maybe we can use Vector and do the binary search for lookups?
Andreas Kling
Comment 2 2012-09-07 09:45:35 PDT
Created attachment 162792 [details] 30-day trial version for EWS
WebKit Review Bot
Comment 3 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
Andreas Kling
Comment 4 2012-09-09 07:53:37 PDT
Created attachment 162996 [details] Proposed patch
Geoffrey Garen
Comment 5 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?
Geoffrey Garen
Comment 6 2012-09-09 14:08:06 PDT
Comment on attachment 162996 [details] Proposed patch r=me
Andreas Kling
Comment 7 2012-09-09 15:26:56 PDT
Committed r128002: <http://trac.webkit.org/changeset/128002>
Note You need to log in before you can comment on or make changes to this bug.