Textarea is slow when pasting things from the clipboard

[cruizba]

Describe the bug
Copy and paste things in textarea breaks somehow the fast user experience while using it in a CLI tool.
When you paste a large text into a textarea, all characters are written one by one.
Also, I've noticed that, the larger the text is and the more number of lines it has, user input slows down incrementally.

Setup
Please complete the following information along with version numbers, if applicable.

• OS: Ubuntu
• Shell: zsh
• Terminal Emulator: Terminator

To Reproduce
Steps to reproduce the behavior:

1. Execute the text area tutorial, change the CharLimit of the TextArea to a high value (so large texts can be pasted) and paste a large file. I am actually pasting a PEM file (a certificate) which has 28 lines and 65 characters per line.
2. See how the text is being copied character by character, and it slowed down the more characters are currently in the textarea

Source Code
https://github.com/charmbracelet/bubbletea/tree/master/examples/textarea

Expected behavior
It should be fast on pasting things to not break the user experience.

[meowgorithm]

Hi! Thanks for the report. I believe there two items that will improve the situation here:

• Bracketed paste (feat: bracketed paste #397)
• A more optimized underlying data structure on textarea, like a rope (Performance of textarea bubbles#301)

[cruizba]

Oh, I did not see there were current issues and PRs about this 🤔

Amazing project BTW 🦾

[wesen]

I have "hacked" textarea to be significantly faster. After profiling, I found wrap() to be the biggest culprit, as it is called by LineInfo which in turn is called by the CursorPosition method.

Here is a sketch of the solution, I built a LRU memoization cache here: https://github.com/go-go-golems/clay/blob/main/pkg/memoization/memoization.go

Here is the "hacked" textarea: wesen/bubbles@30a4341

I was able to easily copy paste longer sections of text: https://asciinema.org/a/Dab4MlcIxaWinhJwQC3kU9fCV

[cruizba]

@wesen That's actually a clever solution without changing anything of the rest of the bubbletea source code.

Why don't you do a PR and include the memoization.go file in the PR (maybe they don't want to add more dependencies into the project)?

I am not a mantainer of the library or anything, just a regular user, but from my point of view this does not looks hard to integrate. It could be a temporal solution before they integrate this PR (#397)

@meowgorithm what do you think?

[decentral1se]

Yeh, just chiming to say that textarea copy/pasta is not really that usable at all atm due to this performance issue. Thanks for raising the issue!

[cruizba]

Bumping

[nadimkobeissi]

Please merge #427!

[meowgorithm]

Hey everyone. Just a note that we're looking into bracketed paste (#397) which should alleviate some of the performance woes. It's a delicate one to execute on properly, and will need some additional changes beyond that PR in practice, so note we're examining the solution carefully.

@nadimkobeissi #427 was merged a long time ago. Perhaps a typo?

[mikelorant]

Have been doing profiling of this issue and discovered a very simple change that causes a nearly doubling of performance. Swapping out the StringWidth function from go-runewidth to the same function in uniseg. Since go-runewidth uses uniseg under the hood, this does not have a change on the imports (just requires a bump to uniseg 0.4.4.

Benchmark results

Function	Count	Speed
BenchmarkStringWidthRunewidth-4	3397	325152 ns/op
BenchmarkStringWidthUniseg-4	12072	101341 ns/op

Code used for benchmarks:

func BenchmarkStringWidthMatt(b *testing.B) {
    loremIpsumGenerator := loremipsum.NewWithSeed(1234)
    str := loremIpsumGenerator.Paragraph()

    for i := 0; i < b.N; i++ {
        runewidth.StringWidth(str)
    }
}

func BenchmarkStringWidthUniseg(b *testing.B) {
    loremIpsumGenerator := loremipsum.NewWithSeed(1234)
    str := loremIpsumGenerator.Paragraph()

    for i := 0; i < b.N; i++ {
        uniseg.StringWidth(str)
    }
}

Iteration count was only 1 so this was testing StringWidth only.

I would also recommend replacing the RuneWidth function, however, uniseg has not made this public. I have requested for this function to be changed from private to public so it could also be used. This would then allow go-runewidth to be dropped entirely.

Be aware, for the full benefits, I would recommend this change being applied to lipgloss and reflow as well.

I have no problems putting up the pull requests for all impacted repositories, however I would like confirmation this aligns with the project directions before I begin the work.

[meowgorithm]

Well that's awesome. We've noticed rivo/uniseg added a StringWidth to the API but weren't aware the benefits are so great. We'll submit PRs accordingly for Bubbles, Lip Gloss, reflow and termenv today.

[maaslalani]

We just merged memoization to improve textarea performance: charmbracelet/bubbles#427

Thank you for the well-written issue @cruizba and for the well-done implementation @wesen!

[maaslalani]

Well that's awesome. We've noticed rivo/uniseg added a StringWidth to the API but weren't aware the benefits are so great. We'll submit PRs accordingly for Bubbles, Lip Gloss, reflow and termenv today.

@mikelorant, thanks so much for the information and benchmarks. We've swapped in uniseg for Bubbles (textinput and textarea) and have opened PRs for termenv and reflow!

• Use uniseg.StringWidth bubbles#462
• Use uniseg.StringWidth muesli/termenv#164
• perf: use uniseg.StringWidth muesli/reflow#69

When the RuneWidth function becomes available to use, we will swap those out too, if it performance is improved!

[mikelorant]

Unfortunately, RuneWidth adds significantly to this complexity. @rivo has provided some detailed feedback on why using this function causes problems.

That's because a single code point (rune) is not always a complete character. That's the whole basis of the uniseg package. The README explains this in detail. One example given there is the country flag emoji. These emojis must always consist of two runes. What's the width of only one of these runes then? It's basically undefined because it's an incomplete grapheme cluster. The mattn/go-runewidth package gets this fundamentally wrong and I have tried for a long time to help them do it right but there was never much interest in following up on it.
rivo/uniseg#48 (comment)

However, for the sake of complete benchmarks I have compared the two packages.

Function	Count	Speed
BenchmarkRuneWidthRunewidth-4	335290	3287 ns/op
BenchmarkRuneWidthUniseg-4	24157	49827 ns/op

Code used for benchmarks:

func BenchmarkRuneWidthRunewidth(b *testing.B) {
    loremIpsumGenerator := loremipsum.NewWithSeed(1234)
    str := loremIpsumGenerator.Paragraph()

    for i := 0; i < b.N; i++ {
        for _, r := range str {
            runewidth.RuneWidth(r)
        }
    }
}

func BenchmarkRuneWidthUniseg(b *testing.B) {
    loremIpsumGenerator := loremipsum.NewWithSeed(1234)
    str := loremIpsumGenerator.Paragraph()

    for i := 0; i < b.N; i++ {
        for _, r := range str {
            uniseg.StringWidth(string(r))
        }
    }
}

The easy answer, there is no benefit to swap out the RuneWidth function.

The complicated answer, we should never be using go-runewidth RuneWidth anywhere as it is a fundamentally broken approach.

I do not wish to add to this closed issue with further details on how we should approach the replacement of RuneWidth so will open a new issue for discussion.

[rivo]

@mikelorant I've spent some time today working on uniseg's performance. You may want to check out the current version (v0.4.6). While it may still not be as fast as go-runewidth's RuneWidth function (which is expected, as it doesn't handle grapheme clusters), you should see large improvements.

For the reasons I mentioned in the comment you quoted above, I would advise against handling runes in isolation, as you do in the BenchmarkRuneWidthUniseg function. You'll want to do it like in this example. Every iteration of the loop will give you a full "character" (i.e. grapheme cluster), along with its width.

[mikelorant]

@rivo Can confirm some very nice performance benefits.

I've also taken the opportunity to continue to do some CPU profiling and think you may have 2 places worth investigating if you want to squeeze some more improvements in.

I may be very wrong about this (fairly new to CPU profiling) so I may not be reading this correctly.

[rivo]

The property function is already a binary search, while fast-tracking ASCII characters. I don't see how this can be improved.

It seems to me that you're still using the Graphemes class. As mentioned in my other comments (and now also in the package comments), this class implements all functionality of the package. Do you really need sentence boundaries, word boundaries, and line break determination? If you're really just interested in grapheme clusters and widths, you'll want to switch to FirstGraphemeClusterInString. It'll save you a lot more CPU cycles.

[mikelorant]

Will reply to you on the reflow issue as it will have more context with the other benchmarks. Good news though, performance is better than it originally was!

[mikelorant]

Absolutely giant increase in performance with muesli/reflow#71. All thanks to @rivo for this one.

@wesen You might want to try updating your reflow to the branch I pushed. I think combining both your pull request and mine we may have dealt with most of the issues. I still think more improvements are possible but this seems like a pretty big jump compared to how it performed before.

[mikelorant]

Just did some very quick benchmarks of my own app with pasting 1000 characters (14 lines) into a textarea of 72 characters wide:

Before: 42s
After: 3s

For context, no optimisations to my own app code at all. All these performance increases came from:

• bubbles
• lipgloss
• reflow
• uniseg

Very happy with the results.