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

Improved performance with large scrollback #357

Conversation

Kotivskyi
Copy link

@Kotivskyi Kotivskyi commented Dec 2, 2024

Improved performance for terminal will large scrollback values (over 10000 lines), so memory will not be allocated for buffers that are not used

  • Implemented optimisation for BufferLine, so data array will not be allocated until it is needed
  • modified resize method to be more efficient if data are not allocated yet

Problem background:
During initialization and window resize Terminal will call resize method for each buffer line during which triggers memory reallocation of BufferLine.data. This operation becomes bottleneck with terminal scrollback > 10000.

Screenshot 2024-12-02 at 17 57 25

Implemented fix takes into account that most of the buffers will contains only blank charactes (until something useful will be printed into it) and can be lazy allocated using known fillCharacter and expectedDataSize. In this case resize method will be as simple as updating expectedDataSize value.

Vitalii Kotivskyi added 2 commits December 2, 2024 17:42
…emory will not be allocated for buffers that are not used

- Implemented optimisation for BufferLine, so data array will not be allocated until it is needed
- modified resize method to be more efficient if data are not allocated yet
@Kotivskyi
Copy link
Author

Kotivskyi commented Dec 2, 2024

Note: Further optimization is possible since most of the lines in CircularList will be empty BufferLines and we don't really need them before they contain anything useful.

@migueldeicaza
Copy link
Owner

Thank you very much, this is great!

Another option is to only allocate the rows on demand, rather than allocating up-front and having the the scroll back be a limit to how much it grows, rather than a startup allocation size.

But I will merge your change until the day I fix that

@migueldeicaza migueldeicaza merged commit b589a97 into migueldeicaza:main Dec 3, 2024
1 check failed
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.

2 participants