Searchpane issue with cascade and ajax reload

setwebmaster

As mentioned in the Searchpane-feedback thread, there's a performance issue (pretty intense issue in fact), appearing when using cascading panes along with data.ajax.reload()

(see here for further details)

colin

Ah, I did mean further issues, but this is fine - easier to track As I mentioned in that other thread, I've raised it internally (DD-1372 for my reference) and we'll report back here when there's an update. There should be a reply within a few days.

Colin

setwebmaster

All right, sorry for "re-posting", I thought it would be better to follow your recommendation and split it into its own issue.

I would have done it through github or another proprer issue tracker as it's easier to track related code changes, but it's your choice to track it here and I respect it )

setwebmaster

I think I have found an infinite loop when using both cascading panes with ajax reload (I didn't find the source yet, but I did put counters in the SearchPane.prototype._updateCommon function and as soon as I use the table.ajax.reload() and then select a filter in one of the panes, the counter starts to go up and keeps going indefinitely

setwebmaster

Is there a way to mimic table.ajax.reload() with http://live.datatables.net/ ?
I'm trying to replicate the issue with the minimum feature set possible, but I can't manage to use the ajax reloading within the jsbin...

I did create a small repro project using asp.net core in which I initialize the table with the least possible options and I can replicate it, but I don't know what's the best way to share it with you so you can investigate better.

Small detail, it seems like it is when selecting 2 filters after using table.ajax.reload() that the counter starts to grow indefinitely (possibly pointing to something related to cascadeRegen maybe?

You told me in the feedback thread that you found some odd things, is this one of them?

kthorngren

Is there a way to mimic table.ajax.reload() with http://live.datatables.net/ ?

You can use one of the Ajax templates here:
https://datatables.net/manual/tech-notes/9

Kevin

colin

You can use this example here - it's using Ajax so you can call ajax.reload().

I was seeing an odd error: Cannot read property 'clientWidth' of null - that was from this example which I created to try and replicate your setup, without success.

Colin

colin

If you're able to tweak my example above to replicate, that would be the best bet.

FYI @sandy hasn't had a chance to look at the issue yet, he only works part-time, but once he gets going, he'll be able to ask for the .net project if he's unable to reproduce the issue without.

Colin

setwebmaster

@colin I couldn't replicate the infinite loop yet (I will have to double check on my side on what differs from the repro I provided here), but already you can see a major performance drop when using the table.ajax.reload() function by clicking the "reload table button)

Do it a couple times (simulating a user changing a filter or anything else that would cause data to be reloaded), and then click on the filters, you will immediately feel the difference. Moreover, if you check the number of times the updateCommon function is called, it keeps growing and growing even if a single selection is made in the panes. (growing everytime you reload table data dynamically)

See repro here: http://live.datatables.net/xecagaxo/1/edit

setwebmaster

Here is a performance profile where you can clearly see there's a problem with the event listeners being registered when a selection is made in one of the panes. A couple table data reloads are made between seconds 7 and 13. Then each spike you see in event listener count is when I select an option in one of the panes.

To me there's something that looks like there's a kind of "compounded" effect here. As you can see in the profile between the first item selected and the second one, the event listeners seem to have almost doubled, so I'm wondering if it's not a kind of cyclic problem where each option change triggers listeners on other options, which are then updating themselves, hence re-triggering other listeners etc... (though not perfectly cyclic as it would be an infinite rise in event listeners, memory etc....) which is not the case in this specific repro scenario, but which seems to be the case on my app...

setwebmaster

I have a question regarding the rebuildPane function, why doesn't it call the destroy method on itself and only calls it on this.s.dtPane?

I saw that SearchPane.prototype.destroy() does effectively unregister all (or most) event listeners when being called, but now when calling rebuildPane, this is never done.... I'm wondering if that could be the major problem here.

sandy

Hi @setwebmaster ,

For your first point in relation to the performance impacts - yes you are absolutely right those were being caused by the event listeners not being cancelled. This in turn triggered more and more calls to the above mentioned function, which led to more event listeners. We actually identified this previously and implemented a fix which is available in the nightly builds. I've updated your example to use these and you should see a large performance increase.

For your second point relating to rebuildPane() - the destroy function has to be called on dtPane otherwise you run into some weird behaviour when rebuilding. The nightly builds you can see feature a setListeners() function which won't run on every rebuild - only if they are not yet set.

Sorry for the slow response and hope that this helps,

Sandy

setwebmaster

@Sandy thanks for the update, I will test it right away and reply with the results I see within my app.

setwebmaster

@Sandy, at first sight it seems really better than previous version, good job!

I saw there still seems to be a little bit of a memory leak when using the table.ajax.reload() which also increases the number of event listeners, but it's way better than it previously was and it seems to be picked up for most of it by the GC eventually.

setwebmaster

I don't know for sure but this fix included in the nightly should probably be pushed as soon as possible to the CDN shouldn't it? As the actual version deployed can cause some major hiccups (I was able to almost make my computer crash when using it with table.ajax.reload() multiple times and selecting a couple filters in different panes (there was an infinite loop somewhere and I was quickly ramping up to over 500K dom nodes (lots of them being Detached) while the page is normally < 20K

allan

We probably aren't too far away from a v1.1 release of SearchPanes, so I think we'll hold off until that is ready, since the fix discussed can already be used locally or from the nightly if the way SearchPanes is being utalised triggers this issue.

We do need to look at doing faster patch releases in general though.

Allan

setwebmaster

@sandy and @allan I have discovered another problem which might be related to the fix that has been applied for this problem here.

When using the clearAll button, it seems to return the state that was actual when the page was first loaded.

Effectively, if using the SearchPanes along with table.ajax.reload(), the new data does not seem to make its way all the way through the binsOriginal array, which will always contain the data available when the table was first initialized.

The source of the problem seems to lie in the _buildPane function -> row 437 (at the moment) where there's a line which checks if data is already available in loadedFilter and if so, it takes old data instead of the new data....

This does not seem apparent at first sight as the panes will update correctly with new data, but whenever you hit the clearAll button, you will be greeted with new data in the main table, but with filters in panes that are not related to the newly loaded data.

To reproduce though through the live.datatables.net fiddle, you would need to have 2 sets of data available so that the refreshTable button does not always get the same data back, in which case the filters always seem to match as the data never really changed.

colin

Thanks for reporting, I've raised it internally (DD-1380 for my reference) and we'll report back here when there's an update.

Colin

setwebmaster

@colin All right, thanks for taking a look at it. Temporarily, this issue can be fixed by disabling stateSave feature. For those who still want to use state save with the main table but not with search panes, the changes required are not too complicated.

I had proposed a PR to enable specific state saving option on the searchPanes extension (before some other changes were applied in latest commits so it would need to be updated to work with newest changes, but it's still not too complicated).

Hope you'll find what's going wrong with the bins, keep up the good work guys!

colin

@setwebmaster This should all be resolved now - could you try with the nightly builds, please, and report back if you're seeing odd behaviour still.

setwebmaster

@colin I'll test the nightly and come back to report.