search

ludo / jquery-treetable

jQuery plugin to show a tree structure in a table
http://ludo.cubicphuse.nl/jquery-treetable
GNU General Public License v2.0
741 stars 278 forks source link

Initialization performance in large table #144

Open hpcsc opened 10 years ago

hpcsc commented 10 years ago

Hi,

I have a reporting table that use treetable, and with the data is average (hundreds of rows, around 50 columns), the initialization of treetable is quite slow. As in the picture below, it takes 4 seconds +

treetable-before

After debugging and profiling the code, I found out that it's actually the checking whether the row is visible using jquery selector is slow (in Node.prototype.expand(), the checking of $(this.row).is(":visible")) After I replace that checking with !this.row[0].hidden, the performance is greatly improved (200 ms +)

Not really sure this breaks anything, but so far it's working for me. So I'm just reporting it and if the author can help to check whether it's a valid fix

treetable-after

acacha commented 9 years ago

@hpcsc I will try this! If I see any errors i wil say to you

It has been a lot of time this issue was posted so maybe you have some updated info about this?

acacha commented 9 years ago

Or maybe @ludo will apply a change I have done a Pull request https://github.com/ludo/jquery-treetable/pull/179

djlerman commented 9 years ago

Will this be applied to a new available version soon or should I try and apply this fix to my local installed version?

ludo commented 9 years ago

I would love to accept this change, but as I mentioned in the related merge request (#179) a test fails when this change is applied. I currently do not have the time to invest a lot of time in this but if you are able to figure out why this test fails I will gladly accept the change. (perhaps the test is invalid, or maybe there is an actual issue with the change)

djlerman commented 8 years ago

I wish I knew what that meant. I don't know anything about the tests. Is there a way via GitHub to start bounties for actions?

pete-woods commented 6 years ago

I added a new pull request (#210) that fixes the bug, while keeping the tests passing.