New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Math-widget ts.computeColumnIndex slowdown #1048

Closed
LvLynx opened this Issue Oct 14, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@LvLynx
Contributor

LvLynx commented Oct 14, 2015

Hi!

I am building a report with quite large data set: 3500 rows with 18 columns total.
I have two tbody class="tablesorter-infoOnly" from which 15 cols have col-sum function 1 of them is on top of table, second in bottom. In middle there is third tbody with all data.

The problem is: firefox freezes for me at least once or twice while working with this data.
I did some profiling and found 1 huge bottleneck:
mathwidget

If I read this correctly seems that ts.computeColumnIndex takes 36% of all execution time, which seems huge. Any ideas how to optimize this?

On first look at least in math widget return value is not used anywhere. So maybe matrixrow.length calculation can be divided in separate function? Also lookup = {}; object is not used anywhere.

Maybe it is possible that this function also gets called more than once?

Thank you!

Update: github didnt display tbody tag in within < >.

@Mottie

This comment has been minimized.

Show comment
Hide comment
@Mottie

Mottie Oct 15, 2015

Owner

Hi @LvLynx!

Because the math widget works with information tbodies, and those tbodies allow colspan and rowspan to be set, the $.tablesorter.computeColumnIndex function needs to be executed on pretty much every row in the table. Sadly the algorithm isn't able to calculate the correct indexes without including rows with the spans and rows without spans. It calls this function on initialization and whenever the table is updated ("update", "updateCell", "addRows", etc), so yes, it may happen more than once.

I think you're right about the lookup variable, I'll remove it.

I wrote the math widget to utilize the data-column attribute while gathering information. And now that I look back at it, it is terribly inefficient. I do need to update the widget to only apply data-column indexing to rows that need it and change a bunch of code to use the cellIndex on rows that don't have spans.

I'm not sure when I'll have time to get to it, so for now, I'll detach the table from the DOM, index the table cells, then add it back. That may speed up the process a little (don't count on it in IE).

Owner

Mottie commented Oct 15, 2015

Hi @LvLynx!

Because the math widget works with information tbodies, and those tbodies allow colspan and rowspan to be set, the $.tablesorter.computeColumnIndex function needs to be executed on pretty much every row in the table. Sadly the algorithm isn't able to calculate the correct indexes without including rows with the spans and rows without spans. It calls this function on initialization and whenever the table is updated ("update", "updateCell", "addRows", etc), so yes, it may happen more than once.

I think you're right about the lookup variable, I'll remove it.

I wrote the math widget to utilize the data-column attribute while gathering information. And now that I look back at it, it is terribly inefficient. I do need to update the widget to only apply data-column indexing to rows that need it and change a bunch of code to use the cellIndex on rows that don't have spans.

I'm not sure when I'll have time to get to it, so for now, I'll detach the table from the DOM, index the table cells, then add it back. That may speed up the process a little (don't count on it in IE).

@LvLynx

This comment has been minimized.

Show comment
Hide comment
@LvLynx

LvLynx Oct 15, 2015

Contributor

Nice, thanks. Just ran some tests, graphics part is better. But unfortunately ts.computeColumnIndex has almost no changes.
mathwidget2

Any ideas, or I just need to reduce data set?

Contributor

LvLynx commented Oct 15, 2015

Nice, thanks. Just ran some tests, graphics part is better. But unfortunately ts.computeColumnIndex has almost no changes.
mathwidget2

Any ideas, or I just need to reduce data set?

@Mottie

This comment has been minimized.

Show comment
Hide comment
@Mottie

Mottie Dec 4, 2015

Owner

Hi @LvLynx!

Sorry for taking so long... please try the updated tablesorter core & math widget in the master branch. A data-column is now only added to cells where the cellIndex property does not match the calculated column index. So there is less DOM manipulation and this change will hopefully provide much better performance.

Let me know how this works out for you!

Owner

Mottie commented Dec 4, 2015

Hi @LvLynx!

Sorry for taking so long... please try the updated tablesorter core & math widget in the master branch. A data-column is now only added to cells where the cellIndex property does not match the calculated column index. So there is less DOM manipulation and this change will hopefully provide much better performance.

Let me know how this works out for you!

@Mottie Mottie added the Next Update label Dec 4, 2015

@Mottie

This comment has been minimized.

Show comment
Hide comment
@Mottie

Mottie Dec 13, 2015

Owner

@LvLynx!

I made even more optimizations. So, please try out the math widget in the master branch. I plan on pushing out a new release tomorrow, so hopefully I'll get some feedback from you before then. Thanks!

Owner

Mottie commented Dec 13, 2015

@LvLynx!

I made even more optimizations. So, please try out the math widget in the master branch. I plan on pushing out a new release tomorrow, so hopefully I'll get some feedback from you before then. Thanks!

@LvLynx

This comment has been minimized.

Show comment
Hide comment
@LvLynx

LvLynx Dec 13, 2015

Contributor

@Mottie Awesome work! I tested same data set and it loads much smoother without any browser freezes. Total execution time is twice as fast and previous bottleneck computeColumnIndex is nowhere to be seen in call graph. Big thanks :)

tablesorter

Contributor

LvLynx commented Dec 13, 2015

@Mottie Awesome work! I tested same data set and it loads much smoother without any browser freezes. Total execution time is twice as fast and previous bottleneck computeColumnIndex is nowhere to be seen in call graph. Big thanks :)

tablesorter

@LvLynx LvLynx closed this Dec 13, 2015

@Mottie Mottie removed the Next Update label Dec 13, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment