[webkit-dev] JS bindings: Adding EventTarget to the prototype chain

Maciej Stachowiak mjs at apple.com
Thu Jun 9 15:53:12 PDT 2011


I don't have a personal opinion on which way is technically better myself. But I think the key is getting our code aligned with where standards are going, wether by changing he code or the standards. EventTarget in the prototype chain seems neither especially awesome nor especially terrible to me, it is not really clear to me why editor's drafts of the listed specs decided to change.

Cheers,
Maciej


On Jun 9, 2011, at 1:14 PM, Sam Weinig wrote:

> I don't think we should do this.  EventTarget is really just an abstract interface, and changing its implementation globally is of limited utility.
> 
> -Sam
> 
> On Jun 8, 2011, at 5:54 PM, Dominic Cooney wrote:
> 
>> [If you don't care about JSC or V8 bindings you can safely ignore
>> this.]
>> 
>> TL;DR I want to change the JavaScript bindings to put EventTarget on
>> the prototype chain so it is easier to work with event targets from
>> JavaScript. What do you think?
>> 
>> Here is the prototype chain for a button today:
>> 
>> HTMLButtonElement-HTMLElement-Element-Node-Object
>> (add/removeEventListener and dispatchEvent are on Node.)
>> 
>> Here is how I think we should make it look:
>> 
>> HTMLButtonElement-HTMLElement-Element-Node-EventTarget-Object
>> (addEventListener etc. are on EventTarget.)
>> 
>> Here’s why I think we should do this:
>> 
>> - Specs are moving towards specifying EventTarget as living on the
>> prototype chain. DOM Core*, Notifications, Indexed DB, SVG and XHR
>> already do it this way. (* Editor’s draft.)
>> 
>> - Frameworks that want to hook add/removeEventListener will be able to
>> do it in one place: on EventTarget.prototype. In WebKit today they
>> have to hook the prototypes of window, Node, XMLHttpRequest, etc.
>> individually (Because WebKit implements EventTarget as a mixin
>> everywhere, there are 20+ different kinds of event targets to hook if
>> you want to be comprehensive.) If we make this change, it gets easier
>> to tell if a given object is an EventTarget; just do
>> EventTarget.prototype.isPrototypeOf(something).
>> 
>> - It will modestly simplify WebKit’s IDLs and bindings. Instead of
>> declaring addEventListener in two dozen places in IDLs, it will be in
>> one place; instead of calling visitJSEventListeners in dozens of
>> places for JSC GC, it will be called in one place; instead of assuming
>> that EventTarget parameters are all Node* under the covers for V8 code
>> generation, we can treat EventTargets uniformly; instead of
>> redundantly specifying EventTarget on Document and Node inheritance
>> will do what you want; etc.
>> 
>> Will doing this break the web? I don’t think so:
>> 
>> Anyone who calls or hooks addEventListener, etc. will continue to
>> work, just their foo.addEventListener might be resolved at one level
>> higher up the prototype chain than it is today. To really observe the
>> different placement of addEventListener, etc. minutely you need to
>> access __proto__ and hasOwnProperty. Other browsers are already differ
>> from WebKit in this regard, too: For example, Firefox reports
>> addEventListener is an own property on *every* step in the prototype
>> chain of DOM objects (until Object.)
>> 
>> Scripts that squirrel up the prototype chain themselves will see one
>> more link (EventTarget’s) but it is towards the top of the chain, past
>> most prototypes they care about (every prototype except Object.)
>> 
>> I tried changing half of the EventTargets in WebKit to put EventTarget
>> in the prototype chain, including Node and XHR (but not window) and
>> used it to surf the web for a few days. I didn’t notice anything break
>> :)
>> 
>> There is also the possibility that this could hurt performance,
>> because accessing addEventListener, etc. will have to search more
>> prototypes (usually just one more.) Accessing the properties of Object
>> on an event target via the prototype chain will have to squirrel
>> through one more prototype (EventTarget’s.) So I prototyped this
>> change in the JSC bindings and put EventTarget in the prototype chain
>> of a number of event targets in WebKit, including Node. Here are the
>> results for Dromaeo’s dom and jslib tests:
>> 
>> <http://dromaeo.com/?id=141811,141813>
>> 
>> (141811 on the left is the status quo.)
>> 
>> I expect the Prototype and jQuery events benchmarks are of particular
>> interest, and the result looks particularly bad for Prototype (~3%
>> slowdown). So I reran <http://dromaeo.com/?event> half a dozen times,
>> and couldn’t produce the poor result for Prototype; on average the
>> prototype was 1.0% slower for Prototype and 0.5% slower for jQuery. I
>> think Dromaeo is too noisy for measuring something as fine as this.
>> 
>> So I wrote three microbenchmarks:
>> 
>> 1. Add/remove click listener on a button.
>> 
>> 2. Add/remove progress listener on an XHR.
>> 
>> 3. Test property presence with 'in':
>> 
>> if ('dispatchEvent' in target)
>>  n++;
>> // return n outside of loop
>> 
>> where target is an XMLHttpRequest and n is a local var n = 0.
>> 
>> Here are the results. A brief note on methodology: release build
>> running in Mac Safari, JSC, averaging 500 samples with 1,000,000
>> iterations of the inner loop per sample.
>> 
>> 1. Add/remove on button
>> Before (ms): min=409, median=434, mean=434.4, max=472
>> After (ms): min=410, median=453.5, mean=452.4, max=497 (mean is 1.04x)
>> 
>> 2. Add/remove on XHR
>> Before (ms): min=286, median=298, mean=298.6, max=315
>> After (ms): min=287, median=300.5, mean=300.7, max=320 (mean is 1.01x)
>> 
>> 3. 'dispatchEvent' in XHR
>> Before (ms): min=85, median=88, mean=87.7, max=91
>> After (ms): min=89, median=91, mean=91.0, max=95 (mean is 1.04x)
>> 
>> So this shows that, yes, this is not free, but it is low-single
>> digits. Since adding and removing event listeners is a relatively
>> infrequent operation, I think this is OK. I want to emphasize that the
>> change I’m proposing has no impact on native event *dispatch*, which
>> is obviously a lot more performance-sensitive than adding and removing
>> event listeners.
>> 
>> There is the question of maintenance: Making EventTarget an
>> Objective-C class is a public API change, so some of the #ifdef-ing
>> for Objective-C in IDLs will remain to avoid that. This change doesn’t
>> need to touch EventTarget on the C++ side; the existing design works
>> well with this new shape of binding.
>> 
>> As I mentioned above, there are a number of little clean-ups and
>> simplifications this makes possible. I can tackle this incrementally,
>> although unfortunately most of the clean-up is contingent on the old
>> way of mixing in EventTarget being completely replaced.
>> 
>> What do you think?
>> 
>> Dominic
>> 
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
> 
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110609/9f424422/attachment.html>


More information about the webkit-dev mailing list