Skip to content
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

tb.toString() performance #68

Open
GopherJ opened this issue Dec 6, 2018 · 5 comments
Open

tb.toString() performance #68

GopherJ opened this issue Dec 6, 2018 · 5 comments
Labels
bug Something isn't working

Comments

@GopherJ
Copy link

GopherJ commented Dec 6, 2018

Hello, I found that when the number of rows is huge, I start to have some performance problems.

I have 1763 rows (5 columns) and it takes me 4473.023ms to run toString method, I want to know if there is a way to improve the performance ?

Thank you!

@GopherJ
Copy link
Author

GopherJ commented Dec 6, 2018

screenshot from 2018-12-06 21-16-09
A row is like this, but isn't always so big.

@Turbo87
Copy link
Contributor

Turbo87 commented Dec 6, 2018

there probably are ways to speed it up, it "just" needs someone with a bit of time and knowledge of profiling 😉

@DanielRuf
Copy link
Contributor

DanielRuf commented Jan 28, 2019

Can you provide some more info like the Node.js version and a reproducible test case?

@DanielRuf DanielRuf added the bug Something isn't working label Jan 28, 2019
@speedytwenty
Copy link
Collaborator

Pending #278, which is intended to resolve a layout bug, includes some performance gains that can be furthered.

After doing some further benchmarking, I believe it might be possible to get the layout rending of 100k cells to under or around 2 seconds—which might get close to "as good as it gets", or, roughly, a 3,600% decrease in rendering time at the least. Rendering 10k cells, would be well below 1 second (depending on table complexity).

Insofar, only the layoutTable() function has been refactored yet all of the makeTableLayout() function can likely be accomplished in a single pass over the table. Presently it's traversing the table 4+ times with some "exponential looping" remaining in fillInTable(), addRowSpanCells(), and addColSpanCells().

I haven't written many performance tests, but think I'll take a stab at it. My hunch is to attempt to test for a time increase exceeding a threshold on nominally sized tables rather than rendering large tables and testing that they rendered within a fixed threshold. I'll try to post a PR with failing tests when I've got one and work from there!

@speedytwenty
Copy link
Collaborator

speedytwenty commented Apr 13, 2022

With the release of 0.6.2 and #278, the performance of cli-table3 should noticeably increase for nearly all large tables—while even small tables will render in substantially less time at a micro level.

Using the performance testing tool at #294, rendering-time for a 1,763 x 5 table has decreased from the stated 4.5 seconds to approximately 0.5 seconds.

While this is 800% better, and likely sufficient in most cases, the memory consumption is still terrible and there is another ~2,500% decrease in rendering time that can be had!

Update to v0.6.2+ for these (initial) performance enhancements.

➭ node scripts/generate 1763 5
Generating basic table with 1763 rows and 5 columns:
Memory usage at startup: 2.6783065795898438mb
Memory usage after table build: 3.9880523681640625mb
table built in 4.863 ms
Memory usage after table rendered: 21.6505126953125mb
table rendered in 490.181 ms

badeball added a commit to badeball/cypress-cucumber-preprocessor that referenced this issue Oct 21, 2022
The former was incredibly unperformant [1].

[1] cli-table/cli-table3#68
badeball added a commit to badeball/cypress-cucumber-preprocessor that referenced this issue Oct 21, 2022
The former was incredibly unperformant [1].

[1] cli-table/cli-table3#68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants