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

Table efficiency #276

Open
pja237 opened this issue Oct 19, 2022 · 1 comment
Open

Table efficiency #276

pja237 opened this issue Oct 19, 2022 · 1 comment

Comments

@pja237
Copy link

pja237 commented Oct 19, 2022

Hello great people of charm community,
i have a question regarding the table component and it's responsiveness with the increase of table rows.
We try to use it in an app we're developing and everything was good until we started getting to multiple thousand rows where we noticed a significant degradation in responsiveness and a noticeable spike in cpu-usage when scrolling up/down.

Some details:
We have a table with 6 columns. When the row count is "low", <1k the moveup/down calls are executed fairly quickly, we put some log printfs to measure the time in the Update()

		case key.Matches(msg, keybindings.DefaultKeyMap.Down):
			m.Log.Printf("Update: Move down\n")
			activeTable.MoveDown(1)
			m.Log.Printf("Update: Move down finished\n")

For the "low" row count (~<1k) the moves take ~0.002 seconds, which then looks nice and responsive:

SC: 14:42:47.489481 update.go:374: Update: Move down
SC: 14:42:47.491627 update.go:376: Update: Move down finished
SC: 14:42:47.524541 update.go:374: Update: Move down
SC: 14:42:47.526015 update.go:376: Update: Move down finished
SC: 14:42:47.550793 update.go:374: Update: Move down
SC: 14:42:47.553671 update.go:376: Update: Move down finished

But as the row count grows, time and cpu-load to execute activeTable.MoveDown() grows significantly, example for 4.5k rows, takes up to 0.5 seconds to do one movedown(), making the app very, very sluggish 😄

SC: 15:20:04.758040 update.go:374: Update: Move down
SC: 15:20:05.055713 update.go:376: Update: Move down finished
SC: 15:20:05.261438 update.go:374: Update: Move down
SC: 15:20:05.607520 update.go:376: Update: Move down finished
SC: 15:20:05.609510 update.go:374: Update: Move down
SC: 15:20:05.925371 update.go:376: Update: Move down finished
SC: 15:20:05.926199 update.go:374: Update: Move down
SC: 15:20:06.121864 update.go:376: Update: Move down finished

By having a brief look at the code, looks like this extra time might come from the

bubbles/table/table.go

Lines 312 to 314 in 85da9ad

func (m *Model) MoveDown(n int) {
m.cursor = clamp(m.cursor+n, 0, len(m.rows)-1)
m.UpdateViewport()

call to UpdateViewPort()
which for every movedown() iterates and makes a copy of the entire table in renderedRows?

bubbles/table/table.go

Lines 243 to 247 in 85da9ad

func (m *Model) UpdateViewport() {
renderedRows := make([]string, 0, len(m.rows))
for i := range m.rows {
renderedRows = append(renderedRows, m.renderRow(i))
}

If this guess is correct, could you perhaps optimize this in some way?
Perhaps render only rows there are "visible" instead of the whole table?
Or if i'm miss-using the table in some way point me on how to better do this?

Many thanks!

@MichaelMure
Copy link

Not doing the rendering of hidden rows is a good first step, but there is also the problem of lazy-loading the rows themselves.
It's quite common to load the content of a table lazily, possibly through a paginated HTTP call. In the current incarnation of that table component, it seems to force knowing from the beginning the full content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants