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

Circular / Ring Buffer for Text Widget #293

Open
timbutler opened this issue Feb 12, 2021 · 3 comments
Open

Circular / Ring Buffer for Text Widget #293

timbutler opened this issue Feb 12, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@timbutler
Copy link

I've started using this for a quick internal project, with one of features being log files monitored and displayed in real-time while other stats are displayed.

While this is working, one issue we've of course hit is when verbose logs are monitored that the size of the text buffer keeps increasing and of course leads to high memory usage and performance degradation.

A possible fix to this is to implement a simple ring buffer, where the Text content has an (optional) maximum it can grow to. During the Write call it could then pop content off the top of the buffer as required so that it never grows further than the maximum content.

I've added this as a basic proof of concept internally, but wanted to confirm if this would be the best approach (or if I'm missing something obvious!) before submitting any pull request.

@mum4k mum4k added the question Further information is requested label Feb 12, 2021
@mum4k
Copy link
Owner

mum4k commented Feb 12, 2021

Hi @timbutler, you raise a good point about the memory usage when the content of the Text widget just grows.

Adding an optional circular buffer sounds like a great idea. Such PR would be a great addition to the Text widget, if you're willing to submit it. Please see CONTRIBUTING.md for additional instructions (e.g. which branch to fork from).

Don't hesitate to let me know if you have any questions or would like to discuss the API change upfront.

@timbutler
Copy link
Author

Thanks for the rapid feedback @mum4k! I'll tidy my quick edits and submit a PR in the next few days for further feedback.

@mum4k
Copy link
Owner

mum4k commented Feb 12, 2021

Sounds great, looking forward to it @timbutler.

@mum4k mum4k added enhancement New feature or request and removed question Further information is requested labels Feb 12, 2021
mum4k pushed a commit that referenced this issue Apr 3, 2021
See issue #293 where memory and performance can degrade with a high number of lines written to the Text widget. 

This is a very simplistic implementation to limit the possible length the text buffer can grow to with the `maxContent` option. 

Default value of -1 means there's no limit and therefore behaviour should remain standard.

It has been working in our test app and allows the use of the Text widget to monitor logs (ie tail) and therefore doesn't bloat over time, but happy to adjust as required.
mum4k added a commit that referenced this issue Apr 3, 2021
See issue #293 where memory and performance can degrade with a high number of lines written to the Text widget. 

This is a very simplistic implementation to limit the possible length the text buffer can grow to with the `maxContent` option. 

Default value of -1 means there's no limit and therefore behaviour should remain standard.

It has been working in our test app and allows the use of the Text widget to monitor logs (ie tail) and therefore doesn't bloat over time, but happy to adjust as required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants