Replace the BLOCK_UI refresh mode with a BatchUIUpdates flag

BLOCK_UI ran the whole refresh on the UI thread and parked it in a
wg.Wait for the duration, so the UI (and its spinner) froze while the
git work ran. Blocking the UI was never the point — the point was to
apply all the scopes' updates in one frame instead of a per-scope
cascade — and if we genuinely wanted to block input it should span the
whole operation, not just its refresh, which needs a gocui-level
mechanism we don't have.

So drop the mode and add a BatchUIUpdates option that achieves the
"one frame" effect without blocking: each scope's UI-thread bounce is
collected into a shared refreshBounceBatch during the refresh, and once
every scope has finished they're all applied inside a single OnUIThread
task. gocui drains every queued event before it redraws, so one task
means one repaint. The refresh itself now runs SYNC — on a worker when
issued from one (checkout, move-to-new-branch, the rebase-edit result
handling), so the UI thread stays live and the spinner keeps animating.

The batch needs a mutex because the scopes add concurrently from their
worker goroutines, and a closed flag so that any bounces enqueued after
the flush starts — the nested ones a flushed bounce produces in turn,
e.g. scrolling the selection into view — are dispatched immediately as
ordinary follow-ups rather than collected into a batch that nothing
will drain.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-07-07 14:02:04 +02:00
parent 504e5b3f74
commit 4acfc88065
4 changed files with 100 additions and 38 deletions

View file

@ -96,6 +96,49 @@ type refreshEnv struct {
// the repo generation captured when the refresh started
generation int
// When non-nil, each scope's UI-thread bounce is collected here instead of
// being dispatched as it's produced, so they can all be applied in a single
// frame once the whole refresh is done (see RefreshOptions.BatchUIUpdates).
// Held by pointer so the copies of env that flow through the scope functions
// all share the one batch.
batch *refreshBounceBatch
}
// refreshBounceBatch collects the UI-thread bounces of a batched refresh so they
// can be applied together in one frame rather than one scope at a time. The
// scopes run on separate worker goroutines and add concurrently, hence the
// mutex. Once the refresh starts flushing it closes the batch, so that any
// bounces enqueued afterwards — the nested ones a flushed bounce produces in
// turn, e.g. scrolling the selection into view — are dispatched immediately as
// ordinary follow-ups instead of being collected into a batch that nothing
// will drain.
type refreshBounceBatch struct {
mutex deadlock.Mutex
funcs []func()
closed bool
}
// add collects f and returns true. Once the batch is closed it collects nothing
// and returns false, telling the caller to dispatch f immediately instead.
func (self *refreshBounceBatch) add(f func()) bool {
self.mutex.Lock()
defer self.mutex.Unlock()
if self.closed {
return false
}
self.funcs = append(self.funcs, f)
return true
}
// close marks the batch flushed and returns everything collected so far.
func (self *refreshBounceBatch) close() []func() {
self.mutex.Lock()
defer self.mutex.Unlock()
self.closed = true
return self.funcs
}
func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFromWorker bool) {
@ -121,18 +164,15 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr
)
}
// f runs on the UI thread when the refresh was initiated there, and also for
// BLOCK_UI, which dispatches f onto the UI thread regardless of the caller.
// Only a SYNC/ASYNC refresh initiated from a worker runs f on that worker.
// This, not calledFromWorker alone, is what decides whether a scope capture
// runs inline or has to hop (see captureOnUIThread).
fRunsOnUIThread := options.Mode == types.BLOCK_UI || !calledFromWorker
// f runs on the UI thread when the refresh was initiated there (Refresh); a
// refresh initiated from a worker (RefreshFromWorker) runs f on that worker.
// This decides whether a scope capture runs inline or has to hop (see
// captureOnUIThread).
fRunsOnUIThread := !calledFromWorker
// Debug-only guard: every refresh must be issued from the entry point that
// matches its goroutine — Refresh on the UI thread, RefreshFromWorker on a
// worker. We check the caller's own goroutine here, before a BLOCK_UI
// refresh dispatches f onto the UI thread, so it holds regardless of the
// mode. goid stays out of production control flow (debug only).
// worker. goid stays out of production control flow (debug only).
if self.c.GetConfig().GetDebug() && self.c.GocuiGui().IsUIThread() == calledFromWorker {
panic("Refresh called from a worker, or RefreshFromWorker called from the UI thread")
}
@ -144,6 +184,9 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr
background: options.Background,
generation: self.c.State().GetRepoGeneration(),
}
if options.BatchUIUpdates {
env.batch = &refreshBounceBatch{}
}
var scopeSet *set.Set[types.RefreshableView]
if len(options.Scope) == 0 {
@ -376,6 +419,20 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr
wg.Wait()
if env.batch != nil {
// Apply all the scopes' collected bounces in a single UI-thread task,
// so they land in one frame: gocui drains every queued event before it
// redraws, so one task means one repaint. Bounces enqueued from within
// these (see refreshBounceBatch) run as ordinary follow-ups.
bounces := env.batch.close()
self.onUIThread(env.background, func() error {
for _, bounce := range bounces {
bounce()
}
return nil
})
}
if options.Then != nil {
// Queue Then via OnUIThread so it runs *after* the refresh-scope
// functions' model-update bounces (which are already queued by
@ -387,14 +444,6 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr
}
}
if options.Mode == types.BLOCK_UI {
self.c.OnUIThread(func() error {
f()
return nil
})
return
}
f()
}
@ -482,8 +531,6 @@ func getModeName(mode types.RefreshMode) string {
return "sync"
case types.ASYNC:
return "async"
case types.BLOCK_UI:
return "block-ui"
default:
return "unknown mode"
}
@ -1057,13 +1104,21 @@ func (self *RefreshHelper) refreshFilesAndSubmodules(captured capturedFilesState
// clobber the new repo's state. The generation is captured once at the start of
// the refresh and carried in env (see refreshEnv).
func (self *RefreshHelper) onUIThreadUnlessRepoChanged(env refreshEnv, f func()) {
self.onUIThread(env.background, func() error {
wrapper := func() {
if self.c.State().GetRepoGeneration() != env.generation {
return nil
return
}
f()
return nil
})
}
// A batched refresh collects its bounces and fires them together at the end
// (see refreshBounceBatch); add reports false once the batch is flushing, so
// bounces enqueued from within a flushed bounce dispatch immediately.
if env.batch != nil && env.batch.add(wrapper) {
return
}
self.onUIThread(env.background, func() error { wrapper(); return nil })
}
// onWorker and onUIThread pick the foreground or background variant of the
@ -1094,12 +1149,11 @@ func (self *RefreshHelper) onUIThread(background bool, f func() error) {
// runs on the UI thread (fRunsOnUIThread is true) fn runs inline; when it runs
// on a worker, fn is dispatched to the UI thread and we block for it.
//
// The inline case matters for correctness as much as the hop: a SYNC or
// BLOCK_UI refresh parks the UI thread in a wg.Wait while its scope workers
// run, so a scope worker that tried to hop to the UI thread there would
// The inline case matters for correctness as much as the hop: a SYNC refresh
// initiated on the UI thread parks that thread in a wg.Wait while its scope
// workers run, so a scope worker that tried to hop to the UI thread there would
// deadlock. Capturing before those workers are spawned — inline, on the UI
// thread — avoids that entirely. This is why BLOCK_UI (which always runs on the
// UI thread, even from a worker caller) captures inline rather than hopping.
// thread — avoids that entirely.
func (self *RefreshHelper) captureOnUIThread(fRunsOnUIThread bool, background bool, fn func()) {
if fRunsOnUIThread {
fn()

View file

@ -56,7 +56,8 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions
scope = append(scope, types.PULL_REQUESTS)
}
self.c.RefreshFromWorker(types.RefreshOptions{
Mode: types.BLOCK_UI,
Mode: types.SYNC,
BatchUIUpdates: true,
Scope: scope,
BranchSelection: types.SelectCheckedOutBranch,
CommitSelection: types.SelectHeadCommit,
@ -368,7 +369,8 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest
}
self.c.Refresh(types.RefreshOptions{
Mode: types.BLOCK_UI,
Mode: types.SYNC,
BatchUIUpdates: true,
BranchSelection: types.SelectCheckedOutBranch,
CommitSelection: types.SelectHeadCommit,
SelectTopReflogCommit: true,
@ -534,7 +536,8 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa
}
self.c.RefreshFromWorker(types.RefreshOptions{
Mode: types.BLOCK_UI,
Mode: types.SYNC,
BatchUIUpdates: true,
BranchSelection: types.SelectCheckedOutBranch,
CommitSelection: types.SelectHeadCommit,
SelectTopReflogCommit: true,
@ -570,7 +573,8 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri
}
self.c.RefreshFromWorker(types.RefreshOptions{
Mode: types.BLOCK_UI,
Mode: types.SYNC,
BatchUIUpdates: true,
BranchSelection: types.SelectCheckedOutBranch,
CommitSelection: types.SelectHeadCommit,
SelectTopReflogCommit: true,

View file

@ -604,7 +604,7 @@ func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, start
return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit, "")
return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(
err, types.RefreshOptions{Mode: types.BLOCK_UI})
err, types.RefreshOptions{BatchUIUpdates: true})
})
}
@ -628,7 +628,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit(
err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash())
return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(
err,
types.RefreshOptions{Mode: types.BLOCK_UI, Then: func() error {
types.RefreshOptions{BatchUIUpdates: true, Then: func() error {
todos := make([]*models.Commit, 0, len(commitsToEdit)-1)
for _, c := range commitsToEdit[:len(commitsToEdit)-1] {
// Merge commits can't be set to "edit", so just skip them

View file

@ -28,9 +28,8 @@ const (
type RefreshMode int
const (
SYNC RefreshMode = iota // wait until everything is done before returning
ASYNC // return immediately, allowing each independent thing to update itself
BLOCK_UI // wrap code in an update call to ensure UI updates all at once and keybindings aren't executed till complete
SYNC RefreshMode = iota // wait until everything is done before returning
ASYNC // return immediately, allowing each independent thing to update itself
)
// CommitSelectionBehavior controls which local commit is selected after the
@ -74,7 +73,12 @@ const (
type RefreshOptions struct {
Then func() error
Scope []RefreshableView // e.g. []RefreshableView{COMMITS, BRANCHES}. Leave empty to refresh everything
Mode RefreshMode // one of SYNC (default), ASYNC, and BLOCK_UI
Mode RefreshMode // one of SYNC (default) and ASYNC
// If true, hold off on updating the UI until all scopes have finished
// refreshing and then apply them together in a single frame, rather than
// letting each scope update the UI as soon as it's done.
BatchUIUpdates bool
// Controls which local branch is selected after the refresh. Defaults to
// KeepBranchSelectionByName.