Skip to content

Commit

Permalink
Improve render performance (#3686)
Browse files Browse the repository at this point in the history
- **PR Description**

Fix a performance regression that I introduced with v0.41: when entering
or leaving staging mode for a file, or when switching from a file that
has only unstaged changes to one that has both staged and unstaged
changes, there was a noticeable lag of about 500ms on my machine. With
the improvements in this PR we get this back down to about 20ms.
  • Loading branch information
stefanhaller authored Jun 23, 2024
2 parents a62a508 + 26132cf commit cf40a5b
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 26 deletions.
6 changes: 3 additions & 3 deletions pkg/gui/context/base_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type BaseContext struct {
focusable bool
transient bool
hasControlledBounds bool
needsRerenderOnWidthChange bool
needsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel
needsRerenderOnHeightChange bool
highlightOnFocus bool

Expand All @@ -46,7 +46,7 @@ type NewBaseContextOpts struct {
Transient bool
HasUncontrolledBounds bool // negating for the sake of making false the default
HighlightOnFocus bool
NeedsRerenderOnWidthChange bool
NeedsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel
NeedsRerenderOnHeightChange bool

OnGetOptionsMap func() map[string]string
Expand Down Expand Up @@ -201,7 +201,7 @@ func (self *BaseContext) HasControlledBounds() bool {
return self.hasControlledBounds
}

func (self *BaseContext) NeedsRerenderOnWidthChange() bool {
func (self *BaseContext) NeedsRerenderOnWidthChange() types.NeedsRerenderOnWidthChangeLevel {
return self.needsRerenderOnWidthChange
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/branches_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewBranchesContext(c *ContextCommon) *BranchesContext {
Key: LOCAL_BRANCHES_CONTEXT_KEY,
Kind: types.SIDE_CONTEXT,
Focusable: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES,
})),
ListRenderer: ListRenderer{
list: viewModel,
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/local_commits_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext {
Key: LOCAL_COMMITS_CONTEXT_KEY,
Kind: types.SIDE_CONTEXT,
Focusable: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES,
NeedsRerenderOnHeightChange: true,
})),
ListRenderer: ListRenderer{
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/reflog_commits_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func NewReflogCommitsContext(c *ContextCommon) *ReflogCommitsContext {
Key: REFLOG_COMMITS_CONTEXT_KEY,
Kind: types.SIDE_CONTEXT,
Focusable: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES,
})),
ListRenderer: ListRenderer{
list: viewModel,
Expand Down
1 change: 0 additions & 1 deletion pkg/gui/context/remote_branches_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func NewRemoteBranchesContext(
Kind: types.SIDE_CONTEXT,
Focusable: true,
Transient: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnHeightChange: true,
})),
ListRenderer: ListRenderer{
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/context/sub_commits_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func NewSubCommitsContext(
Kind: types.SIDE_CONTEXT,
Focusable: true,
Transient: true,
NeedsRerenderOnWidthChange: true,
NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES,
NeedsRerenderOnHeightChange: true,
})),
ListRenderer: ListRenderer{
Expand Down
7 changes: 3 additions & 4 deletions pkg/gui/controllers/helpers/window_arrangement_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/jesseduffield/lazygit/pkg/config"
"github.com/jesseduffield/lazygit/pkg/gui/types"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/mattn/go-runewidth"
"golang.org/x/exp/slices"
)

Expand Down Expand Up @@ -272,7 +271,7 @@ func infoSectionChildren(args WindowArrangementArgs) []*boxlayout.Box {
return []*boxlayout.Box{
{
Window: "searchPrefix",
Size: runewidth.StringWidth(args.SearchPrefix),
Size: utils.StringWidth(args.SearchPrefix),
},
{
Window: "search",
Expand Down Expand Up @@ -325,7 +324,7 @@ func infoSectionChildren(args WindowArrangementArgs) []*boxlayout.Box {
// app status appears very briefly in demos and dislodges the caption,
// so better not to show it at all
if args.AppStatus != "" {
result = append(result, &boxlayout.Box{Window: "appStatus", Size: runewidth.StringWidth(args.AppStatus)})
result = append(result, &boxlayout.Box{Window: "appStatus", Size: utils.StringWidth(args.AppStatus)})
}
}

Expand All @@ -338,7 +337,7 @@ func infoSectionChildren(args WindowArrangementArgs) []*boxlayout.Box {
&boxlayout.Box{
Window: "information",
// unlike appStatus, informationStr has various colors so we need to decolorise before taking the length
Size: runewidth.StringWidth(utils.Decolorise(args.InformationStr)),
Size: utils.StringWidth(utils.Decolorise(args.InformationStr)),
})
}

Expand Down
27 changes: 26 additions & 1 deletion pkg/gui/controllers/screen_mode_actions.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controllers

import (
"github.com/jesseduffield/gocui"
"github.com/jesseduffield/lazygit/pkg/gui/types"
)

Expand All @@ -16,7 +17,7 @@ func (self *ScreenModeActions) Next() error {
),
)

return nil
return self.rerenderViewsWithScreenModeDependentContent()
}

func (self *ScreenModeActions) Prev() error {
Expand All @@ -27,9 +28,33 @@ func (self *ScreenModeActions) Prev() error {
),
)

return self.rerenderViewsWithScreenModeDependentContent()
}

// these views need to be re-rendered when the screen mode changes. The commits view,
// for example, will show authorship information in half and full screen mode.
func (self *ScreenModeActions) rerenderViewsWithScreenModeDependentContent() error {
for _, context := range self.c.Context().AllList() {
if context.NeedsRerenderOnWidthChange() == types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES {
if err := self.rerenderView(context.GetView()); err != nil {
return err
}
}
}

return nil
}

func (self *ScreenModeActions) rerenderView(view *gocui.View) error {
context, ok := self.c.Helpers().View.ContextForView(view.Name())
if !ok {
self.c.Log.Errorf("no context found for view %s", view.Name())
return nil
}

return context.HandleRender()
}

func nextIntInCycle(sl []types.WindowMaximisation, current types.WindowMaximisation) types.WindowMaximisation {
for i, val := range sl {
if val == current {
Expand Down
7 changes: 3 additions & 4 deletions pkg/gui/information_panel.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/jesseduffield/lazygit/pkg/constants"
"github.com/jesseduffield/lazygit/pkg/gui/style"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/mattn/go-runewidth"
)

func (gui *Gui) informationStr() string {
Expand Down Expand Up @@ -34,7 +33,7 @@ func (gui *Gui) handleInfoClick() error {
width, _ := view.Size()

if activeMode, ok := gui.helpers.Mode.GetActiveMode(); ok {
if width-cx > runewidth.StringWidth(gui.c.Tr.ResetInParentheses) {
if width-cx > utils.StringWidth(gui.c.Tr.ResetInParentheses) {
return nil
}
return activeMode.Reset()
Expand All @@ -43,10 +42,10 @@ func (gui *Gui) handleInfoClick() error {
var title, url string

// if we're not in an active mode we show the donate button
if cx <= runewidth.StringWidth(gui.c.Tr.Donate) {
if cx <= utils.StringWidth(gui.c.Tr.Donate) {
url = constants.Links.Donate
title = gui.c.Tr.Donate
} else if cx <= runewidth.StringWidth(gui.c.Tr.Donate)+1+runewidth.StringWidth(gui.c.Tr.AskQuestion) {
} else if cx <= utils.StringWidth(gui.c.Tr.Donate)+1+utils.StringWidth(gui.c.Tr.AskQuestion) {
url = constants.Links.Discussions
title = gui.c.Tr.AskQuestion
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/gui/layout.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (gui *Gui) layout(g *gocui.Gui) error {
}

mustRerender := false
if context.NeedsRerenderOnWidthChange() {
if context.NeedsRerenderOnWidthChange() == types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES {
// view.Width() returns the width -1 for some reason
oldWidth := view.Width() + 1
newWidth := dimensionsObj.X1 - dimensionsObj.X0 + 2*frameOffset
Expand Down
6 changes: 3 additions & 3 deletions pkg/gui/presentation/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func getBranchDisplayStrings(
// Recency is always three characters, plus one for the space
availableWidth := viewWidth - 4
if len(branchStatus) > 0 {
availableWidth -= runewidth.StringWidth(utils.Decolorise(branchStatus)) + 1
availableWidth -= utils.StringWidth(utils.Decolorise(branchStatus)) + 1
}
if icons.IsIconEnabled() {
availableWidth -= 2 // one for the icon, one for the space
Expand All @@ -65,7 +65,7 @@ func getBranchDisplayStrings(
availableWidth -= utils.COMMIT_HASH_SHORT_SIZE + 1
}
if checkedOutByWorkTree {
availableWidth -= runewidth.StringWidth(worktreeIcon) + 1
availableWidth -= utils.StringWidth(worktreeIcon) + 1
}

displayName := b.Name
Expand All @@ -79,7 +79,7 @@ func getBranchDisplayStrings(
}

// Don't bother shortening branch names that are already 3 characters or less
if runewidth.StringWidth(displayName) > max(availableWidth, 3) {
if utils.StringWidth(displayName) > max(availableWidth, 3) {
// Never shorten the branch name to less then 3 characters
len := max(availableWidth, 4)
displayName = runewidth.Truncate(displayName, len, "…")
Expand Down
16 changes: 14 additions & 2 deletions pkg/gui/types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ type ParentContexter interface {
GetParentContext() (Context, bool)
}

type NeedsRerenderOnWidthChangeLevel int

const (
// view doesn't render differently when its width changes
NEEDS_RERENDER_ON_WIDTH_CHANGE_NONE NeedsRerenderOnWidthChangeLevel = iota
// view renders differently when its width changes. An example is a view
// that truncates long lines to the view width, e.g. the branches view
NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES
// view renders differently only when the screen mode changes
NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_SCREEN_MODE_CHANGES
)

type IBaseContext interface {
HasKeybindings
ParentContexter
Expand All @@ -60,8 +72,8 @@ type IBaseContext interface {
// determined independently.
HasControlledBounds() bool

// true if the view needs to be rerendered when its width changes
NeedsRerenderOnWidthChange() bool
// to what extent the view needs to be rerendered when its width changes
NeedsRerenderOnWidthChange() NeedsRerenderOnWidthChangeLevel

// true if the view needs to be rerendered when its height changes
NeedsRerenderOnHeightChange() bool
Expand Down
19 changes: 16 additions & 3 deletions pkg/utils/formatting.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package utils
import (
"fmt"
"strings"
"unicode"

"github.com/mattn/go-runewidth"
"github.com/samber/lo"
Expand All @@ -21,10 +22,22 @@ type ColumnConfig struct {
Alignment Alignment
}

func StringWidth(s string) int {
// We are intentionally not using a range loop here, because that would
// convert the characters to runes, which is unnecessary work in this case.
for i := 0; i < len(s); i++ {
if s[i] > unicode.MaxASCII {
return runewidth.StringWidth(s)
}
}

return len(s)
}

// WithPadding pads a string as much as you want
func WithPadding(str string, padding int, alignment Alignment) string {
uncoloredStr := Decolorise(str)
width := runewidth.StringWidth(uncoloredStr)
width := StringWidth(uncoloredStr)
if padding < width {
return str
}
Expand Down Expand Up @@ -144,7 +157,7 @@ func getPadWidths(stringArrays [][]string) []int {
return MaxFn(stringArrays, func(stringArray []string) int {
uncoloredStr := Decolorise(stringArray[i])

return runewidth.StringWidth(uncoloredStr)
return StringWidth(uncoloredStr)
})
})
}
Expand All @@ -161,7 +174,7 @@ func MaxFn[T any](items []T, fn func(T) int) int {

// TruncateWithEllipsis returns a string, truncated to a certain length, with an ellipsis
func TruncateWithEllipsis(str string, limit int) string {
if runewidth.StringWidth(str) > limit && limit <= 2 {
if StringWidth(str) > limit && limit <= 2 {
return strings.Repeat(".", limit)
}
return runewidth.Truncate(str, limit, "…")
Expand Down
25 changes: 25 additions & 0 deletions pkg/utils/formatting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"strings"
"testing"

"github.com/mattn/go-runewidth"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -250,3 +251,27 @@ func TestRenderDisplayStrings(t *testing.T) {
assert.EqualValues(t, test.expectedColumnPositions, columnPositions)
}
}

func BenchmarkStringWidthAsciiOriginal(b *testing.B) {
for i := 0; i < b.N; i++ {
runewidth.StringWidth("some ASCII string")
}
}

func BenchmarkStringWidthAsciiOptimized(b *testing.B) {
for i := 0; i < b.N; i++ {
StringWidth("some ASCII string")
}
}

func BenchmarkStringWidthNonAsciiOriginal(b *testing.B) {
for i := 0; i < b.N; i++ {
runewidth.StringWidth("some non-ASCII string 🍉")
}
}

func BenchmarkStringWidthNonAsciiOptimized(b *testing.B) {
for i := 0; i < b.N; i++ {
StringWidth("some non-ASCII string 🍉")
}
}

0 comments on commit cf40a5b

Please sign in to comment.