Consider removing sEventListenerManagersHash and storing the ELM pointer inside nsSlots

RESOLVED INVALID

Status

()

Core
DOM: Events
P2
normal
RESOLVED INVALID
11 months ago
9 months ago

People

(Reporter: Ehsan, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

11 months ago
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.

Updated

11 months ago
Priority: -- → P2

Comment 1

11 months ago
So this is 10ms out of 32s?
(Reporter)

Comment 2

9 months ago
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.
Status: NEW → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.