Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jun 10, 2019, 17:56 UTC
Message-ID: <20190610175642.GB4012@szeder.dev>
In-Reply-To: <xmqq36khnyky.fsf@gitster-ct.c.googlers.com>

On Mon, Jun 10, 2019 at 10:23:57AM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > turn these
> >
> >   test "$(cat file1)" = "that"'
> >
> > checks into
> >
> >   echo that >expect &&
> >   test_cmp expect file1
> >
> > because 'test_cmp' on Windows first compares the two files in shell
> > and runs 'diff' only when there is a difference to report.
> 
> Needs measuring.  Is two extra file I/Os that inexpensive?

Compared to subshells and external processes, yes.

I run this on Linux:

  --- >8 ---

#!/bin/sh

test_description='test'

. ./lib-bash.sh

test_expect_success 'test' '
	echo "master updated" >file1 &&

	echo "original:" &&
	time for i in $(seq 1000)
	do
		test "$(cat file1)" = "master updated"
	done &&

	echo "using test_cmp:" &&
	time for i in $(seq 1000)
	do
		echo "master updated" >expect &&
		test_cmp expect file1
	done &&

	echo "using mingw_test_cmp:" &&
	time for i in $(seq 1000)
	do
		echo "master updated" >expect &&
		mingw_test_cmp expect file1
	done
'

test_done

  --- >8 ---

And it produced:

  original:
  
  real    0m1.888s
  user    0m1.491s
  sys     0m0.532s
  using test_cmp:
  
  real    0m1.233s
  user    0m0.877s
  sys     0m0.432s
  using mingw_test_cmp:
  
  real    0m0.344s
  user    0m0.298s
  sys     0m0.026s
  ok 1 - test

It's faster even on Linux, so it should be much faster on Windows, where both external commands and subshells have much higher overhead.

However, there are only about 50 "$(cat ...)" in t7610 that can be eliminated this way, which is not all that much compared to the several thousand processes shown in the cover letter...

Anyway, there is still the better developer experience in case of one of these checks were to fail without '-x' tracing... :)