Skip to content

Commit b1382da

Browse files
authored
Fix delay with flicking through files or commits when git diff is very slow (#4803)
One reason why git diff can be very slow is when "diff.algorithm = histogram" is being used. In this case, showing a very long single-file diff can take seconds to load, and you'll see the "loading..." message in the main view until we got the first lines of the diff to show. There's nothing really we can do about this delay; however, when switching to another, shorter file (or commit) while the "loading..." message is still showing, this switch should be instantaneous. And it was before 0.54.0, but we broke this in 0.54.0 with 8d7740a (#4782); now users have to wait for the slow git diff command to output more text before the switch occurs. To fix this, don't block waiting for the process to terminate if we just stopped it. In addition, improve CPU usage by terminating git processes gracefully (by sending them a TERM signal); this helps keep CPU usage low when flicking through several of these huge diffs with "diff.algorithm = histogram", because it avoids having several git processes running in the background, calculating expensive diffs that we are never going to show. Unfortunately this is only possible on Linux and Mac, so Windows users will have to live with the higher CPU usage. The recommended workaround is to not use "diff.algorithm = histogram". Fixes #4798.
2 parents 77a9207 + e056e33 commit b1382da

File tree

4 files changed

+56
-18
lines changed

4 files changed

+56
-18
lines changed

pkg/commands/oscommands/os.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package oscommands
22

33
import (
4-
"fmt"
54
"io"
65
"os"
76
"os/exec"
@@ -329,15 +328,3 @@ func GetLazygitPath() string {
329328
}
330329
return `"` + filepath.ToSlash(ex) + `"`
331330
}
332-
333-
func (c *OSCommand) UpdateWindowTitle() error {
334-
if c.Platform.OS != "windows" {
335-
return nil
336-
}
337-
path, getWdErr := os.Getwd()
338-
if getWdErr != nil {
339-
return getWdErr
340-
}
341-
argString := fmt.Sprint("title ", filepath.Base(path), " - Lazygit")
342-
return c.Cmd.NewShell(argString, c.UserConfig().OS.ShellFunctionsFile).Run()
343-
}

pkg/commands/oscommands/os_default_platform.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ package oscommands
55

66
import (
77
"os"
8+
"os/exec"
89
"runtime"
910
"strings"
11+
"syscall"
1012
)
1113

1214
func GetPlatform() *Platform {
@@ -34,3 +36,15 @@ func getUserShell() string {
3436

3537
return "bash"
3638
}
39+
40+
func (c *OSCommand) UpdateWindowTitle() error {
41+
return nil
42+
}
43+
44+
func TerminateProcessGracefully(cmd *exec.Cmd) error {
45+
if cmd.Process == nil {
46+
return nil
47+
}
48+
49+
return cmd.Process.Signal(syscall.SIGTERM)
50+
}

pkg/commands/oscommands/os_windows.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,30 @@
11
package oscommands
22

3+
import (
4+
"fmt"
5+
"os"
6+
"os/exec"
7+
"path/filepath"
8+
)
9+
310
func GetPlatform() *Platform {
411
return &Platform{
512
OS: "windows",
613
Shell: "cmd",
714
ShellArg: "/c",
815
}
916
}
17+
18+
func (c *OSCommand) UpdateWindowTitle() error {
19+
path, getWdErr := os.Getwd()
20+
if getWdErr != nil {
21+
return getWdErr
22+
}
23+
argString := fmt.Sprint("title ", filepath.Base(path), " - Lazygit")
24+
return c.Cmd.NewShell(argString, c.UserConfig().OS.ShellFunctionsFile).Run()
25+
}
26+
27+
func TerminateProcessGracefully(cmd *exec.Cmd) error {
28+
// Signals other than SIGKILL are not supported on Windows
29+
return nil
30+
}

pkg/tasks/tasks.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"github.com/jesseduffield/gocui"
12+
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
1213
"github.com/jesseduffield/lazygit/pkg/utils"
1314
"github.com/sasha-s/go-deadlock"
1415
"github.com/sirupsen/logrus"
@@ -165,6 +166,17 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
165166
// and the user is flicking through a bunch of items.
166167
self.throttle = time.Since(startTime) < THROTTLE_TIME && timeToStart > COMMAND_START_THRESHOLD
167168

169+
// Kill the still-running command. The only reason to do this is to save CPU usage
170+
// when flicking through several very long diffs when diff.algorithm = histogram is
171+
// being used, in which case multiple git processes continue to calculate expensive
172+
// diffs in the background even though they have been stopped already.
173+
//
174+
// Unfortunately this will do nothing on Windows, so Windows users will have to live
175+
// with the higher CPU usage.
176+
if err := oscommands.TerminateProcessGracefully(cmd); err != nil {
177+
self.Log.Errorf("error when trying to terminate cmd task: %v; Command: %v %v", err, cmd.Path, cmd.Args)
178+
}
179+
168180
// close the task's stdout pipe (or the pty if we're using one) to make the command terminate
169181
onDone()
170182
}
@@ -291,11 +303,15 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
291303

292304
refreshViewIfStale()
293305

294-
if err := cmd.Wait(); err != nil {
295-
select {
296-
case <-opts.Stop:
297-
// it's fine if we've killed this program ourselves
298-
default:
306+
select {
307+
case <-opts.Stop:
308+
// If we stopped the task, don't block waiting for it; this could cause a delay if
309+
// the process takes a while until it actually terminates. We still want to call
310+
// Wait to reclaim any resources, but do it on a background goroutine, and ignore
311+
// any errors.
312+
go func() { _ = cmd.Wait() }()
313+
default:
314+
if err := cmd.Wait(); err != nil {
299315
self.Log.Errorf("Unexpected error when running cmd task: %v; Failed command: %v %v", err, cmd.Path, cmd.Args)
300316
}
301317
}

0 commit comments

Comments
 (0)