rivo / tview

Terminal UI library with rich, interactive widgets — written in Golang
MIT License
10.61k stars 554 forks source link

TextView consumes significant CPU when wrapping is enabled #686

Closed denandz closed 2 years ago

denandz commented 2 years ago

Hello, I'm currently using tview for an interactive HTTP/HTTPS intercept proxy (https://github.com/denandz/glorp) and have come across an issue. In my case, i'd like to display large HTTP responses with word wrapping enabled, which causes issues.

If word wrapping is enabled on a TextView, and a large amount of data is written to the view, then a massive CPU consumption and application lockup until TextView.reindexBuffer returns.

I found out this is due to runewidth.Truncate with large amounts of data. pprof output below, web attached.

(pprof) top10  
Showing nodes accounting for 27290ms, 90.72% of 30080ms total
Dropped 145 nodes (cum <= 150.40ms)
Showing top 10 nodes out of 38
      flat  flat%   sum%        cum   cum%
    7420ms 24.67% 24.67%     7420ms 24.67%  aeshashbody
    6620ms 22.01% 46.68%    16000ms 53.19%  runtime.mapaccess2
    5900ms 19.61% 66.29%     5900ms 19.61%  github.com/rivo/uniseg.property (inline)
    2800ms  9.31% 75.60%    24700ms 82.11%  github.com/rivo/uniseg.(*Graphemes).Next
     980ms  3.26% 78.86%     8840ms 29.39%  runtime.memhash128
     970ms  3.22% 82.08%     2520ms  8.38%  github.com/rivo/uniseg.NewGraphemes
     770ms  2.56% 84.64%      770ms  2.56%  runtime.memclrNoHeapPointers
     670ms  2.23% 86.87%      980ms  3.26%  runtime.scanobject
     620ms  2.06% 88.93%    27550ms 91.59%  github.com/mattn/go-runewidth.(*Condition).StringWidth
     540ms  1.80% 90.72%      540ms  1.80%  unicode/utf8.RuneCountInString

profile001

The following code can be used to replicate this issue, and was used to generate the profile above:

package main

import (
    "fmt"
    "log"
    "math/rand"
    "os"
    "runtime/pprof"
    "time"

    "github.com/rivo/tview"
)

var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")

func randString(n int) string {
    b := make([]rune, n)
    for i := range b {
        b[i] = letters[rand.Intn(len(letters))]
    }
    return string(b)
}

func main() {
    f, err := os.Create("profile.pprof")
    if err != nil {
        log.Fatal(err)
    }
    pprof.StartCPUProfile(f)
    defer pprof.StopCPUProfile()

    rand.Seed(time.Now().UnixNano())

    mainLayout := tview.NewFlex()
    mainLayout.SetDirection(tview.FlexRow)

    app := tview.NewApplication()

    textView1 := tview.NewTextView().SetWrap(true)
    textView1.SetBorder(true)
    fmt.Fprint(textView1, "small stuff is no problem")

    textView2 := tview.NewTextView().SetWrap(true) // if this is set to 'false', no problem
    textView2.SetBorder(true)

    fmt.Fprint(textView2, "aaa\nbbb\neee\ndddd\n\n\n")
    fmt.Fprint(textView2, string(randString(256*1024))) // this is going to cause problems if word wrap is on

    tbl := tview.NewTable()
    tbl.SetFixed(1, 1)
    tbl.SetBorders(false).SetSeparator(tview.Borders.Vertical)

    tbl.SetBorderPadding(0, 0, 0, 0)
    tbl.SetSelectable(true, false)

    flexi := tview.NewFlex()
    flexi.AddItem(textView1, 0, 1, false)
    flexi.AddItem(textView2, 0, 1, false)

    mainLayout.AddItem(tbl, 0, 2, true)
    mainLayout.AddItem(flexi, 0, 3, false)

    if err := app.SetRoot(mainLayout, true).EnableMouse(true).Run(); err != nil {
        panic(err)
    }
}
denandz commented 2 years ago

In my case, I don't really care about rune processing right now and I'm fairly happy just displaying bytes. I was able to fix my issue by tweaking TextView to hack up the string using slices instead.

Would setting a "raw" mode on a TextView be an option? I'd envisage some kind of flag where I could specify NewTextArea.SetRaw(true) which would disable the rune processing and just slam bytes into the TextView with reckless abandon. This might be an easy win?

This was my hacky change to textview.go reindexBuffer, just for reference:

        if t.wrap && len(str) > 0 {
            for len(str) > 0 {
                var extract string
                /* extract := runewidth.Truncate(str, width, "")
                if len(extract) == 0 {
                    // We'll extract at least one grapheme cluster.
                    gr := uniseg.NewGraphemes(str)
                    gr.Next()
                    _, to := gr.Positions()
                    extract = str[:to]
                }*/
                if len(str) > width {
                    extract = str[:width]
                } else {
                    extract = str
                }

If raw mode was going to be implemented, then write would also need to be tweaked to not do its partial trailing utf8 rune logic when raw mode is enabled.

If this is something you'd consider I can take a closer look and try get a PR together.

denandz commented 2 years ago

Another, less hacky option, would be to change the reindexBuffer wrap logic to avoid the use of runewidth.Truncate all together. I tested the following change with an 8MB textview buffer and did not experience the CPU consumption issue (I didn't test the word-wrap functionality, only wrap, FWIW):

for len(str) > 0 {
                //extract := runewidth.Truncate(str, width, "")
                var extract string
                w := 0
                for _, r := range str {
                    cw := runewidth.RuneWidth(r)
                    if w+cw > width {
                        break
                    }
                    extract += string(r)
                    w += cw
                }

                if len(extract) == 0 {
                    // We'll extract at least one grapheme cluster.
                    gr := uniseg.NewGraphemes(str)
                    gr.Next()
                    _, to := gr.Positions()
                    extract = str[:to]
                }

                if t.wordWrap && len(extract) < len(str) {
...yoink...
rivo commented 2 years ago

I'll preface my response by saying that I've mentioned multiple times in other issues that TextView is not suitable for very large texts. You will always run into performance issues such as the one you encountered. I'm actually going to add a note about this in the official TextView documentation because this comes up quite often.

As for your suggestions, I'm afraid that this will not be a route I will take. It all started like your first example. Then I quickly learned that it wasn't that simple. So I implemented your second example. And that wasn't enough either. (Check https://github.com/rivo/uniseg for an explanation.) Dozens of issues and complaints later, we're at an implementation that works with Unicode, different languages, wide characters, even emojis. Of course, all of these requirements from users come with a performance hit. If you find a way to make runewidth.Truncate() faster, I think @mattn and I will be all ears. But tview won't go back to the naive implementation. (And I currently don't intend to write and maintain an entire second implementation for "raw" mode.)

You may want to consider TextView.SetMaxLines() to limit the number of lines in the text view. Or maybe use virtual tabes if random access is required. (But then you won't get word wrapping for free.)

denandz commented 2 years ago

Okay, if you don't intend to address the performance issues in TextView then you can close this issue WONTFIX.

TextView.SetMaxLines() doesn't work for my application, I'd like to be able to view 100 or 200kb of text in a text view without pinning my cpu.

I do have two requests though:

rivo commented 2 years ago

Actually, one other suggestion that just came to mind was that you might experience the high CPU load when you're redrawing the TextView component very frequently. It's probably worth looking into reducing the number of redraws to maybe a few times per second. I haven't seen your implementation but if you update the screen after every io.Write() to the TextView, this might be a bit too much if only a few bytes are written to the text view at a time.

Finally, if your application really only deals with ASCII characters, it shouldn't be very difficult to implement line-breaking yourself and simply pipe that pre-broken-over text into the text view. Then you can turn off wrapping and runewidth.Truncate() will never be called.

denandz commented 2 years ago

You can replicate this issue using the original code snippet in the original issue message, which has no calls to draw on each io.Write(). It's fairly simple and should demonstrate the performance hits.

I can figure out a solution for my specific use case that doesn't involve changing anything in tview

denandz commented 2 years ago

In case anyone stumbles across this issue, I ended up implementing a very basic line wrapped text view primitive here: https://github.com/denandz/glorp/blob/master/views/textprimitive.go

Multi-megabyte buffers can be handled in a reasonable time frame, but none of the other TextView functionality is implemented.