Clusterize.js icon indicating copy to clipboard operation
Clusterize.js copied to clipboard

Performance hit when table has inputs

Open thdoan opened this issue 7 years ago • 4 comments

Clusterize.js works like a charm when the table has text content, but if every row has an input in it I noticed that in Chrome (v59) it slows to a crawl. I've posted a comparison here:

https://codepen.io/thdoan/pen/NgqmVG

Try dragging on the scrollbar for each table. You should see that you can barely drag the scrollbar on the second table (with the inputs) -- the dragging is very choppy.

At first I thought this was a result of the browser rendering, but that can't be it because you can scroll through a 5000-row table by dragging on the scrollbar fairly smoothly:

https://codepen.io/thdoan/pen/qjdGow

(Wait for the table to fully load before dragging the scrollbar; the table is fully loaded when you can put focus inside an input.)

It would be an interesting challenge to optimize Clusterize.js so that it would have better performance when dynamically rendering rows containing input elements.

thdoan avatar Jun 07 '17 12:06 thdoan

Hey Yeah, I see. Seems to be browser needs more time to render input field because of it's dynamic nature.

Currently clusterize does not explore content of your rows. It may be good idea to create inputs once and reuse those inputs for other "clusters" during scrolling, if only clusterize was specific library for table with "input in every row". But because of it's general use the easies solution for this case may be just decrease amount of rendered rows per cluster, try to set "rows_in_block: 10", should work faster.

NeXTs avatar Jun 07 '17 12:06 NeXTs

Thanks for the suggestion, @NeXTs . I haven't looked at the code, but are you using insertRow() or document fragments to do the clustering? There are jsperfs that show using document fragments is faster than multiple insertRow().

thdoan avatar Jun 07 '17 13:06 thdoan

Much easier, the whole list is replaced using .innerHTML

https://github.com/NeXTs/Clusterize.js/blob/master/clusterize.js#L297

NeXTs avatar Jun 07 '17 13:06 NeXTs

Adding a couple lines to the end of insertToDOM() made a noticeable difference in the scrolling performance:

https://github.com/thdoan/Clusterize.js/commit/838394218a2969e92033e3ff838e8a7614f63039

You can see the improvement here:

https://codepen.io/thdoan/pen/RgWxqe

I didn't do a PR because it's just a quick fix for myself. After impelmenting this you will obviously need to make some changes so that it won't override the user's rows_in_block option.

thdoan avatar Jun 08 '17 07:06 thdoan