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

Implement Memoization to Speed Up Textarea Rendering #427

Merged

Conversation

wesen
Copy link
Contributor

@wesen wesen commented Nov 1, 2023

This pull request introduces a significant performance improvement to the
textarea rendering process by implementing memoization to cache the results of
the wrapping computation. This change reduces the computational overhead
associated with repeated calculations, particularly for larger text areas.

This at least partially fixes charmbracelet/bubbletea#831
although insertion and rewrapping are still "suboptimal". I think it does the job
quite nicely in practice.

Key Changes:

  1. Introduction of memoization.go: This new file contains the
    implementation of a generic memoization cache with a Least Recently Used
    (LRU) eviction policy. The cache is thread-safe and can be used with any type
    that implements the Hasher interface.

  2. Addition of MemoCache to Model struct: The Model struct in
    textarea.go now includes a MemoCache instance for caching the results of
    the wrapping computation.

  3. Modification of wrap function calls: All calls to the wrap function
    in textarea.go have been replaced with calls to the new memoizedWrap
    function. This function checks the cache for a precomputed result before
    falling back to the wrap function if necessary.

  4. Addition of WrapInput struct and Hash method: The WrapInput struct
    and its associated Hash method have been added to facilitate the hashing of
    inputs to the wrapping computation for cache storage.

Important: I reused my generic Memoization which bumps go to 1.18. I'm not sure how consequent it is to introduce that kind of change incidentally, but didn't want to remove the generics for now. I can "instantiate" it all to string if sticking with 1.17 is desired.

Disclaimer: I'm pretty bullish on LLMs for programming. The memoization code, the fuzzer and the unit tests are heavily copiloted and GPT4-ed. I haven't tested the concurrency of the data structure explicitly, I'm happy to add a bunch of tests and fuzzers for that as well.

@wesen wesen force-pushed the bug/831/slow-textarea-performance branch 2 times, most recently from 3319f7c to 637866f Compare November 1, 2023 03:10
@nadimkobeissi
Copy link

Please merge this!

@wesen
Copy link
Contributor Author

wesen commented Dec 28, 2023

Please merge this!

You can try to use https://github.com/go-go-golems/bobatea which is my alternative version, until it's merged. I haven't tried it out outside of my go workspace, so I hope it works for you.

@maaslalani
Copy link
Contributor

Hi! Thank so much for this PR, I am going to test this out very soon so we can get this in! We really appreciate the work on this one.

@meowgorithm
Copy link
Member

Note that this would close #456.

Also note that this bumps Bubbles from Go v1.17 to v1.18 — which I don't think is an issue, but let us know if you feel differently @muesli.

@maaslalani maaslalani force-pushed the bug/831/slow-textarea-performance branch from 90254c4 to a8d6c52 Compare January 22, 2024 16:44
@maaslalani
Copy link
Contributor

maaslalani commented Jan 22, 2024

Really solid work @wesen. Thank you so much for these improvements! I noticed a ton of performance benefits with this.

One thing I did change (when testing with 1000 lines of text) it was still slow until I changed the cache capacity to match the number of lines. Instead of hardcoding this, I make it so that the capacity matches the MaxHeight of the textarea so we can support the performance at large MaxHeights as well.

@maaslalani maaslalani merged commit 6fe92f9 into charmbracelet:master Jan 22, 2024
9 checks passed
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.

Textarea is slow when pasting things from the clipboard
4 participants