threads / rfc / 10626

RFC patchMake gitk use --early-output

Subject: [RFC PATCH] Make gitk use --early-output

## tl;dr

8 messages between Nov 3, 2007 and Nov 4, 2007. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Paul Mackerras· Nov 3, 2007, 23:49 UTC · lore
This makes gitk use the --early-output flag on the git log command.

When gitk sees the "Final output:" line from git log, it goes into a mode where it basically just checks that it is getting the commits again in the same order as before. If they are, well and good; if not, it truncates its internal list at the point of difference and proceeds to read in the commits in the new order from there on, and re-does the graph layout if necessary.

This gives a much more immediate feel to the startup; gitk shows its window with the first screenful of commits displayed very quickly this way.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
Show changes to gitk +107 −8
diff --git a/gitk b/gitk
index 1da0b0a..7d9a2f2 100755
--- a/gitk
+++ b/gitk
@@ -84,25 +84,27 @@ proc start_rev_list {view} {
     global commfd leftover tclencoding datemode
     global viewargs viewfiles commitidx viewcomplete vnextroot
     global showlocalchanges commitinterest mainheadid
-    global progressdirn progresscoords proglastnc curview
+    global progressdirn progresscoords proglastnc curview rereading
 
     set startmsecs [clock clicks -milliseconds]
     set commitidx($view) 0
     set viewcomplete($view) 0
     set vnextroot($view) 0
-    set order "--topo-order"
+    set order "--early-output=50"
     if {$datemode} {
-	set order "--date-order"
+	lappend order "--date-order"
     }
     if {[catch {
-	set fd [open [concat | git log --no-color -z --pretty=raw $order --parents \
-			 --boundary $viewargs($view) "--" $viewfiles($view)] r]
+	set fd [open [concat | git log --no-color -z --pretty=raw \
+			  $order --parents --boundary \
+			  $viewargs($view) "--" $viewfiles($view)] r]
     } err]} {
 	error_popup "Error executing git rev-list: $err"
 	exit 1
     }
     set commfd($view) $fd
     set leftover($view) {}
+    set rereading($view) -1
     if {$showlocalchanges} {
 	lappend commitinterest($mainheadid) {dodiffindex}
     }
@@ -161,6 +163,7 @@ proc getcommitlines {fd view}  {
     global parentlist children curview hlview
     global vparentlist vdisporder vcmitlisted
     global ordertok vnextroot idpending
+    global rereading nullid nullid2
 
     set stuff [read $fd 500000]
     # git log doesn't terminate the last commit with a null...
@@ -236,6 +239,15 @@ proc getcommitlines {fd view}  {
 	}
 	set start [expr {$i + 1}]
 	set j [string first "\n" $cmit]
+	if {$j >= 0 && [string match "Final output:*" $cmit]} {
+	    set rereading($view) 0
+	    set cmit [string range $cmit [expr {$j + 1}] end]
+	    set j [string first "\n" $cmit]
+	    if {$view == $curview} {
+		layoutmore
+		update
+	    }
+	}
 	set ok 0
 	set listed 1
 	if {$j >= 0 && [string match "commit *" $cmit]} {
@@ -255,6 +267,7 @@ proc getcommitlines {fd view}  {
 		    break
 		}
 	    }
+	    set cmit [string range $cmit [expr {$j + 1}] end]
 	}
 	if {!$ok} {
 	    set shortcmit $cmit
@@ -265,13 +278,31 @@ proc getcommitlines {fd view}  {
 	    exit 1
 	}
 	set id [lindex $ids 0]
+	if {$rereading($view) >= 0} {
+	    set r $rereading($view)
+	    set oldid [lindex $displayorder $r]
+	    while {$oldid eq $nullid || $oldid eq $nullid2} {
+		set oldid [lindex $displayorder [incr r]]
+	    }
+	    if {$oldid eq $id} {
+		# commits are still in the same order; just skip to the next
+		set rereading($view) [expr {$r + 1}]
+		continue
+	    }
+	    if {$r < $commitidx($view)} {
+		# commits are in a different order now;
+		# truncate the list and redisplay
+		truncate_view $view $r
+	    }
+	    set rereading($view) -1
+	}
 	if {![info exists ordertok($view,$id)]} {
 	    set otok "o[strrep $vnextroot($view)]"
 	    incr vnextroot($view)
 	    set ordertok($view,$id) $otok
 	} else {
 	    set otok $ordertok($view,$id)
-	    unset idpending($view,$id)
+	    catch {unset idpending($view,$id)}
 	}
 	if {$listed} {
 	    set olds [lrange $ids 1 end]
@@ -301,7 +332,7 @@ proc getcommitlines {fd view}  {
 	if {![info exists children($view,$id)]} {
 	    set children($view,$id) {}
 	}
-	set commitdata($id) [string range $cmit [expr {$j + 1}] end]
+	set commitdata($id) $cmit
 	set commitrow($view,$id) $commitidx($view)
 	incr commitidx($view)
 	if {$view == $curview} {
@@ -323,7 +354,7 @@ proc getcommitlines {fd view}  {
     }
     if {$gotsome} {
 	run chewcommits $view
-	if {$view == $curview} {
+	if {0 && $view == $curview} {
 	    # update progress bar
 	    global progressdirn progresscoords proglastnc
 	    set inc [expr {($commitidx($view) - $proglastnc) * 0.0002}]
@@ -354,6 +385,43 @@ proc getcommitlines {fd view}  {
     return 2
 }
 
+proc truncate_view {view row} {
+    global curview commitidx displayorder parentlist commitlisted
+    global vdisporder vparentlist vcmitlisted commitrow children
+    global numcommits localfrow localirow
+
+    set rm1 [expr {$row - 1}]
+    if {$view == $curview} {
+	set disporder $displayorder
+	set displayorder [lrange $disporder 0 $rm1]
+	set parents $parentlist
+	set parentlist [lrange $parents 0 $rm1]
+	set commitlisted [lrange $commitlisted 0 $rm1]
+    } else {
+	set disporder $vdisporder($view)
+	set vdisporder($view) [lrange $disporder 0 $rm1]
+	set parents $vparentlist($view)
+	set vparentlist($view) [lrange $parents 0 $rm1]
+	set vcmitlisted($view) [lrange $vcmitlisted($view) 0 $rm1]
+    }
+    for {set r $commitidx($view)} {[incr r -1] >= $row} {} {
+	set id [lindex $disporder $r]
+	foreach p [lindex $parents $r] {
+	    if {[lindex $children($view,$p) end] eq $id} {
+		set children($view,$p) [lrange $children($view,$p) 0 end-1]
+	    }
+	}
+	unset commitrow($view,$id)
+    }
+    set commitidx($view) $row
+    if {$view == $curview} {
+	truncate_localchanges $row
+	if {$row < $numcommits} {
+	    undolayout $row
+	}
+    }
+}
+
 proc chewcommits {view} {
     global curview hlview viewcomplete
     global selectedline pending_select
@@ -2843,6 +2911,20 @@ proc dohidelocalchanges {} {
     incr lserial
 }
 
+proc truncate_localchanges {row} {
+    global localfrow localirow
+
+    if {$localfrow >= $row} {
+	set localfrow -1
+    }
+    if {$localirow >= $row} {
+	set localirow -1
+    }
+    if {$localfrow == $row - 1 || $localirow == $row - 1} {
+	dohidelocalchanges
+    }
+}
+
 # spawn off a process to do git diff-index --cached HEAD
 proc dodiffindex {} {
     global localirow localfrow lserial showlocalchanges
@@ -3840,6 +3922,23 @@ proc drawcommits {row {endrow {}}} {
     }
 }
 
+proc undolayout {row} {
+    global uparrowlen mingaplen downarrowlen
+    global rowidlist rowisopt rowfinal need_redisplay
+
+    set r [expr {$row - ($uparrowlen + $mingaplen + $downarrowlen)}]
+    if {$r < 0} {
+	set r 0
+    }
+    if {[llength $rowidlist] > $r} {
+	set rowidlist [lrange $rowidlist 0 $r]
+	set rowfinal [lrange $rowfinal 0 $r]
+	set rowisopt [lrange $rowisopt 0 $r]
+	set need_redisplay 1
+	run drawvisible
+    }
+}
+
 proc drawfrac {f0 f1} {
     global canv linespc
Michael J. Cohen· Nov 4, 2007, 02:07 UTC · re: Paul Mackerras · lore

Re: [RFC PATCH] Make gitk use --early-output

On Nov 3, 2007, at 7:49 PM, Paul Mackerras wrote:
> This makes gitk use the --early-output flag on the git log command.
On Nov 3, 2007, at 2:11 PM, Linus Torvalds wrote:
Show 5 quoted lines
> Try it out, with
>
> 	git log --early-output=2
>
> and look at what happens
This is awesome, guys.

I was initially doing some research to see what makes gitk/qgit slow over samba, but this seems to have solved it. :P

-mjc
Linus Torvalds· Nov 4, 2007, 05:30 UTC · re: Paul Mackerras · lore

Re: [RFC PATCH] Make gitk use --early-output

On Sun, 4 Nov 2007, Paul Mackerras wrote:
Show 13 quoted lines
>
> This makes gitk use the --early-output flag on the git log command.
> 
> When gitk sees the "Final output:" line from git log, it goes into a
> mode where it basically just checks that it is getting the commits
> again in the same order as before.  If they are, well and good; if
> not, it truncates its internal list at the point of difference and
> proceeds to read in the commits in the new order from there on, and
> re-does the graph layout if necessary.
> 
> This gives a much more immediate feel to the startup; gitk shows its
> window with the first screenful of commits displayed very quickly this
> way.

Goodie. Seems to work for me. I'll tweak the behaviour of --early-output a bit more, because right now if things are really cold in the cache, the "--early-output" logic will often trigger with just a single commit in the list (because the timeout is so short), but it already seems to work pretty well.

		Linus
Marco Costalba· Nov 4, 2007, 10:37 UTC · re: Paul Mackerras · lore

Re: [RFC PATCH] Make gitk use --early-output

On 11/4/07, Paul Mackerras <paulus@samba.org> wrote:
>
>      set vnextroot($view) 0
> -    set order "--topo-order"
> +    set order "--early-output=50"
But --early-output does not imply --topo-order, I guess...
I would think you need _both_ in git log:
git log --early-output --topo-order <...remaining stuff...>
Am I missing something?
Marco

P.S: Why did you choose not let git log (i.e. Linus) to handle the default number of commits?

"--early-output=50" instead of just "--early-output"

I would say, he added this feature mainly for his personal use, so why don't let him to tweak git-log defaults to his wishes ;-)

Paul Mackerras· Nov 4, 2007, 11:04 UTC · re: Marco Costalba · lore

Re: [RFC PATCH] Make gitk use --early-output

Marco Costalba writes:
Show 7 quoted lines
> On 11/4/07, Paul Mackerras <paulus@samba.org> wrote:
> >
> >      set vnextroot($view) 0
> > -    set order "--topo-order"
> > +    set order "--early-output=50"
> 
> But --early-output does not imply --topo-order, I guess...
Look here in Linus' patch:

+ if (!prefixcmp(arg, "--early-output")) { + int count = 100; + switch (arg[14]) { + case '=': + count = atoi(arg+15); + /* Fallthrough */ + case 0: + revs->topo_order = 1; + revs->early_output = count; + continue; + } + }

So yes, --early-output does imply --topo-order.
> P.S: Why did you choose not let git log (i.e. Linus) to handle the
> default number of commits?
> 
> "--early-output=50" instead of just "--early-output"

Because I was thinking of adding a control in the edit/preferences window for it later on.

Paul.
Marco Costalba· Nov 4, 2007, 11:57 UTC · re: Paul Mackerras · lore

Re: [RFC PATCH] Make gitk use --early-output

On 11/4/07, Paul Mackerras <paulus@samba.org> wrote:
>
> So yes, --early-output does imply --topo-order.
>
Thanks, I should have checked myself.
Show 8 quoted lines
> > P.S: Why did you choose not let git log (i.e. Linus) to handle the
> > default number of commits?
> >
> > "--early-output=50" instead of just "--early-output"
>
> Because I was thinking of adding a control in the edit/preferences
> window for it later on.
>

I see. Perhaps this default number could become obsolete very quickly if Linus implements what he has suggested in a similar thread:

"One other thing I was thinking of was also to perhaps allow multiple partial early-output things, in case we get just 5 commits in the first 0.1 seconds, then 50 in the first second, and 200 after 2 seconds.. I can well imagine getting the full list taking a long time over a network filesystem (somebody mentioned samba), and maybe having just a single trigger is too inflexible."

One thing I see playing with this new --early-output feature in qgit is that for small /warm cache repos the list of revisions is already the final one, i.e. the line

"Final output"

appears as the first (and useless in this case) line of the git-log output stream.

If my proposal to teach git-log to check the final output revisions against the already outputted one is accepted then the handling of the above case would come free.

The proposal is that in case early-output has already streamed out 'n' revisions, when the final ones are ready git-log checks the firsts 'n' final output revisions and if they exactly match with the already outputted ones then "Final output" line is skipped and final output stream starts directly from revisions 'n+1'.

Given the statistically very low number of out of order revisions in big repos the above could end up being the common case.

Marco
Linus Torvalds· Nov 4, 2007, 17:53 UTC · re: Marco Costalba · lore

Re: [RFC PATCH] Make gitk use --early-output

On Sun, 4 Nov 2007, Marco Costalba wrote:
> 
> But --early-output does not imply --topo-order, I guess...

Well, it does right now, because I imagined that the primary users would always want the topological sort.

However, I have to admit that --early-output *could* be used even without the topological sort, because it also works for other cases that require up-front limiter logic - things like ranges of commits also have to be fully evaluated before they are totally certain, so I could imagine seeing some visualizer some day that doesn't need the topo-order sort, but does want to get a "preliminary" list.

That said, it does seem unlikely. Anybody who asks for --early-output is pretty much invariably going to be an interactive visulizer: the whole notion doesn't make much sense otherwise. So I think I made the right choice in making --early-output imply topo-order, and if somebody ever wants to not get the output topologically sorted (unlikely), we could add a "--no-topo-order" flag.

Side note: if you want the "--date-order", you do need to specify *both* --early-output and --date-order, and it will do the right thing (ie both the preliminary output and the final one will be topologically sorted, but within that topo-sort it will be in date order rather than clumped by the "shape" of the history).

			Linus
David Kastrup· Nov 4, 2007, 18:28 UTC · re: Paul Mackerras · lore

Re: [RFC PATCH] Make gitk use --early-output

Paul Mackerras <paulus@samba.org> writes:
Show 12 quoted lines
> This makes gitk use the --early-output flag on the git log command.
>
> When gitk sees the "Final output:" line from git log, it goes into a
> mode where it basically just checks that it is getting the commits
> again in the same order as before.  If they are, well and good; if
> not, it truncates its internal list at the point of difference and
> proceeds to read in the commits in the new order from there on, and
> re-does the graph layout if necessary.
>
> This gives a much more immediate feel to the startup; gitk shows its
> window with the first screenful of commits displayed very quickly this
> way.

This is not strictly related with the patch: would it be possible to let gitk just stall reading from git-rev-list if it has rendered enough content on-screen? The behavior I have with gitk on enormous repositories now is that it starts up reasonably fast and nice and then proceeds to suck up all memory in the background.

Particularly annoying is that closing its window appears to work, but wish will still proceed sucking up all the pending git-rev-list output and allocating memory for it before it will actually exit.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

← back to recent threads