On 10/10/19(Thu) 10:35, Stefan Sperling wrote:
> On Thu, Oct 10, 2019 at 10:01:35AM +0200, Martin Pieuchot wrote:
> > I'm using "tog log -c master /sys" then search for 'mpi' to check what I
> > did during the 6.6 release cycle.
> > 
> > Due to the fact that 'tog log' do not prefetch more commits than it can
> > display, launching it is very fast.  However every time it continues to
> > search "past" the already unpacked commits, it waits for got-read-pack.
> > 
> > This makes searching in big repositories, like openbsd-src, frustrating.
> > One has to wait more than a couple of seconds every time 'n' is pressed.
> > 
> > Without going the tig(1) way of loading everything upon startup, which
> > makes it slow on big repository, would it be possible to improve the
> > search experiences?
> 
> We can keep loading more commits while a search is active.
> Please try the diff below.
>  
> > Another related suggestion, would it be possible to invert the contrast
> > of the searched terms, like it is done in less(1)?
> 
> I would like this, too. I am not sure how much work is involved.
> It would require touching the show() function of each view that
> supports search (log, blame, tree). Locating the matched substring
> seems to be possible with the REG_STARTEND feature of regexec(3).
> 
> 
> Diff to pre-load more commits during search in the log view.

With this diff, tog(1) display 48 commits upon start, just as before.

After the first search '/mpi', which finds and entry a couple of commits
below, got-read-pack starts consuming ~100% of CPU (as reported by top(1))
and 24624 commits are loaded during 45 seconds.  The fans even start :)

If I press 'n' after got-read-pack is finished with loading its commits,
tog(1) jumps to the next entry without doing any more work.  Your
solution looks like a big hammer to me, however my machine isn't slow.

Note that a similar problem is present in non-search mode when one goes
back through the history.  Pressing 'arrow-down' stalls until
got-read-pack has finished.  This diff doesn't address the problem.

What I'm trying to point here is that this looks like a usual 'prefetch'
case.  We just need to prefetch "enough" commits to have a nice user
experience when displaying next commits.  Defining "enough" is the
thought part as it depends on the speed of the machine.  Your approach
seems to define "enough" as "everything" (o:

In the case of history scrolling, "enough" might be the number of
commits displayable at once, or twice that quantity.  Maybe that's good
enough for search as well, or we can prefetch 4K commits... whatever.

That said, I'd suggest to test this diff on a miod-grade machine, to see
if it isn't too slow :)

> diff d59c0cb27bc304bc11f9b1094c6eb85f248c7c5f /home/stsp/src/got
> blob - aad8017a1963e96977883c2dadfcef0fce8eac58
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -1827,12 +1827,23 @@ search_next_log_view(struct tog_view *view)
>  			 * Poke the log thread for more commits and return,
>  			 * allowing the main loop to make progress. Search
>  			 * will resume at s->search_entry once we come back.
> +			 * Prefetch a large number of commits to avoid stalling
> +			 * every time the user hits the 'n' key.
>  			 */
> -			s->thread_args.commits_needed++;
> -			return trigger_log_thread(1,
> +			s->thread_args.commits_needed += 4096;
> +			return trigger_log_thread(0,
>  			    &s->thread_args.commits_needed,
>  			    &s->thread_args.log_complete,
>  			    &s->thread_args.need_commits);
> +		} else if (s->matched_entry == NULL) {
> +			/* Found a first match, kick off commit prefetching. */
> +			s->thread_args.commits_needed += 4096;
> +			err = trigger_log_thread(0,
> +			    &s->thread_args.commits_needed,
> +			    &s->thread_args.log_complete,
> +			    &s->thread_args.need_commits);
> +			if (err)
> +				return err;
>  		}
>  
>  		err = got_object_id_str(&id_str, entry->id);