Update Document's event listener type bitfield when adopting a Node
Created attachment 197189 [details] Patch
Comment on attachment 197189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review Looks fine. > Source/WebCore/dom/Node.cpp:2158 > + Vector<AtomicString> types = listenerMap.eventTypes(); If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory?
(In reply to comment #2) > (From update of attachment 197189 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review > > Looks fine. > > > Source/WebCore/dom/Node.cpp:2158 > > + Vector<AtomicString> types = listenerMap.eventTypes(); > > If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? I poked around for a bit because I had the same thoughts. The solution would be to expose some sort of iterator interface from EventListenerMap to let one iterate over the types without exposing the implementation (as a vector of pairs). However, I think it's likely not worth doing just for this use, since the normal case is no EventTargetData, or just a few listeners. So the vector isn't likely to be large, even in the rare case when it's allocated.
Comment on attachment 197189 [details] Patch Clearing flags on attachment: 197189 Committed r148072: <http://trac.webkit.org/changeset/148072>
All reviewed patches have been landed. Closing bug.
Comment on attachment 197189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review >>> Source/WebCore/dom/Node.cpp:2158 >>> + Vector<AtomicString> types = listenerMap.eventTypes(); >> >> If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? > > I poked around for a bit because I had the same thoughts. The solution would be to expose some sort of iterator interface from EventListenerMap to let one iterate over the types without exposing the implementation (as a vector of pairs). > > However, I think it's likely not worth doing just for this use, since the normal case is no EventTargetData, or just a few listeners. So the vector isn't likely to be large, even in the rare case when it's allocated. If the common case is a small vector, then an example of an efficient data structure that does not require memory allocation could be Vector<AtomicString, 4>.
(In reply to comment #6) > (From update of attachment 197189 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197189&action=review > > >>> Source/WebCore/dom/Node.cpp:2158 > >>> + Vector<AtomicString> types = listenerMap.eventTypes(); > >> > >> If I had more time I’d study the implementation of EventListenerMap::eventTypes, but is it really necessary to copy a vector of atomic strings? Is there any more efficient way to get at the event type strings, perhaps without allocating memory? > > > > I poked around for a bit because I had the same thoughts. The solution would be to expose some sort of iterator interface from EventListenerMap to let one iterate over the types without exposing the implementation (as a vector of pairs). > > > > However, I think it's likely not worth doing just for this use, since the normal case is no EventTargetData, or just a few listeners. So the vector isn't likely to be large, even in the rare case when it's allocated. > > If the common case is a small vector, then an example of an efficient data structure that does not require memory allocation could be Vector<AtomicString, 4>. Seems like a reasonable follow-up, not sure if I'll have time to put it together myself. 2 seems like it might be the right size, to match the sizing in EventListenerMap::m_entries. C++11 would make such changes easier, as one could use "auto" instead of having to update callers of eventTypes().