Firing run loop on mousemove unnecessary?

[asakusuma]

Looks like a new run loop is kicked off every time the mouse moves.

Is this really necessary? @stefanpenner suggested that we might instead have components opt-in to needing this, instead of assuming we need a run loop on mouse movement.

[stefanpenner]

It's only happening just in-case someone has a mouse related handler on one of the rendered components. If instead components informed the event dispatcher the events they need on render and no longer need on un render. We could remove this, instead only listening to exactly what is required.

The mouse/touch events introduce an unfortunate cost, especially when they are not frequently used.

[asakusuma]

Is there a way to infer this from the component, or would the component need to manually declare what is needs?

If the latter, we'd need to choose the set of events that are listened for by default, and the set of events you need to manually opt in to.

[stefanpenner]

Is there a way to infer this from the component

yes, https://github.com/emberjs/ember.js/blob/master/packages/ember-views/lib/views/component.js#L223 is how one declares interest in an event from a component.

[asakusuma]

So then we could just ref count this on component creation and the decrement on destruction, and then add and remove this listener as needed?

[chadhietala]

I think this gets tricky with improved actions, because you can declare event handlers in the template.

<input onmouseover={{action 'reasonable/unreasonable'}} />

You completely loose the ability to introspect the class.

[rwjblue]

@chadhietala - In that scenario, you are not actually using the EventDispatcher (or Ember event bubbling at all), you are registering a custom onmouseover handler.

[chadhietala]

@rwjblue So it doesn't cause a runloop? Is this the same for the old on= syntax as well?

[rwjblue]

@chadhietala - No, {{action "foo" on="mouseover"}} would definitely go through EventDispatcher, and would have the issue you are cautioning against if we didn't take it into account.

[stefanpenner]

Yup it causes a run loop, but it's one that you want. Since you are absolutely invoking ember code. This is much better then all the preemptive handlers that run for. All mouse events regardless of if a handler is present.

An interim step is actually to skip the run loop for events that don't find a handler.

[stefanpenner]

https://github.com/emberjs/ember.js/blob/master/packages/ember-views/lib/system/event_dispatcher.js#L234-L242 could likely detect if the given view has such a handler, and only invoke the run-loop if it is about to enter user code.

To handle bubbling correctly, it may need to be slightly more creative.

This approach I believe will be the easiest, no ref counting needed. Sure we sill need to attach event listeners to everything, but we only bother invoking a run-loop if we truely are about to invoke code that requires it.

I suspect this will cure the original concern which prompted this issue.

[asakusuma]

So to make sure I understand this, the idea is to:

• Check the immediate view to see if actually has a handler for the given event
• If handler present, proceed
• If not, repeat on parent, if no parent, no-op

This sounds like it will address my issue. However, what is more expensive, traversing the view hierarchy looking for handlers, or executing a no-op run loop?

[stefanpenner]

We traverse it anyways

[runspired]

The mouse/touch events introduce an unfortunate cost, especially when they are not frequently used.

jQuery fast paths event handling for many events, I think Ember should likely look into doing so too, in addition to any inquiry into using useCapture for events. Therese a huge perf win being able to delegate directly instead of bubbling, and the current implementation not only needs to bubble, but needs to run multiple checks at each view/component layer for handlers.

I'd put the overall cost on this as an extra .5-4ms / touchmove or mousemove event that's being used to manipulate position on screen (much of which depends on how many layers it needs to bubble through etc.). Not horrendous, but definitely expensive when lots of such events are involved.

(Edit: that cost I peg above is a rough eyeball of the difference between using touchmove through Ember and using it as a direct handler on the component)

[stefanpenner]

I believe we are limited by our ie9 support and a true bubbling phase. Maybe not?

Would love for someone to dive into this area an propose some improvements. Enumerating cost/benefit would also be handy

[runspired]

IE9 allows for useCapture, IE8 support doesn't.

[runspired]

I've also got the resources/knowledge to do a decent cost/benefit run down, I'll see what I can whip up.

[stefanpenner]

Good thing we dropped ie8 for 2.0

[runspired]

Yep :)

[stefanpenner]

Anyone interesting in championing this?

[asakusuma]

@runspired are you still looking into doing a cost/benefit run down?

[runspired]

Yes.

[asakusuma]

@runspired how would fastpathing component events in Ember work? Would there be a step on initiation where the tree was walked to identify any relevant listeners?

[runspired]

More likely events should be moved back into an events hash, I'm not sure why they were moved out of one before. Once there, they can be auto registered on initialization, but that might require ember having knowledge of a component tree. There's trade offs there for sure. The quickest gain is useCapture.

[stefanpenner]

What does the events hash look like I'm es6 classes?

@runspired I think most of core would love to see us switch to our own capture based delegation. But time is thing, are you interested to pushing forward an rfc?

[runspired]

@stefanpenner I'm absolutely in favor of pushing forward an RFC, although I might (humbly) suggest pushing forward two RFCs.

The first, to use useCapture (or better), and possibly to go back to an events hash on the component. It bugs me that event callbacks are intermingled with other functions and properties on controllers and components, and it'd be much simpler (I think) to know that something is an event handler if it's within the events hash. This would also possibly allow Ember to detect which events are required and ensure only those events are listened for.

The second, to convert to using Pointer events instead of mouse/touch events. This would make Ember one of the first frameworks to truly play well immediately on mobile out of the box.

[runspired]

As for the question on what this hash would look like with ES6 classes, I'm not sure I follow. Perhaps I don't know enough about the limitations/implementation of ES6 classes.

[asakusuma]

Pardon my ignorance, but two questions:

1. What is the performance advantage of using useCapture? My understanding is that useCapture triggers the event as the event propagates down from the root DOM node instead of when bubbling on the way up.
2. What was the original reasoning behind moving away from an events hash?

+1 for the Pointer event, though it sounds like that might deserve it's own ticket.

[runspired]

For info on PEP: https://github.com/jquery/PEP

[runspired]

@asakusuma

1. yes. The perf advantage with this is that the event would be captured by Ember immediately (instead of there being a bubbling phase prior to reaching Ember), which is ideal for Ember's eventing.

2. I believe actions were originally within events: {}, so there was a confusion layer there that needed to be done away with. I think we should basically encourage use of someting like eventManager http://emberjs.com/api/classes/Ember.View.html#toc_event-managers

see: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener

[stefanpenner]

1. What is the performance advantage of using useCapture? My understanding is that useCapture triggers the event as the event propagates down from the root DOM node instead of when bubbling on the way up.

My motivation isn't as a performance advantage, but rather to enable more sanity when dealing with complex event scenarios. It may also have a positive perf side-affect.

In addition, to today we actually can incur several re-entrant scenarios, causing perf/bugs because we blindly allow bubbling and delegation.

I'm trying to finish up some tests for the memory leak @raytiley i'll try to post in more detail here later.

[runspired]

I do think a sane first step might be to remove the use of the run loop until there's actual user code to execute (vs each bubble being a run loop call).

[stefanpenner]

I do think a sane first step might be to remove the use of the run loop until there's actual user code to execute (vs each bubble being a run loop call).

this is already the case: https://github.com/emberjs/ember.js/blob/master/packages/ember-views/lib/system/event_dispatcher.js#L228-L230`

This is partially already the case. But the bubble phase run.join should be moved into view.handleEvent likely https://github.com/emberjs/ember.js/blob/master/packages/ember-views/lib/system/event_dispatcher.js#L241

[runspired]

Wouldn't run.join in dispatch event create a new loop if there weren't one?

[stefanpenner]

If we move this to view.handeEvent, we run the risk (if naively) implemented in spewing sequently loops for each handler reached via bubbling.

We likely need to add

if (view.hasOrAncestorHas(eventName)) {
  Ember.run.join(... bubble ...);
} else {
  // noop
}

[runspired]

Yeah the _bubbleEvent code is what I was referring to.

[runspired]

I did some perf testing on useCapture using a mechanism that would allow us to maintain ember's current eventing mechanics for the moment without changing any expected behavior: http://jsperf.com/event-delegation-with-capture-vs-bubble

TL;DR ~45% performance boost across the board.

[stefanpenner]

@rwjblue closed in favor of rfc/implementation?

[runspired]

your commit doesn't fix the run loop join, only the ability to turn mousemove off

[runspired]

^@rwjblue

[stefanpenner]

Maybe we need to bundle up some of the talking points and push some work for capture phase + improved event dispatcher.

Having vague hard to action issues is annoying.

[runspired]

@stefanpenner I have a branch where I am working on this actually, I think it's been added to your Friday schedule as a talking point

[stefanpenner]

Awesome. Can you link it. Or submit a WIP for review?

[runspired]

PR coming as soon as I finish off the walker. Will also be adding a second flag for resolution caching (e.g. from element to handler) so that repeated firing is super fast.

runspired@f682df0

[rwjblue]

@stefanpenner - It is also on our agenda for Friday.

[rwjblue]

Sorry I was trying to reference this issue, not close it.