Improved filter performance on simple attribute selectors

[ibaca]

I have a listener on the body filtered by a data attribute to create advanced tooltips. I use this patch to fix the performance problem (https://github.com/ArcBees/gwtquery/issues/177). I think that this filters on attributes are quite common for delegated event listeners, and this pull-request solves in a simple way the performance problem in this situation.

[jDramaix]

Thanks for the contribution.

Could you add tests covering your change ?

[ibaca]

Basic test added, this don't actually test if the shortcut is used (I check that using breakpoints :dizzy_face:), but checks the basic case for each attr operator.

[manolo]

Hi Ignacio, I had already a fix to improve filter performance, not this case concretely but any other so as we can fix issue #177 . It never went to gquery because I didn't get the time to do exhaustive performance testing. IIRC, the problem in filter is the necessity to add a ghost parent per filter and per element. I will commit this so as you can check whether my fix solves your performance problem, or even if we can improve merging both approaches.

[ibaca]

Ok, I think than my pull request is more a hack (or shortcut) than a fix. So, I will wait to your fix, and hope that you really fix this :smile:.

The problem is the parentNode call in the filter method (https://github.com/ArcBees/gwtquery/blob/master/gwtquery-core/src/main/java/com/google/gwt/query/client/GQuery.java#L2291), and when I try to find an alternative solution, i.e. use the current node not the parent node, I cannot find any alternative because all selectors methods expect the parent element as an argument (and update the query selector looks too complex). I'm really interested to see how you solve this.

[manolo]

Ignacio, can you test this: https://github.com/ArcBees/gwtquery/pull/311 ?

Maybe you can use the example project to play with it in order to improve the implementation if you have a better idea. In the example project I have copied both filter implementations so as it's easy to compare performance.

[ibaca]

Two profiling screenshot for a $("[data-tooltip]").live("hover", callback), the callback do nothing, just moving the mouse and the filtering method cause all the cpu ussage.

This is with the simpleAttrFilter. This is with your improvement (almost the same as current GQuery filter version).

[manolo]

Ok, I'm not against this patch in essence, but I would like not to maintain the Filter class in gQuery, because of different reasons.

What about a method in gQuery to register customised filter predicates, something like:

GQuery.registerFilterPredicate(FilterPredicate p) GQuery.getFilterPredicate(String selector)

[ibaca]

Register new filter predicates is good enough. But, what do you think of moving the filter code inside the Selector Engine (which currently contains some filtering code, and I think that this filtering code must be part of the selector engine, some selector engine currently support filtering), and instead of allow the user to subscribe new filter predicates, permit to override the SelectorEngine.

In this situation, if someone want to add some custom filtering may create a SelectorEngine decorator which add some custom filtering or event selector code, and delegates other cases to the default selector engine.

I will wait that this https://github.com/ArcBees/gwtquery/pull/311 be merged, and I will made a porposal in this pull request. If you don't like the idea :disappointed:, I will rollback to the GQuery.registerFilterPredicate idea.

[manolo]

311 has been merged, filter stuff has been moved to SelectorEngine so as you can override since GQuery is instanciating it with GWT.create. Make your proposal based on this change

[ibaca]

I suppose that this issue is not longer required. I tried it in this test project https://github.com/ibaca/gwtquery-issue-302. Extends SelectorEngine to add custom filters is perfect solution for this edge cases. Thanks.

[manolo]

Closing this request since its not needed anymore. People coming to this can use @ibaca's example as reference.

Ignacio, muchas gracias.