[perf] filtering improvement

[m4dz]

:warning: This PR is not ready to merge at this time. It is in progress and here for review / tests only.

This PR introduces a perf improvement for filtering / indexing that lets Ui be more reactive.

EDIT: Now ready \o/

[poupotte]

Pictures reload when I edit a contact : Steps to reproduce :

• Open application
• Contacts pictures appear
• Edit a contact
• All pictures are removed We have to scroll to force pictures reloading.

[Edit - 18/01] : Fixed !

[poupotte]

Contacts counter is wrong on filter.

Steps to reproduce :

• Filter contacts (with tag)
• Display x contacts but counter corresponds to all contacts

[Edit - 18/01] : Fixed !

[poupotte]

Search seems broken.

Steps to reproduce :

• Search string in your contacts list
• Nothings happens

Error in console js :

"TypeError: this.model is undefined" "http://localhost:9114/scripts/app.js" 175 4 TypeError: this.model is undefined
Stack trace:
Application</Application.prototype.search@http://localhost:9114/scripts/app.js:175:5
SearchView</SearchView.prototype.updateSearch@http://localhost:9114/scripts/app.js:6717:12
bind/<@http://localhost:9114/scripts/app.js:6669:54
complete@http://localhost:9114/scripts/vendor.js:7790:20
delayed@http://localhost:9114/scripts/vendor.js:7800:11
 "Application</Application.prototype.search@http://localhost:9114/scripts/app.js:175:5
SearchView</SearchView.prototype.updateSearch@http://localhost:9114/scripts/app.js:6717:12
bind/<@http://localhost:9114/scripts/app.js:6669:54
complete@http://localhost:9114/scripts/vendor.js:7790:20
delayed@http://localhost:9114/scripts/vendor.js:7800:11
"

[Edit - 18/01] : Fixed !

[poupotte]

Problem with highlight on search.

Steps to reproduce :

• Search a part of your word. Example 'te'
• Wait first responses for your search
• Complete your search. Example 'te' -> 'test'
• Highlight isn't updated

[Edit - 19/01] : Fixed !

[poupotte]

Problem with new search :

Steps to reproduce :

• Search a string. Example 'test'
• Wait responses for your search
• Modify your search. Example 'test' -> 'te'
• Counter is updating but contacts list doesn't change.

[Edit - 19/01] : Fixed !

[frankrousseau]

In my development environment it keeps raising the same error:

Uncaught TypeError: Cannot read property 'push' of undefined
module.exports.FilteredCollection.updateIndex   @   filtered.coffee:72
(anonymous function)    @   filtered.coffee:82
module.exports.FilteredCollection.resetIndexes  @   filtered.coffee:81
module.exports.FilteredCollection.reset @   filtered.coffee:140
triggerEvents   @   backbone.js:370
triggerApi  @   backbone.js:356
eventsApi   @   backbone.js:155
Events.tri

[frankrousseau]

Ok, ignore my previous message. After cleaning my sources, it works. So here is my feedback;

• Filtering and dialog display is really faster.
• Search is broken: can't do two searches in a row and at some point the whole browser crashes.
• Contact counter doesn't match the number of displayed contacts.
• What is still slow is the inital loading and the contact creation.

[m4dz]

What is still slow is the inital loading and the contact creation.

Yup, I'll open another PR for those ones cause it's not related to filtering perf. Thanks @frankrousseau for your tests!

[poupotte]

It's ok for me :+1: !

[m4dz]

@nono, @frankrousseau do you want to do some other reviews/tests, or is it ok for merging?

[nono]

It's OK for me

[frankrousseau]

Search is still very slow and freezes my browser. Moreover after one or two searches, the result is always empty. Unfortunately we can't merge this PR now.

For information, latest contact version search feature worked fine.

[poupotte]

@frankrousseau : I don't arrive to reproduce your issues :/

• You test on local or on a real cozy ?
• How much contacts have you (about) ?
• Empty results arrive after two differents search or two differents completion on the same search (ex: 'te', then 'tes', then 'test') ?
• Have you some errors on your console js ?
• Have you some errors on your server ?

[frankrousseau]

Freeze occurs when the search is emptied.

I found this error in my web console:

TypeError: selector is undefined "http://localhost:9114/scripts/vendor.js" 32213 1 TypeError: selector is undefined
Stack trace:
Marionette.CompositeView<.getChildViewContainer@http://localhost:9114/scripts/vendor.js:32213:1
Marionette.CompositeView<._insertAfter@http://localhost:9114/scripts/vendor.js:32188:24
Marionette.CollectionView<.attachHtml@http://localhost:9114/scripts/vendor.js:31940:11
Marionette.CollectionView<.renderChildView@http://localhost:9114/scripts/vendor.js:31855:7
Marionette.CollectionView<._addChildView@http://localhost:9114/scripts/vendor.js:31833:7
Marionette.CollectionView<.addChild@http://localhost:9114/scripts/vendor.js:31778:7
Marionette.CollectionView<._onCollectionAdd@http://localhost:9114/scripts/vendor.js:31452:9
triggerEvents@http://localhost:9114/scripts/vendor.js:20002:32
triggerApi@http://localhost:9114/scripts/vendor.js:19987:19
eventsApi@http://localhost:9114/scripts/vendor.js:19786:1

[m4dz]

Ok, the switch between the 2 modes (indexed vs scored) is still slow, especially when you empty the search field. I tried ti figure why.

Your error with selector is undefined has been fixed is last commits. Can you try another git pull and test again? Thanks!

[frankrousseau]

You test on local or on a real cozy ?

I test on a local Cozy with Firefox.

How much contacts have you (about) ?

869 contacts

Empty results arrive after two differents search or two differents completion on the same search (ex: 'te', then 'tes', then 'test') ?

See my previous message. The first search is slow. When I delete my search, it freezes for several seconds, then it doesn't return any results. Here is what I type:

• 'alex' then Enter.
• Backspace * 4 then 'Zoe' then Enter

[frankrousseau]

I edited the previous message, the final scenario was wrong.

[frankrousseau]

OK i deleted and rebuild the repo on my local computer. I didn't handle the conflicts properly after @m4dz additions (btw I don't understand why I have conflicts on a branch I didn't change).

I don't have the bugs anymore but there is still the freezing problem.

[frankrousseau]

Here is my CPU profile: https://frank.cozycloud.cc/public/files/files/399d38ef0557f71cffd7ac5aae28e670/attach/slow-search-20160119T124336.cpuprofile

[poupotte]

ok, thanks @frankrousseau for the details

[poupotte]

@frankrousseau , can you test again with new m4dz changes ?

I have a bug : when I edit a contact with a picture, his picture disappears. I have to scroll to retrieve his picture. This issue isn't fonctionnaly blocking, so I propose to merge this PR (already big) and fix it tomorrow in another PR if you haven't others issues.

[frankrousseau]

@m4dz do you rewrite history ? I don't know why every time I fetch changes, it leads to many conflicts. I have to rebuild the whole project from scratch. It's time consuming.

[frankrousseau]

I'm little bit embarassed. Search performances are still very poor. Especially when you empty the search.

[frankrousseau]

Let's merge it but we have a big issue here with search performances.

[frankrousseau]

We cannot publish the app in that state.

[m4dz]

@m4dz do you rewrite history ?

Yep, I'll often rebase my commits on development branch before push my own branches to prevent a useless and unreadable history. If you already have my fork registered as remote, you can simply update the forced branch with:

git fetch
git reset --hard FETCH_HEAD
git clean -df

So you won't have to rebuild the project from scratch :).

---

Thanks for the merge! For your remaining perfs issues, I'm disappointed as I can't reproduce them even with 1500 contacts… Maybe it's due to your data specifically. I suggest we make a peer-debug session on Thursday. Is it ok for you?

[frankrousseau]

Ok

[frankrousseau]

Thank you for the git tip by the way.

[frankrousseau]

NB: Additional commits are not useless neither unreadable. They make things simpler for your reviewer because he can more easily follow what is changing.

[m4dz]

You catch a good point concerning review… I'll think a bit about it in our dedicated mail thread :wink:.

Also, ones give me this other tip that setting pull.rebase to preserve will simply do all the tricks for you :sweat_smile: (and it's probably a good idea to always rely to rebase preserve on pulls):

git config --global pull.rebase preserve

[frankrousseau]

It forces other developer to think before doing a simple and reccurent action. It's not something we should foster in the Cozy project. That's why I feel very doubtful about history rewrite.