Skip to content

Commit

Permalink
Optimise SetContent
Browse files Browse the repository at this point in the history
* Remove reflect.DeepEqual call in SetContent
  > There's no need for reflection as we know the types.
  > Given tcell wants to stay compatible with older go versions, we now have
  > a small function that handles this, instead of using something like
  > `slices.Equal`.
* Add Simple benchmark to compare against the old versions
* Reuse `currComb` and `lastComb` slices to prevent allocations
* Optimise `SetDirty` calls in `SetContent`, rdDon't re-lookup first cell
  > This should be an improvent for ascii only rendering at least
  • Loading branch information
Bios-Marcel committed Jan 18, 2025
1 parent 7815866 commit 3449fdf
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 14 deletions.
50 changes: 36 additions & 14 deletions cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package tcell

import (
"os"
"reflect"

runewidth "github.com/mattn/go-runewidth"
)
Expand All @@ -32,6 +31,19 @@ type cell struct {
lock bool
}

func (c *cell) setDirty(dirty bool) {
if dirty {
c.lastMain = rune(0)
} else {
if c.currMain == rune(0) {
c.currMain = ' '
}
c.lastMain = c.currMain
c.lastComb = append(c.lastComb[:0], c.currComb...)
c.lastStyle = c.currStyle
}
}

// CellBuffer represents a two-dimensional array of character cells.
// This is primarily intended for use by Screen implementors; it
// contains much of the common code they need. To create one, just
Expand All @@ -44,6 +56,21 @@ type CellBuffer struct {
cells []cell
}

// we purposefully don't use slices.Equal in order to stay compatible
// with earlier go versions.
func runeSliceEqual(a, b []rune) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}

return true
}

// SetContent sets the contents (primary rune, combining runes,
// and style) for a cell at a given location. If the background or
// foreground of the style is set to ColorNone, then the respective
Expand All @@ -58,13 +85,17 @@ func (cb *CellBuffer) SetContent(x int, y int,
// dirty as well as the base cell, to make sure we consider
// both cells as dirty together. We only need to do this
// if we're changing content
if (c.width > 0) && (mainc != c.currMain || len(combc) != len(c.currComb) || (len(combc) > 0 && !reflect.DeepEqual(combc, c.currComb))) {
for i := 0; i < c.width; i++ {
if c.width > 0 && (mainc != c.currMain || !runeSliceEqual(combc, c.currComb)) {
// Prevent unnecessary boundchecks for first cell, since we already
// received that one.
c.setDirty(true)
for i := 1; i < c.width; i++ {
cb.SetDirty(x+i, y, true)
}
}

c.currComb = append([]rune{}, combc...)
// Reuse slice to prevent allocations
c.currComb = append(c.currComb[:0], combc...)

if c.currMain != mainc {
c.width = runewidth.RuneWidth(mainc)
Expand Down Expand Up @@ -148,16 +179,7 @@ func (cb *CellBuffer) Dirty(x, y int) bool {
func (cb *CellBuffer) SetDirty(x, y int, dirty bool) {
if x >= 0 && y >= 0 && x < cb.w && y < cb.h {
c := &cb.cells[(y*cb.w)+x]
if dirty {
c.lastMain = rune(0)
} else {
if c.currMain == rune(0) {
c.currMain = ' '
}
c.lastMain = c.currMain
c.lastComb = c.currComb
c.lastStyle = c.currStyle
}
c.setDirty(dirty)
}
}

Expand Down
114 changes: 114 additions & 0 deletions cell_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package tcell

import (
"reflect"
"testing"

runewidth "github.com/mattn/go-runewidth"
)

// SetContent sets the contents (primary rune, combining runes,
// and style) for a cell at a given location. If the background or
// foreground of the style is set to ColorNone, then the respective
// color is left un changed.
func (cb *CellBuffer) SetContentOld(x int, y int,
mainc rune, combc []rune, style Style,
) {
if x >= 0 && y >= 0 && x < cb.w && y < cb.h {
c := &cb.cells[(y*cb.w)+x]

// Wide characters: we want to mark the "wide" cells
// dirty as well as the base cell, to make sure we consider
// both cells as dirty together. We only need to do this
// if we're changing content
if (c.width > 0) && (mainc != c.currMain || len(combc) != len(c.currComb) || (len(combc) > 0 && !reflect.DeepEqual(combc, c.currComb))) {
for i := 0; i < c.width; i++ {
cb.SetDirty(x+i, y, true)
}
}

c.currComb = append([]rune{}, combc...)

if c.currMain != mainc {
c.width = runewidth.RuneWidth(mainc)
}
c.currMain = mainc
if style.fg == ColorNone {
style.fg = c.currStyle.fg
}
if style.bg == ColorNone {
style.bg = c.currStyle.bg
}
c.currStyle = style
}
}

func Benchmark_SetContentOld_ascii(b *testing.B) {
buffer := &CellBuffer{}
buffer.Resize(100, 100)
for i := 0; i < b.N; i++ {
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContentOld(w, h, 'a', nil, StyleDefault)
}
}
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContentOld(w, h, 'b', nil, StyleDefault)
}
}
}
}

func Benchmark_SetContent_ascii(b *testing.B) {
buffer := &CellBuffer{}
buffer.Resize(100, 100)
for i := 0; i < b.N; i++ {
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContent(w, h, 'a', nil, StyleDefault)
}
}
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContent(w, h, 'b', nil, StyleDefault)
}
}
}
}

func Benchmark_SetContentOld(b *testing.B) {
buffer := &CellBuffer{}
buffer.Resize(100, 100)
flag := []rune("🇦🇺")
for i := 0; i < b.N; i++ {
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContentOld(w, h, 'a', nil, StyleDefault)
}
}
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContentOld(w, h, flag[0], flag[1:], StyleDefault)
}
}
}
}

func Benchmark_SetContent(b *testing.B) {
buffer := &CellBuffer{}
buffer.Resize(100, 100)
flag := []rune("🇦🇺")
for i := 0; i < b.N; i++ {
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContent(w, h, 'a', nil, StyleDefault)
}
}
for w := 0; w < 100; w++ {
for h := 0; h < 100; h++ {
buffer.SetContent(w, h, flag[0], flag[1:], StyleDefault)
}
}
}
}

0 comments on commit 3449fdf

Please sign in to comment.