1376580 - Consider removing sEventListenerManagersHash and storing the ELM pointer inside nsSlots

Status: RESOLVED INVALID

(Core :: DOM: Events, enhancement, P2)

Description

[(no longer active)]

The hashtable lookups on sEventListenerManagerHash for example under nsContentUtils::GetExistingListenerManagerForNode() show up under Speedometer profiles.  For example see this partial Speedometer run profile: http://bit.ly/2tijPJM

It would be interesting to experiment with replacing this mechanism for storing the ELM pointer for a node with something else.  The idea that Olli and I discussed was storing a pointer to this inside nsSlots for the node.

The main concern about this is memory usage, since currently slots can be quite large, so the memory usage aspects of this patch needs to be carefully examined.  We should probably:
  * Run Talos on the patch and ensure memory usage tests do not regress.
  * Run AWSY on the patch to see what impact it has on memory usage.

The other concern is that this hashtable is currently iterated during the forgetSkippable phase in order to gather a list of all existing ELMs, and we lose an easy way to do this by moving it to the slots.  Olli's idea on how to fix this was to investigate the size of the ELM data structure to see how well it currently fits in the jemalloc buckets, and if there is room storing pointers to create a linked list of ELMs as they are stored inside the slots, and use this linked list for traversal.

Comment 1

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

So this is 10ms out of 32s?

Comment 2

[(no longer active)]

Sorry I didn't see your question.  I did some reprofiling today since I didn't remember any more, and I don't see this function any more, so let's close this bug.