gdamore / tcell

Tcell is an alternate terminal package, similar in some ways to termbox, but better in others.
Apache License 2.0
4.59k stars 310 forks source link

Speed up SetContent by checking length of combining characters before reflect.DeepEqual #744

Closed kivattt closed 3 weeks ago

kivattt commented 3 months ago

Adding these length checks before the reflect.DeepEqual speeds up SetContent when not drawing Unicode combining characters

With xset r rate 300 255 (key repeating 255 times per second), I recorded myself scrolling through 2000 files in my file manager project where I'm using tview with and without (left) this change:

https://github.com/user-attachments/assets/9a4f8f83-7a8d-4b5f-b368-8687c53e1ad3

(6x sped up)

image

I believe the performance difference is a lot less dramatic for most projects, since in this old version of my file manager the screen was always unnecessarily being cleared with Box.DrawForSubclass() in tview

kivattt commented 4 weeks ago

With the way I'm testing it, I see no difference with your change, but it's certainly easier to read and I'm not that familiar with the problem

kivattt commented 4 weeks ago

Nevermind, I can see a stuck character on the screen when I use your change, I'll send a video demonstrating it

gdamore commented 4 weeks ago

My concern is that there may be cases where the lengths are unequal.  Those need to be treated as dirty. On Oct 6, 2024 at 6:10 PM -0700, kivattt @.***>, wrote:

Nevermind, I can see a stuck character on the screen when I use your change, I'll send a video demonstrating it — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

kivattt commented 4 weeks ago

On second glance, I think you just had the ! in the wrong place, this new commit fixes the stuck character issue I noticed

kivattt commented 4 weeks ago

Nevermind, my commit changes the behaviour, so I think your change is the more correct one and the "stuck" character I'm seeing is not an issue with tcell

kivattt commented 3 weeks ago

I use this to bruteforce check if something changed

package main

import (
        "fmt"
        "math/rand"
        "os"
        "reflect"
        "strconv"
)

func oldOne(c cell, mainc rune, combc []rune) bool {
        return (c.width > 0) && (mainc != c.currMain || !reflect.DeepEqual(combc, c.currComb))
}

func newOne(c cell, mainc rune, combc []rune) bool {
        return (c.width > 0) && (mainc != c.currMain || len(combc) != len(c.currComb) || (len(combc) > 0 && !reflect.DeepEqual(combc, c.currComb)))
}

func randRune() rune {
        return rune(rand.Intn(10)) // Low enough so 2 random runes can match
}

func randRuneSlice() []rune {
        var ret []rune
        upper := rand.Intn(5)
        for i := 1; i < upper; i++ {
                ret = append(ret, randRune())
        }
        return ret
}

type cell struct {
        currMain rune
        currComb []rune
        width    int
}

func main() {
        for i := 0; i < 10000000; i++ {
                c := cell{
                        currMain: randRune(),
                        currComb: randRuneSlice(),
                        width:    rand.Intn(5),
                }
                mainc := randRune()
                combc := randRuneSlice()

                if oldOne(c, mainc, combc) != newOne(c, mainc, combc) {
                        fmt.Println("oldOne and newOne differ!")
                        fmt.Println("c.currMain:", c.currMain)
                        fmt.Println("c.currComb:", c.currComb)
                        fmt.Println("c.width:", c.width)
                        fmt.Println("mainc: " + strconv.Itoa(int(mainc)))
                        fmt.Println("combc:", combc)
                        os.Exit(1)
                }
        }

        fmt.Println("oldOne and newOne match!")
}