git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>, git@vger.kernel.org
Subject: Re: [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells
Date: Mon, 10 Jun 2019 19:56:42 +0200	[thread overview]
Message-ID: <20190610175642.GB4012@szeder.dev> (raw)
In-Reply-To: <xmqq36khnyky.fsf@gitster-ct.c.googlers.com>

On Mon, Jun 10, 2019 at 10:23:57AM -0700, Junio C Hamano wrote:
> 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... :)


  reply	other threads:[~2019-06-10 17:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-10  8:58 [PATCH 0/3] Reduce number of processes spawned by git-mergetool Johannes Sixt
2019-06-10  8:58 ` [PATCH 1/3] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt
2019-06-10  9:59   ` SZEDER Gábor
2019-06-10 17:23     ` Junio C Hamano
2019-06-10 17:56       ` SZEDER Gábor [this message]
2019-06-10 18:29     ` Johannes Schindelin
2019-06-10 18:57       ` SZEDER Gábor
2019-06-10  8:58 ` [PATCH 2/3] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt
2019-06-10 17:17   ` Junio C Hamano
2019-06-10 21:34     ` Johannes Sixt
2019-06-10  8:59 ` [PATCH 3/3] mergetool: use shell variable magic instead of `awk` Johannes Sixt
2019-06-10 17:21   ` Junio C Hamano
2019-06-10 22:01     ` Johannes Sixt
2019-06-12 16:33 ` [PATCH v2 0/4] Reduce number of processes spawned by git-mergetool Johannes Sixt
2019-06-12 16:33   ` [PATCH v2 1/4] t7610-mergetool: do not place pipelines headed by `yes` in subshells Johannes Sixt
2019-06-12 16:33   ` [PATCH v2 2/4] t7610-mergetool: use test_cmp instead of test $(cat file) = $txt Johannes Sixt
2019-06-12 16:33   ` [PATCH v2 3/4] mergetool: dissect strings with shell variable magic instead of `expr` Johannes Sixt
2019-06-12 16:33   ` [PATCH v2 4/4] mergetool: use shell variable magic instead of `awk` Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190610175642.GB4012@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).