Fix performance issue with the vue-utilities v-mousetrap

[scottwittenburg]

MIQA uses v-mousetrap from vue-utilities, and actually moved that library code into the application since it's no longer being maintained at its previous home.

Debugging performance degradation over many experiment loads, I discovered over 100K listeners bound to the DOM, most of which were created by `v-mousetrap. In this branch commit, I experimented with just removing the use of that library, and the number of listeners stopped growing without bound, and experiment loading performance was greatly enhanced.

However, key bindings are an important feature in the application, so we need to either fix v-mousetrap, or else replace it with something else.

[scottwittenburg]

This turned out not to be an issue with v-mousetrap, but rather with the underlying mousetrap library.

See the linked miqa PR, above, for links to the mousetrap issue and fix.

[scottwittenburg]

@BryonLewis I wonder if you ever encountered this issue in viame-web? Either way, do you have any thoughts/ideas you want to share?

Otherwise, I plan to merge #46 here soon, as it has resolved the issue with unbounded growth of listener count in miqa.

[BryonLewis]

I haven't seen this but it could be a result in the way that mousetrap works and the way we've implemented it. I'm wondering if some elements aren't properly calling unbind and are being abandoned and still connected.

export default function install(Vue) {
  let count = 0;
  Vue.directive('mousetrap', {
    inserted(el, { value, modifiers }) {
      console.log('inserted');
      bind(el, value, modifiers.element);
      count += 1;
      console.log(count);
    },
    update(el, { value, modifiers }) {
      console.log('update');
      console.log(el);
      unbind(el);
      bind(el, value, modifiers.element);
      console.log(count);
    },
    unbind(el) {
      console.log('unbind');
      console.log(el);
      unbind(el);
      count -= 1;
      console.log(count);
    },
  });
}

I did that on viame-web to check and see how many items are being bound and unbound. I loaded multiple datasets and switched between them and at most I had 12-13 bound depending on what dialogs were open. We even have a unique case were we present a list of tracks in the side that could be 2000K items long but it uses a virtual scrollbox so only 10 are visible at a time and it was properly binding and unbinding the values.

https://github.com/VIAME/VIAME-Web/blob/master/client/viame-web-common/vue-utilities/v-mousetrap/index.js
That is a link to our implementation which appears slightly different than yours I haven't reviewed each fully, but it looks similar enough.

I wish I could be more help immediately but my first guess is to confirm that these items are calling the unbind directive when they are removed.
https://vuejs.org/v2/guide/custom-directive.html

It could very well be something with mousetrap as well.

[scottwittenburg]

To be certain it's not a failure to call unbind, I did the same as you, and the count never went over 17, though after scanning through many images, the listener count was still over 30K. The updated method was called 1000's of times during this interaction session though, and each time, it calls unbind (calling Mousetrap.reset(), which doesn't remove the event listeners added internally), and then bind again.

If you're noticing a lot of calls to updated as well, then you likely have the same issue we have in miqa. To check, you could do a performance capture right after a reload of the application and look at the listener count. Then interact with the app a lot so updated is called a bunch, and then do another capture to see the listener count again. Another way to see it is after a lot of updated calls, look at the Event Listeners tab on the right under Elements and make sure you have Ancestors -> All selected. Open any of keydown, keyup, or keypress listeners, as in this screenshot:

I looked at your vue-utilities/v-mousetrap/index.js as well, and mostly the only difference was where I changed it expose the other event types supported by mousetrap (if you recall this closed PR). So, I agree with you that it looks similar enough.

If you look at the issue and fix on the mousetrap repo, it lines up pretty much exactly with what I'm seeing here. In fact, several other people have reported the same issue, but in different words, usually less clearly, and without a corresponding fix.

Thanks for taking the time to have a look at this @BryonLewis, I really appreciate it 😁

[BryonLewis]

Thanks for the explanation, I was looking at the event Listener tool and didn't notice the 'reload' icon. Now that I'm actually using the reload button I can see the same exact thing happening when scrolling though those lists.

Although it's interesting if I decide to iterate through all of the DOM nodes using: https://developers.google.com/web/tools/chrome-devtools/console/utilities#geteventlisteners
This reports a standard number of bound events for keypress/keydown no mater what I do.

I haven't read the mousetrap stuff but I imagine they are having some issues with garbage collection of the events so while unbound they are remaining in memory.

[dzenanz]

Fixed by #46.