[PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories

[PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories

[Alexander Gavrilov]

Full copy detection can take quite some time on large repositories, which
substantially decreases perceived usability of the 'git gui blame' command.
This set of patches tries to overcome it by:

1) Allowing the user to disable '-C -C' and/or set the detection threshold.

2) Explicitly killing back-end processes, which don't produce any output
  during copy detection, and thus normally won't receive SIGPIPE until
  it is finished. Runaway processes are annoying.

3) To compensate for (1), adding a context menu item to manually invoke
  blame -C -C -C on a group of lines.

Alexander Gavrilov (3):
      Add options to control the search for copies in blame.
      Kill the blame back-end on window close.
      Add a menu item to invoke full copy detection in blame.

 git-gui.sh     |   16 ++++++++
 lib/blame.tcl  |  105 +++++++++++++++++++++++++++++++++++++++++++++++++------
 lib/option.tcl |    2 +
 3 files changed, 111 insertions(+), 12 deletions(-)

[PATCH (GIT-GUI) 1/3] Add options to control the search for copies in blame.

[Alexander Gavrilov]

On huge repositories, -C -C can be way too slow to be
unconditionally enabled, and it can also be useful to control
its precision.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 git-gui.sh     |    2 ++
 lib/blame.tcl  |   20 ++++++++++++--------
 lib/option.tcl |    2 ++
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index e3b6669..b1ed0ec 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -642,6 +642,8 @@ set default_config(user.email) {}
 set default_config(gui.matchtrackingbranch) false
 set default_config(gui.pruneduringfetch) false
 set default_config(gui.trustmtime) false
+set default_config(gui.fastcopyblame) false
+set default_config(gui.copyblamethreshold) 40
 set default_config(gui.diffcontext) 5
 set default_config(gui.commitmsgwidth) 75
 set default_config(gui.newbranchtemplate) {}
diff --git a/lib/blame.tcl b/lib/blame.tcl
index 92fac1b..192505d 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -33,13 +33,6 @@ variable group_colors {
 	#ececec
 }
 
-# Switches for original location detection
-#
-variable original_options [list -C -C]
-if {[git-version >= 1.5.3]} {
-	lappend original_options -w ; # ignore indentation changes
-}
-
 # Current blame data; cleared/reset on each load
 #
 field commit               ; # input commit to blame
@@ -511,7 +504,6 @@ method _exec_blame {cur_w cur_d options cur_s} {
 method _read_blame {fd cur_w cur_d} {
 	upvar #0 $cur_d line_data
 	variable group_colors
-	variable original_options
 
 	if {$fd ne $current_fd} {
 		catch {close $fd}
@@ -684,6 +676,18 @@ method _read_blame {fd cur_w cur_d} {
 	if {[eof $fd]} {
 		close $fd
 		if {$cur_w eq $w_asim} {
+			# Switches for original location detection
+			set threshold [get_config gui.copyblamethreshold]
+			set original_options [list "-C$threshold"]
+
+			if {![is_config_true gui.fastcopyblame]} {
+				# thorough copy search; insert before the threshold
+				set original_options [linsert $original_options 0 -C]
+			}
+			if {[git-version >= 1.5.3]} {
+				lappend original_options -w ; # ignore indentation changes
+			}
+
 			_exec_blame $this $w_amov @amov_data \
 				$original_options \
 				[mc "Loading original location annotations..."]
diff --git a/lib/option.tcl b/lib/option.tcl
index 9270512..ffb3f00 100644
--- a/lib/option.tcl
+++ b/lib/option.tcl
@@ -123,6 +123,8 @@ proc do_options {} {
 		{b gui.trustmtime  {mc "Trust File Modification Timestamps"}}
 		{b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}}
 		{b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
+		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
+		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
 		{i-0..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
 		{i-0..99 gui.commitmsgwidth {mc "Commit Message Text Width"}}
 		{t gui.newbranchtemplate {mc "New Branch Name Template"}}
-- 
1.5.6.3.17.g3f148

[PATCH (GIT-GUI) 2/3] Kill the blame back-end on window close.

[Alexander Gavrilov]

Currently 'git-gui blame' does not kill its back-end
process, hoping that it will die anyway when the pipe
is closed. However, in some cases the process works
for a long time without producing any output. This
behavior results in a runaway CPU hog.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	The -f flag is necessary for msysgit.

	For this fix I submitted to msysgit a patch that includes
	a Cygwin-compatible kill.exe in the installer.

	-- Alexander

 git-gui.sh    |   14 ++++++++++++++
 lib/blame.tcl |   16 ++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index b1ed0ec..83e2645 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -497,6 +497,20 @@ proc githook_read {hook_name args} {
 	return {}
 }
 
+proc kill_file_process {fd} {
+	set process [pid $fd]
+
+	catch {
+		if {[is_Windows]} {
+			# Use a Cygwin-specific flag to allow killing
+			# native Windows processes
+			exec kill -f $process
+		} else {
+			exec kill $process
+		}
+	}
+}
+
 proc sq {value} {
 	regsub -all ' $value "'\\''" value
 	return "'$value'"
diff --git a/lib/blame.tcl b/lib/blame.tcl
index 192505d..2c19048 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -326,19 +326,27 @@ constructor new {i_commit i_path} {
 	bind $w.file_pane <Configure> \
 	"if {{$w.file_pane} eq {%W}} {[cb _resize %h]}"
 
+	wm protocol $top WM_DELETE_WINDOW "destroy $top"
+	bind $top <Destroy> [cb _kill]
+
 	_load $this {}
 }
 
+method _kill {} {
+	if {$current_fd ne {}} {
+		kill_file_process $current_fd
+		catch {close $current_fd}
+		set current_fd {}
+	}
+}
+
 method _load {jump} {
 	variable group_colors
 
 	_hide_tooltip $this
 
 	if {$total_lines != 0 || $current_fd ne {}} {
-		if {$current_fd ne {}} {
-			catch {close $current_fd}
-			set current_fd {}
-		}
+		_kill $this
 
 		foreach i $w_columns {
 			$i conf -state normal
-- 
1.5.6.3.17.g3f148

[PATCH (GIT-GUI) 3/3] Add a menu item to invoke full copy detection in blame.

[Alexander Gavrilov]

Add a context menu item to invoke blame -C -C -C on a chunk
of the file. The results are used to update the 'original
location' column of the blame display.

The chunk is computed as the smallest line range that covers
both the 'last change' and 'original location' ranges of the
line that was clicked to open the menu.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	This is my most complex Tcl/Tk code so far, so I might have
	done some stupid things.

	-- Alexander

 lib/blame.tcl |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/lib/blame.tcl b/lib/blame.tcl
index 2c19048..b6e42cb 100644
--- a/lib/blame.tcl
+++ b/lib/blame.tcl
@@ -256,6 +256,9 @@ constructor new {i_commit i_path} {
 	$w.ctxm add command \
 		-label [mc "Copy Commit"] \
 		-command [cb _copycommit]
+	$w.ctxm add command \
+		-label [mc "Do Full Copy Detection"] \
+		-command [cb _fullcopyblame]
 
 	foreach i $w_columns {
 		for {set g 0} {$g < [llength $group_colors]} {incr g} {
@@ -708,6 +711,72 @@ method _read_blame {fd cur_w cur_d} {
 	}
 } ifdeleted { catch {close $fd} }
 
+method _find_commit_bound {data_list start_idx delta} {
+	upvar #0 $data_list line_data
+	set pos $start_idx
+	set limit       [expr {[llength $line_data] - 1}]
+	set base_commit [lindex $line_data $pos 0]
+
+	while {$pos > 0 && $pos < $limit} {
+		set new_pos [expr {$pos + $delta}]
+		if {[lindex $line_data $new_pos 0] ne $base_commit} {
+			return $pos
+		}
+
+		set pos $new_pos
+	}
+
+	return $pos
+}
+
+method _fullcopyblame {} {
+	if {$current_fd ne {}} {
+		tk_messageBox \
+			-icon error \
+			-type ok \
+			-title [mc "Busy"] \
+			-message [mc "Annotation process is already running."]
+
+		return
+	}
+
+	# Switches for original location detection
+	set threshold [get_config gui.copyblamethreshold]
+	set original_options [list -C -C "-C$threshold"]
+
+	if {[git-version >= 1.5.3]} {
+		lappend original_options -w ; # ignore indentation changes
+	}
+
+	# Find the line range
+	set pos @$::cursorX,$::cursorY
+	set lno [lindex [split [$::cursorW index $pos] .] 0]
+	set min_amov_lno [_find_commit_bound $this @amov_data $lno -1]
+	set max_amov_lno [_find_commit_bound $this @amov_data $lno 1]
+	set min_asim_lno [_find_commit_bound $this @asim_data $lno -1]
+	set max_asim_lno [_find_commit_bound $this @asim_data $lno 1]
+
+	if {$min_asim_lno < $min_amov_lno} {
+		set min_amov_lno $min_asim_lno
+	}
+
+	if {$max_asim_lno > $max_amov_lno} {
+		set max_amov_lno $max_asim_lno
+	}
+
+	lappend original_options -L "$min_amov_lno,$max_amov_lno"
+
+	# Clear lines
+	for {set i $min_amov_lno} {$i <= $max_amov_lno} {incr i} {
+		lset amov_data $i [list ]
+	}
+
+	# Start the back-end process
+	_exec_blame $this $w_amov @amov_data \
+		$original_options \
+		[mc "Running thorough copy detection..."]
+}
+
 method _click {cur_w pos} {
 	set lno [lindex [split [$cur_w index $pos] .] 0]
 	_showcommit $this $cur_w $lno
-- 
1.5.6.3.17.g3f148

Re: [PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories

[Shawn O. Pearce]

Alexander Gavrilov <angavrilov@gmail.com> wrote:
> Full copy detection can take quite some time on large repositories, which
> substantially decreases perceived usability of the 'git gui blame' command.
> This set of patches tries to overcome it by:
> 
> 1) Allowing the user to disable '-C -C' and/or set the detection threshold.
> 
> 2) Explicitly killing back-end processes, which don't produce any output
>   during copy detection, and thus normally won't receive SIGPIPE until
>   it is finished. Runaway processes are annoying.
> 
> 3) To compensate for (1), adding a context menu item to manually invoke
>   blame -C -C -C on a group of lines.

This series is nicely done.  Works well on revision.c in git.git,
where the blame can be costly to compute with full copy detection.
And getting the incremental from the context menu also works well.

Thanks.
 
-- 
Shawn.

Re: [PATCH (GIT-GUI) 0/3] Improve gui blame usability for large repositories

[Alexander Gavrilov]

Hello,

On Thursday 17 July 2008 06:16:23 Shawn O. Pearce wrote:
> Alexander Gavrilov <angavrilov@gmail.com> wrote:
> > Full copy detection can take quite some time on large repositories, which
> > substantially decreases perceived usability of the 'git gui blame' command.
> > This set of patches tries to overcome it by:
> > 
> > 1) Allowing the user to disable '-C -C' and/or set the detection threshold.
> > 
> > 2) Explicitly killing back-end processes, which don't produce any output
> >   during copy detection, and thus normally won't receive SIGPIPE until
> >   it is finished. Runaway processes are annoying.
> > 
> > 3) To compensate for (1), adding a context menu item to manually invoke
> >   blame -C -C -C on a group of lines.
> 
> This series is nicely done.  Works well on revision.c in git.git,
> where the blame can be costly to compute with full copy detection.
> And getting the incremental from the context menu also works well.

I also thought that it would be useful to be able to specify the line range explicitly,
but couldn't invent a good UI for it. Selecting lines with the mouse also causes a
commit to be selected and recolored in green, and popping up a dialog to request
line numbers seems too lame.

Alexander