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

Improvements to reflow performance by removing O(n^2) behavior #3045

Merged
merged 3 commits into from
Jan 18, 2024

Conversation

aidanhs
Copy link
Contributor

@aidanhs aidanhs commented Dec 30, 2023

cc #2622

First in a series: #3045 (this pr), #3032, #3043

This PR contains targeted improvements to reflow performance. Gives about 3x improvement on a 1MB line, significantly more on a 10MB line (because of quadratic behavior).

I highly recommend reviewing these changes commit-by-commit, as I've carefully tried to make each one standalone and self-consistent.

next_lines is always consolidated to a single Row, which immediately
gets removed - we can remove some dead code as a result
Given a 1MB line catted into the terminal, a toggle-fullscreen +
toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s
Given a 10MB line catted into the terminal, a toggle-fullscreen +
toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s
@imsnif
Copy link
Member

imsnif commented Jan 18, 2024

This looks great. Thank you for your patience and for breaking this down into small chunks. This is very old code that has not seen as much attention as I would have liked.

Great work, I'm going to also take a look at the other PRs.

@imsnif imsnif merged commit ed8ca93 into zellij-org:main Jan 18, 2024
6 checks passed
@aidanhs aidanhs deleted the aphs-improve-reflow-perf-0 branch January 18, 2024 15:53
xuanyuan300 pushed a commit to trysthout/zellij that referenced this pull request Jan 23, 2024
… O(n^2) behavior (zellij-org#3045)

* refactor: Simplify transfer_rows_from_viewport_to_lines_above

next_lines is always consolidated to a single Row, which immediately
gets removed - we can remove some dead code as a result

* perf: Batch remove rows from the viewport for performance

Given a 1MB line catted into the terminal, a toggle-fullscreen +
toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s

* perf: Optimize Row::drain_until by splitting chars in one step

Given a 10MB line catted into the terminal, a toggle-fullscreen +
toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s
@giyany
Copy link
Member

giyany commented Mar 19, 2024

Thank you, I have been waiting for this improvement for literally over a year.

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

Successfully merging this pull request may close these issues.

3 participants