From 8e1464f720d839b6e2c493b8fead0e4d39bbaaa7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 22 Jun 2024 17:55:30 +0200 Subject: Don't redraw remote branches view when its width changes The rendering of remote branches is in no way dependent on the width of the view (or the screen mode). Unlike in the local branches view, we don't truncate long branch names here (because there's no more information after them). This is an error introduced in d5b4f7bb3e. --- pkg/gui/context/remote_branches_context.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/gui/context/remote_branches_context.go b/pkg/gui/context/remote_branches_context.go index fff80e076..892953f82 100644 --- a/pkg/gui/context/remote_branches_context.go +++ b/pkg/gui/context/remote_branches_context.go @@ -43,7 +43,6 @@ func NewRemoteBranchesContext( Kind: types.SIDE_CONTEXT, Focusable: true, Transient: true, - NeedsRerenderOnWidthChange: true, NeedsRerenderOnHeightChange: true, })), ListRenderer: ListRenderer{ -- cgit v1.2.3 From a67eda39a56440f54be7fbb43974fa7a966bce53 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 22 Jun 2024 18:00:31 +0200 Subject: Rerender fewer views when their width changes In d5b4f7bb3e and 58a83b0862 we introduced a combined mechanism for rerendering views when either their width changes (needed for the branches view which truncates long branch names), or the screen mode (needed for those views that display more information in half or full screen mode, e.g. the commits view). This was a bad idea, because it unnecessarily rerenders too many views when just their width changes, which causes a noticable lag. This is a problem, for example, when selecting a file in the files panel that has only unstaged changes, and then going to one that has both staged and unstaged changes; this splits the main view, causing the side panels to become a bit narrower, and rerendering all those views took almost 500ms on my machine. Another similar example is entering or leaving staging mode. Fix this by being more specific about which views need rerendering under what conditions; this improves the time it takes to rerender in the above scenarios from 450-500s down to about 20ms. This reintroduces the code that was removed in 58a83b0862, but in a slightly different way. --- pkg/gui/context/base_context.go | 6 +++--- pkg/gui/context/branches_context.go | 2 +- pkg/gui/context/local_commits_context.go | 2 +- pkg/gui/context/reflog_commits_context.go | 2 +- pkg/gui/context/sub_commits_context.go | 2 +- pkg/gui/controllers/screen_mode_actions.go | 27 ++++++++++++++++++++++++++- pkg/gui/layout.go | 2 +- pkg/gui/types/context.go | 16 ++++++++++++++-- 8 files changed, 48 insertions(+), 11 deletions(-) diff --git a/pkg/gui/context/base_context.go b/pkg/gui/context/base_context.go index dfcced021..ca04a2fa9 100644 --- a/pkg/gui/context/base_context.go +++ b/pkg/gui/context/base_context.go @@ -23,7 +23,7 @@ type BaseContext struct { focusable bool transient bool hasControlledBounds bool - needsRerenderOnWidthChange bool + needsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel needsRerenderOnHeightChange bool highlightOnFocus bool @@ -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 @@ -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 } diff --git a/pkg/gui/context/branches_context.go b/pkg/gui/context/branches_context.go index d2647ef84..d289f2729 100644 --- a/pkg/gui/context/branches_context.go +++ b/pkg/gui/context/branches_context.go @@ -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, diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index ab42cfb70..fcb9b00ca 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -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{ diff --git a/pkg/gui/context/reflog_commits_context.go b/pkg/gui/context/reflog_commits_context.go index 57ca7c4dc..cec54988d 100644 --- a/pkg/gui/context/reflog_commits_context.go +++ b/pkg/gui/context/reflog_commits_context.go @@ -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, diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go index ab0d2784a..ddbb380c5 100644 --- a/pkg/gui/context/sub_commits_context.go +++ b/pkg/gui/context/sub_commits_context.go @@ -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{ diff --git a/pkg/gui/controllers/screen_mode_actions.go b/pkg/gui/controllers/screen_mode_actions.go index 1db27f2e2..2d0026793 100644 --- a/pkg/gui/controllers/screen_mode_actions.go +++ b/pkg/gui/controllers/screen_mode_actions.go @@ -1,6 +1,7 @@ package controllers import ( + "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -16,7 +17,7 @@ func (self *ScreenModeActions) Next() error { ), ) - return nil + return self.rerenderViewsWithScreenModeDependentContent() } func (self *ScreenModeActions) Prev() error { @@ -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 { diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 4e2b49477..2123731e4 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -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 diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 003035fc2..70458c16f 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -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 @@ -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 -- cgit v1.2.3 From 26132cf5bdae7ec81e9a0708c722ad2a9cf0c2cf Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 22 Jun 2024 13:34:15 +0200 Subject: Use utils.StringWidth to optimize rendering performance runewidth.StringWidth is an expensive call, even if the input string is pure ASCII. Improve this by providing a wrapper that short-circuits the call to len if the input is ASCII. Benchmark results show that for non-ASCII strings it makes no noticable difference, but for ASCII strings it provides a more than 200x speedup. BenchmarkStringWidthAsciiOriginal-10 718135 1637 ns/op BenchmarkStringWidthAsciiOptimized-10 159197538 7.545 ns/op BenchmarkStringWidthNonAsciiOriginal-10 486290 2391 ns/op BenchmarkStringWidthNonAsciiOptimized-10 502286 2383 ns/op --- .../helpers/window_arrangement_helper.go | 7 +++--- pkg/gui/information_panel.go | 7 +++--- pkg/gui/presentation/branches.go | 6 +++--- pkg/utils/formatting.go | 19 +++++++++++++--- pkg/utils/formatting_test.go | 25 ++++++++++++++++++++++ 5 files changed, 50 insertions(+), 14 deletions(-) diff --git a/pkg/gui/controllers/helpers/window_arrangement_helper.go b/pkg/gui/controllers/helpers/window_arrangement_helper.go index 0eb7cdb4a..322cd1bd6 100644 --- a/pkg/gui/controllers/helpers/window_arrangement_helper.go +++ b/pkg/gui/controllers/helpers/window_arrangement_helper.go @@ -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" ) @@ -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", @@ -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)}) } } @@ -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)), }) } diff --git a/pkg/gui/information_panel.go b/pkg/gui/information_panel.go index 00867fb92..3eac1e77c 100644 --- a/pkg/gui/information_panel.go +++ b/pkg/gui/information_panel.go @@ -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 { @@ -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() @@ -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 } diff --git a/pkg/gui/presentation/branches.go b/pkg/gui/presentation/branches.go index b4c4a75c7..b75dfc95b 100644 --- a/pkg/gui/presentation/branches.go +++ b/pkg/gui/presentation/branches.go @@ -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 @@ -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 @@ -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, "…") diff --git a/pkg/utils/formatting.go b/pkg/utils/formatting.go index a6bbc5670..b7817346a 100644 --- a/pkg/utils/formatting.go +++ b/pkg/utils/formatting.go @@ -3,6 +3,7 @@ package utils import ( "fmt" "strings" + "unicode" "github.com/mattn/go-runewidth" "github.com/samber/lo" @@ -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 } @@ -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) }) }) } @@ -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, "…") diff --git a/pkg/utils/formatting_test.go b/pkg/utils/formatting_test.go index 5b56a9b33..ac2adee5f 100644 --- a/pkg/utils/formatting_test.go +++ b/pkg/utils/formatting_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "github.com/mattn/go-runewidth" "github.com/stretchr/testify/assert" ) @@ -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 🍉") + } +} -- cgit v1.2.3