mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-08-24 10:13:41 -05:00
Redraw only the spinner when nothing else has changed (#5571)
### PR Description This fixes #4734, where lazygit would use a lot of CPU when the spinner was shown due to redrawing the entire screen every time the spinner needed a redraw. The change adds the ability to request a UI redraw without invalidating the full UI, thus only redrawing the spinner, re-routing the spinner to this new flow. CPU usage on a chromebook went from 60% to 5% when the spinner is shown. Changing the refresh rate of the spinner from 50ms to 200ms brings it down from 5% to 1% if ever we would like to change the default, personally I prefer the slower moving spinner, but that's subjective.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"reflect"
|
||||
@@ -8,6 +9,8 @@ import (
|
||||
"strings"
|
||||
|
||||
"github.com/jesseduffield/lazygit/pkg/constants"
|
||||
"github.com/jesseduffield/lazygit/pkg/utils"
|
||||
"github.com/samber/lo"
|
||||
)
|
||||
|
||||
func (config *UserConfig) Validate() error {
|
||||
@@ -49,6 +52,22 @@ func (config *UserConfig) Validate() error {
|
||||
if err := validateCustomCommands(config.CustomCommands); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := validateSpinner(config.Gui.Spinner); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateSpinner(spinner SpinnerConfig) error {
|
||||
if len(spinner.Frames) == 0 {
|
||||
return errors.New("gui.spinner.frames must not be empty.")
|
||||
}
|
||||
firstWidth := utils.StringWidth(spinner.Frames[0])
|
||||
if lo.SomeBy(spinner.Frames, func(frame string) bool {
|
||||
return utils.StringWidth(frame) != firstWidth
|
||||
}) {
|
||||
return errors.New("All gui.spinner.frames entries must have the same width.")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -289,3 +289,33 @@ func TestUserConfigValidate_enums(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUserConfigValidate_spinnerFrames(t *testing.T) {
|
||||
scenarios := []struct {
|
||||
name string
|
||||
frames []string
|
||||
valid bool
|
||||
}{
|
||||
{name: "empty", frames: []string{}, valid: false},
|
||||
{name: "single frame", frames: []string{"|"}, valid: true},
|
||||
{name: "all same width", frames: []string{"|", "/", "-", "\\"}, valid: true},
|
||||
{name: "all same width, multi-char", frames: []string{". ", ".. ", "..."}, valid: true},
|
||||
{name: "all same width, wide runes", frames: []string{"⠋", "⠙", "⠹"}, valid: true},
|
||||
{name: "differing widths", frames: []string{"|", "//"}, valid: false},
|
||||
{name: "first differs from rest", frames: []string{"||", "/", "-"}, valid: false},
|
||||
}
|
||||
|
||||
for _, s := range scenarios {
|
||||
t.Run(s.name, func(t *testing.T) {
|
||||
config := GetDefaultConfig()
|
||||
config.Gui.Spinner.Frames = s.frames
|
||||
err := config.Validate()
|
||||
|
||||
if s.valid {
|
||||
assert.NoError(t, err)
|
||||
} else {
|
||||
assert.Error(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,202 @@
|
||||
package gocui
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func newTestGui(t *testing.T) *Gui {
|
||||
t.Helper()
|
||||
g, err := NewGui(NewGuiOpts{
|
||||
OutputMode: OutputNormal,
|
||||
Headless: true,
|
||||
Width: 80,
|
||||
Height: 24,
|
||||
})
|
||||
assert.NoError(t, err)
|
||||
t.Cleanup(func() { g.Close() })
|
||||
return g
|
||||
}
|
||||
|
||||
// setupViews creates a few views and does an initial full flush so all views
|
||||
// start in a clean (non-tainted) state.
|
||||
func setupViews(t *testing.T, g *Gui) (*View, *View) {
|
||||
t.Helper()
|
||||
|
||||
status, _ := g.SetView("status", 0, 22, 40, 24, 0)
|
||||
status.Frame = false
|
||||
main, _ := g.SetView("main", 0, 0, 80, 22, 0)
|
||||
|
||||
// Initial content
|
||||
status.SetContent("Ready")
|
||||
main.SetContent("hello world")
|
||||
|
||||
// Full flush to draw everything and clear tainted flags
|
||||
assert.NoError(t, g.flush())
|
||||
|
||||
return status, main
|
||||
}
|
||||
|
||||
// pushContentOnly pushes a content-only event directly to the channel
|
||||
// (synchronous, deterministic — unlike Update which spawns a goroutine).
|
||||
func pushContentOnly(g *Gui, f func(*Gui) error) {
|
||||
g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: true}
|
||||
}
|
||||
|
||||
// pushRegular pushes a regular event directly to the channel.
|
||||
func pushRegular(g *Gui, f func(*Gui) error) {
|
||||
g.userEvents <- userEvent{f: f, task: g.NewTask(), contentOnly: false}
|
||||
}
|
||||
|
||||
func TestFlushContentOnly_SkipsUntaintedViews(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, main := setupViews(t, g)
|
||||
|
||||
// After initial flush, both views should be untainted
|
||||
assert.False(t, status.IsTainted(), "status view should not be tainted after flush")
|
||||
assert.False(t, main.IsTainted(), "main view should not be tainted after flush")
|
||||
|
||||
// Modify only the status view
|
||||
status.SetContent("Fetching /")
|
||||
|
||||
assert.True(t, status.IsTainted(), "status view should be tainted after SetContent")
|
||||
assert.False(t, main.IsTainted(), "main view should not be tainted (was not modified)")
|
||||
|
||||
// flushContentOnly should succeed and clear status tainted flag
|
||||
assert.NoError(t, g.flushContentOnly(g.views))
|
||||
|
||||
assert.False(t, status.IsTainted(), "status view should not be tainted after flushContentOnly")
|
||||
assert.False(t, main.IsTainted(), "main view should not be tainted after flushContentOnly")
|
||||
}
|
||||
|
||||
func TestFlushContentOnly_WritesCorrectContent(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, _ := setupViews(t, g)
|
||||
|
||||
status.SetContent("Fetching |")
|
||||
assert.NoError(t, g.flushContentOnly(g.views))
|
||||
|
||||
assert.Equal(t, "Fetching |", status.Buffer())
|
||||
}
|
||||
|
||||
func TestProcessEvent_ContentOnlyEvent_SkipsTaintedCheck(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, main := setupViews(t, g)
|
||||
|
||||
// Send a content-only event that modifies only the status view
|
||||
pushContentOnly(g, func(gui *Gui) error {
|
||||
status.SetContent("Fetching /")
|
||||
return nil
|
||||
})
|
||||
|
||||
assert.NoError(t, g.processEvent())
|
||||
|
||||
// status was modified and drawn → tainted cleared
|
||||
assert.False(t, status.IsTainted(), "status should not be tainted after processEvent with contentOnly")
|
||||
// main was NOT modified → should still be untainted
|
||||
assert.False(t, main.IsTainted(), "main should not be tainted after processEvent with contentOnly")
|
||||
}
|
||||
|
||||
func TestProcessEvent_RegularEvent_UsesFullFlush(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, _ := setupViews(t, g)
|
||||
|
||||
// Regular event (not content-only) should trigger full flush
|
||||
pushRegular(g, func(gui *Gui) error {
|
||||
status.SetContent("Fetching \\")
|
||||
return nil
|
||||
})
|
||||
|
||||
assert.NoError(t, g.processEvent())
|
||||
|
||||
assert.False(t, status.IsTainted(), "status should not be tainted after full flush")
|
||||
}
|
||||
|
||||
func TestProcessEvent_MixedBatch_UsesFullFlush(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, main := setupViews(t, g)
|
||||
|
||||
// Queue a content-only event followed by a regular event.
|
||||
// processEvent picks up the first; processRemainingEvents picks up
|
||||
// the second. Since the second is not contentOnly, full flush runs.
|
||||
pushContentOnly(g, func(gui *Gui) error {
|
||||
status.SetContent("Fetching -")
|
||||
return nil
|
||||
})
|
||||
pushRegular(g, func(gui *Gui) error {
|
||||
main.SetContent("updated main")
|
||||
return nil
|
||||
})
|
||||
|
||||
assert.NoError(t, g.processEvent())
|
||||
|
||||
// Both views were modified and should have been drawn by full flush
|
||||
assert.False(t, status.IsTainted(), "status should not be tainted after full flush")
|
||||
assert.False(t, main.IsTainted(), "main should not be tainted after full flush")
|
||||
}
|
||||
|
||||
func TestProcessEvent_RegularThenContentOnly_UsesFullFlush(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, main := setupViews(t, g)
|
||||
|
||||
// Even if a regular event comes first and the remaining are contentOnly,
|
||||
// the batch must use full flush.
|
||||
pushRegular(g, func(gui *Gui) error {
|
||||
main.SetContent("new main content")
|
||||
return nil
|
||||
})
|
||||
pushContentOnly(g, func(gui *Gui) error {
|
||||
status.SetContent("Fetching |")
|
||||
return nil
|
||||
})
|
||||
|
||||
assert.NoError(t, g.processEvent())
|
||||
|
||||
assert.False(t, status.IsTainted(), "status should not be tainted after full flush")
|
||||
assert.False(t, main.IsTainted(), "main should not be tainted after full flush")
|
||||
}
|
||||
|
||||
func TestProcessRemainingEvents_AllContentOnly_ReturnsTrue(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, _ := setupViews(t, g)
|
||||
|
||||
pushContentOnly(g, func(gui *Gui) error {
|
||||
status.SetContent("a")
|
||||
return nil
|
||||
})
|
||||
pushContentOnly(g, func(gui *Gui) error {
|
||||
status.SetContent("b")
|
||||
return nil
|
||||
})
|
||||
|
||||
contentOnly, err := g.processRemainingEvents()
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, contentOnly, "should return true when all events are contentOnly")
|
||||
}
|
||||
|
||||
func TestProcessRemainingEvents_MixedEvents_ReturnsFalse(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
status, _ := setupViews(t, g)
|
||||
|
||||
pushContentOnly(g, func(gui *Gui) error {
|
||||
status.SetContent("a")
|
||||
return nil
|
||||
})
|
||||
pushRegular(g, func(gui *Gui) error {
|
||||
status.SetContent("b")
|
||||
return nil
|
||||
})
|
||||
|
||||
contentOnly, err := g.processRemainingEvents()
|
||||
assert.NoError(t, err)
|
||||
assert.False(t, contentOnly, "should return false when any event is not contentOnly")
|
||||
}
|
||||
|
||||
func TestProcessRemainingEvents_EmptyQueue_ReturnsTrue(t *testing.T) {
|
||||
g := newTestGui(t)
|
||||
|
||||
contentOnly, err := g.processRemainingEvents()
|
||||
assert.NoError(t, err)
|
||||
assert.True(t, contentOnly, "should return true when no events are queued")
|
||||
}
|
||||
+48
-22
@@ -604,6 +604,10 @@ func (g *Gui) SetRenderSearchStatusFunc(renderSearchStatusFunc func(*View, int,
|
||||
type userEvent struct {
|
||||
f func(*Gui) error
|
||||
task Task
|
||||
// Signals that this event only modifies view content (e.g. SetContent).
|
||||
// When all events in a batch are contentOnly, processEvent
|
||||
// can skip the expensive layout() call in flush().
|
||||
contentOnly bool
|
||||
}
|
||||
|
||||
// Update executes the passed function. This method can be called safely from a
|
||||
@@ -630,6 +634,12 @@ func (g *Gui) updateAsyncAux(f func(*Gui) error, task Task) {
|
||||
g.userEvents <- userEvent{f: f, task: task}
|
||||
}
|
||||
|
||||
// Like Update, but signals that the callback only modifies content.
|
||||
func (g *Gui) UpdateContentOnly(f func(*Gui) error) {
|
||||
task := g.NewTask()
|
||||
g.userEvents <- userEvent{f: f, task: task, contentOnly: true}
|
||||
}
|
||||
|
||||
// Calls a function in a goroutine. Handles panics gracefully and tracks
|
||||
// number of background tasks.
|
||||
// Always use this when you want to spawn a goroutine and you want lazygit to
|
||||
@@ -743,6 +753,8 @@ func (g *Gui) handleError(err error) error {
|
||||
}
|
||||
|
||||
func (g *Gui) processEvent() error {
|
||||
contentOnly := false
|
||||
|
||||
select {
|
||||
case ev := <-g.gEvents:
|
||||
task := g.NewTask()
|
||||
@@ -752,6 +764,7 @@ func (g *Gui) processEvent() error {
|
||||
return err
|
||||
}
|
||||
case ev := <-g.userEvents:
|
||||
contentOnly = ev.contentOnly
|
||||
defer func() { ev.task.Done() }()
|
||||
|
||||
if err := g.handleError(ev.f(g)); err != nil {
|
||||
@@ -759,32 +772,38 @@ func (g *Gui) processEvent() error {
|
||||
}
|
||||
}
|
||||
|
||||
if err := g.processRemainingEvents(); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := g.flush(); err != nil {
|
||||
remainingContentOnly, err := g.processRemainingEvents()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
contentOnly = contentOnly && remainingContentOnly
|
||||
|
||||
return nil
|
||||
if contentOnly {
|
||||
return g.flushContentOnly(g.views)
|
||||
}
|
||||
return g.flush()
|
||||
}
|
||||
|
||||
// processRemainingEvents handles the remaining events in the events pool.
|
||||
func (g *Gui) processRemainingEvents() error {
|
||||
// Returns true if all processed events were content-only.
|
||||
func (g *Gui) processRemainingEvents() (bool, error) {
|
||||
contentOnly := true
|
||||
for {
|
||||
select {
|
||||
case ev := <-g.gEvents:
|
||||
contentOnly = false
|
||||
if err := g.handleError(g.handleEvent(&ev)); err != nil {
|
||||
return err
|
||||
return false, err
|
||||
}
|
||||
case ev := <-g.userEvents:
|
||||
contentOnly = ev.contentOnly && contentOnly
|
||||
err := g.handleError(ev.f(g))
|
||||
ev.task.Done()
|
||||
if err != nil {
|
||||
return err
|
||||
return false, err
|
||||
}
|
||||
default:
|
||||
return nil
|
||||
return contentOnly, nil
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1148,27 +1167,34 @@ func (g *Gui) flush() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (g *Gui) ForceLayoutAndRedraw() error {
|
||||
return g.flush()
|
||||
}
|
||||
|
||||
// force redrawing one or more views outside of the normal main loop. Useful during longer
|
||||
// operations that block the main thread, to update a spinner in a status view.
|
||||
func (g *Gui) ForceRedrawViews(views ...*View) error {
|
||||
for _, m := range g.managers {
|
||||
if err := m.Layout(g); err != nil {
|
||||
// Redraws only tainted views and skips the layout pass.
|
||||
// tcell's cell-level dirty tracking ensures only
|
||||
// actually-changed cells are emitted to the terminal.
|
||||
func (g *Gui) flushContentOnly(views []*View) error {
|
||||
for _, v := range views {
|
||||
if !v.tainted {
|
||||
continue
|
||||
}
|
||||
if err := g.draw(v); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
for _, v := range views {
|
||||
v.draw()
|
||||
}
|
||||
|
||||
Screen.Show()
|
||||
return nil
|
||||
}
|
||||
|
||||
func (g *Gui) ForceLayoutAndRedraw() error {
|
||||
return g.flush()
|
||||
}
|
||||
|
||||
// Redraws only tainted views outside of the normal main
|
||||
// loop, without a layout pass. Useful during longer operations that block the
|
||||
// main thread, e.g. to update a spinner in a status view.
|
||||
func (g *Gui) ForceFlushViewsContentOnly(views []*View) error {
|
||||
return g.flushContentOnly(views)
|
||||
}
|
||||
|
||||
// draw manages the cursor and calls the draw function of a view.
|
||||
func (g *Gui) draw(v *View) error {
|
||||
if g.suspended {
|
||||
|
||||
@@ -96,7 +96,7 @@ func (self *AppStatusHelper) renderAppStatus() {
|
||||
for range ticker.C {
|
||||
appStatus, color := self.statusMgr().GetStatusString(self.c.UserConfig())
|
||||
self.c.Views().AppStatus.FgColor = color
|
||||
self.c.OnUIThread(func() error {
|
||||
self.c.OnUIThreadContentOnly(func() error {
|
||||
self.c.SetViewContent(self.c.Views().AppStatus, appStatus)
|
||||
return nil
|
||||
})
|
||||
@@ -111,7 +111,7 @@ func (self *AppStatusHelper) renderAppStatus() {
|
||||
|
||||
func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) {
|
||||
go func() {
|
||||
ticker := time.NewTicker(time.Millisecond * 50)
|
||||
ticker := time.NewTicker(time.Millisecond * time.Duration(self.c.UserConfig().Gui.Spinner.Rate))
|
||||
defer ticker.Stop()
|
||||
|
||||
// Forcing a re-layout and redraw after we added the waiting status;
|
||||
@@ -136,7 +136,7 @@ func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) {
|
||||
self.c.Views().AppStatus, self.c.Views().Options, self.c.Views().Information,
|
||||
self.c.Views().StatusSpacer1, self.c.Views().StatusSpacer2,
|
||||
}
|
||||
_ = self.c.GocuiGui().ForceRedrawViews(bottomLineViews...)
|
||||
_ = self.c.GocuiGui().ForceFlushViewsContentOnly(bottomLineViews)
|
||||
case <-stop:
|
||||
break outer
|
||||
}
|
||||
|
||||
@@ -428,6 +428,8 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.Context
|
||||
|
||||
gui.c.Context().Push(contextToPush, types.OnFocusOpts{})
|
||||
|
||||
gui.render()
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1186,6 +1188,12 @@ func (gui *Gui) onUIThread(f func() error) {
|
||||
})
|
||||
}
|
||||
|
||||
func (gui *Gui) onUIThreadContentOnly(f func() error) {
|
||||
gui.g.UpdateContentOnly(func(*gocui.Gui) error {
|
||||
return f()
|
||||
})
|
||||
}
|
||||
|
||||
func (gui *Gui) onWorker(f func(gocui.Task) error) {
|
||||
gui.g.OnWorker(f)
|
||||
}
|
||||
|
||||
@@ -120,6 +120,10 @@ func (self *guiCommon) OnUIThread(f func() error) {
|
||||
self.gui.onUIThread(f)
|
||||
}
|
||||
|
||||
func (self *guiCommon) OnUIThreadContentOnly(f func() error) {
|
||||
self.gui.onUIThreadContentOnly(f)
|
||||
}
|
||||
|
||||
func (self *guiCommon) OnWorker(f func(gocui.Task) error) {
|
||||
self.gui.onWorker(f)
|
||||
}
|
||||
|
||||
@@ -71,6 +71,10 @@ type IGuiCommon interface {
|
||||
// Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine.
|
||||
// All controller handlers are executed on the UI thread.
|
||||
OnUIThread(f func() error)
|
||||
// Like OnUIThread, but signals that the callback only modifies view
|
||||
// content (e.g. spinner), allows the event loop to skip
|
||||
// the expensive layout recalculation when only content changed.
|
||||
OnUIThreadContentOnly(f func() error)
|
||||
// Runs a function in a goroutine. Use this whenever you want to run a goroutine and keep track of the fact
|
||||
// that lazygit is still busy. See docs/dev/Busy.md
|
||||
OnWorker(f func(gocui.Task) error)
|
||||
|
||||
Reference in New Issue
Block a user