All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Jeff King <peff@peff.net>, Stefan Beller <sbeller@google.com>,
	git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
	Thomas Rast <tr@thomasrast.ch>, Phil Haack <haacked@gmail.com>,
	Jason Frey <jfrey@redhat.com>,
	Philip Oakley <philipoakley@iee.org>,
	Duy Nguyen <pclouds@gmail.com>
Subject: Re: A potential approach to making tests faster on Windows
Date: Tue, 3 Apr 2018 13:43:10 +0200 (DST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.1804031150030.5026@qfpub.tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <87fu4hwgfa.fsf@evledraar.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

Hi Ævar,

On Fri, 30 Mar 2018, Ævar Arnfjörð Bjarmason wrote:

> On Fri, Mar 30 2018, Johannes Schindelin wrote [expressing frustrations
> about Windows test suite slowness]:

To be precise (and I think it is important to be precise here): it is not
the Windows test suite about which I talked, it is Git's test suite, as
run on Windows. It might sound like a small difference, but it is not: the
fault really lies with Git because it wants to be a portable software.

> I've wondered for a while whether it wouldn't be a viable approach to
> make something like an interpreter for our test suite to get around this
> problem, i.e. much of it's very repetitive and just using a few shell
> functions we've defined, what if we had C equivalents of those?

There has even been an attempt to do this by Linus Torvalds himself:

https://public-inbox.org/git/Pine.LNX.4.64.0602232229340.3771@g5.osdl.org/

It has not really gone anywhere...

To be honest, I had a different idea (because I do not really want to
maintain yet another piece of software): BusyBox. The source code is clean
enough, and it should, in theory, allow us to go really fast.

> Duy had a WIP patch set a while ago to add C test suite support, but I
> thought what if we turn that inside-out, and instead have a shell
> interpreter that knows about the likes of test_cmp, and executes them
> directly?

The problem, of course, is: if you add Git-test-suite-specific stuff to
any Unix shell, you are going to have to maintain this fork, and all of a
sudden it has become a lot harder to develop Git, and to port it.

Quite frankly, I would rather go with Duy's original approach, or a
variation thereof, as snuck into the wildmatch discussion here:

	https://public-inbox.org/git/20180110090724.GA2893@ash/

> Here's proof of concept as a patch to the dash shell:
> 
>     u dash (debian/master=) $ git diff
>     diff --git a/src/builtins.def.in b/src/builtins.def.in
>     index 4441fe4..b214a17 100644
>     --- a/src/builtins.def.in
>     +++ b/src/builtins.def.in
>     @@ -92,3 +92,4 @@ ulimitcmd     ulimit
>      #endif
>      testcmd                test [
>      killcmd                -u kill
>     +testcmpcmd     test_cmp
>     diff --git a/src/jobs.c b/src/jobs.c
>     index c2c2332..905563f 100644
>     --- a/src/jobs.c
>     +++ b/src/jobs.c
>     @@ -1502,3 +1502,12 @@ getstatus(struct job *job) {
>                     jobno(job), job->nprocs, status, retval));
>             return retval;
>      }
>     +
>     +#include <stdio.h>
>     +int
>     +testcmpcmd(argc, argv)
>     +       int argc;
>     +       char **argv;
>     +{
>     +       fprintf(stderr, "Got %d arguments\n", argc);
>     +}
> 
> I just added that to jobs.c because it was easiest, then test_cmp
> becomes a builtin:
> 
>     u dash (debian/master=) $ src/dash -c 'type test_cmp'
>     test_cmp is a shell builtin
>     u dash (debian/master=) $ src/dash -c 'echo foo && test_cmp 1 2 3'
>     foo
>     Got 4 arguments
> 
> I.e. it's really easy to add new built in commands to the dash shell
> (and probably other shells, but dash is really small & fast).
> 
> We could carry some patch like that to dash, and also patch it so
> test-lib.sh could know that that was our own custom shell, and we'd then
> skip defining functions like test_cmp, and instead use that new builtin.

Or even use the output of `type test_cmp` as a tell-tale.

> Similarly, it could then be linked to our own binaries, and the
> test-tool would be a builtin that would appropriately dispatch, and we
> could even eventually make "git" a shell builtin.
> 
> I don't have time or interest to work on this now, but thought it was
> interesting to share. This assumes that something in shellscript like:
> 
>     while echo foo; do echo bar; done
> 
> Is no slower on Windows than *nix, since it's purely using built-ins, as
> opposed to something that would shell out.

It is still interpreting stuff. And it still goes through the POSIX
emulation layer.

I did see reports on the Git for Windows bug tracker that gave me the
impression that such loops in Unix shell scripts may not, in fact, be as
performant in MSYS2's Bash as you would like to believe:

https://github.com/git-for-windows/git/issues/1533#issuecomment-372025449

Ciao,
Dscho

  parent reply	other threads:[~2018-04-03 11:43 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-29 15:18 [PATCH 0/9] Assorted fixes for `git config` (including the "empty sections" bug) Johannes Schindelin
2018-03-29 15:18 ` [PATCH 1/9] git_config_set: fix off-by-two Johannes Schindelin
2018-03-29 18:15   ` Stefan Beller
2018-03-29 19:41     ` Jeff King
2018-03-30 12:32       ` Johannes Schindelin
2018-03-30 14:15         ` Ævar Arnfjörð Bjarmason
2018-03-30 16:24           ` Junio C Hamano
2018-03-30 18:44             ` Johannes Schindelin
2018-03-30 19:00               ` Junio C Hamano
2018-04-03  9:31                 ` Johannes Schindelin
2018-04-03 15:29                   ` Duy Nguyen
2018-04-03 15:47                     ` Johannes Schindelin
2018-04-08 23:12                   ` Junio C Hamano
2018-03-30 16:36         ` Duy Nguyen
2018-03-30 18:53           ` Johannes Schindelin
2018-03-30 19:16             ` Duy Nguyen
2018-03-30 18:45         ` A potential approach to making tests faster on Windows Ævar Arnfjörð Bjarmason
2018-03-30 18:58           ` Junio C Hamano
2018-03-30 19:16           ` Jeff King
2018-04-03  9:49             ` Johannes Schindelin
2018-04-03 11:28               ` Ævar Arnfjörð Bjarmason
2018-04-03 15:55                 ` Johannes Schindelin
2018-04-03 21:36               ` Eric Sunshine
2018-04-03 11:43           ` Johannes Schindelin [this message]
2018-04-03 13:27             ` Jeff King
2018-04-03 16:00               ` Johannes Schindelin
2018-04-06 21:40                 ` Jeff King
2018-04-06 21:57                   ` Stefan Beller
2018-03-29 15:18 ` [PATCH 2/9] t1300: rename it to reflect that `repo-config` was deprecated Johannes Schindelin
2018-03-29 19:42   ` Jeff King
2018-03-30 12:37     ` Johannes Schindelin
2018-03-29 15:18 ` [PATCH 3/9] t1300: avoid relying on a bug Johannes Schindelin
2018-03-29 19:43   ` Jeff King
2018-03-30 12:38     ` Johannes Schindelin
2018-03-29 15:18 ` [PATCH 4/9] t1300: remove unreasonable expectation from TODO Johannes Schindelin
2018-03-29 19:52   ` Jeff King
2018-03-29 20:45     ` Junio C Hamano
2018-03-30 12:42     ` Johannes Schindelin
2018-03-29 15:18 ` [PATCH 5/9] t1300: `--unset-all` can leave an empty section behind (bug) Johannes Schindelin
2018-03-29 19:54   ` Jeff King
2018-03-29 15:18 ` [PATCH 6/9] git_config_set: simplify the way the section name is remembered Johannes Schindelin
2018-03-29 15:19 ` [PATCH 7/9] git config --unset: remove empty sections (in normal situations) Johannes Schindelin
2018-03-29 21:32   ` Jeff King
2018-03-30 13:00     ` Johannes Schindelin
2018-03-30 13:09       ` Jeff King
2018-03-29 15:19 ` [PATCH 8/9] git_config_set: use do_config_from_file() directly Johannes Schindelin
2018-03-29 21:38   ` Jeff King
2018-03-30 13:02     ` Johannes Schindelin
2018-03-30 13:14       ` Jeff King
2018-03-30 14:01         ` Johannes Schindelin
2018-03-30 14:08           ` Jeff King
2018-03-30 19:04             ` Johannes Schindelin
2018-03-29 15:19 ` [PATCH 9/9] git_config_set: reuse empty sections Johannes Schindelin
2018-03-29 21:50   ` Jeff King
2018-03-30 13:15     ` Johannes Schindelin
2018-03-29 17:58 ` [PATCH 0/9] Assorted fixes for `git config` (including the "empty sections" bug) Stefan Beller
2018-03-30 12:14   ` Johannes Schindelin
2018-03-29 19:39 ` Jeff King
2018-03-30 12:35   ` Johannes Schindelin
2018-03-30 14:17 ` Ævar Arnfjörð Bjarmason
2018-03-30 18:46   ` Johannes Schindelin
2018-04-03 16:27 ` [PATCH v2 00/15] " Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 01/15] git_config_set: fix off-by-two Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 02/15] t1300: rename it to reflect that `repo-config` was deprecated Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 03/15] t1300: demonstrate that --replace-all can "invent" newlines Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 04/15] config --replace-all: avoid extra line breaks Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 05/15] t1300: avoid relying on a bug Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 06/15] t1300: remove unreasonable expectation from TODO Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 07/15] t1300: `--unset-all` can leave an empty section behind (bug) Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 08/15] config: introduce an optional event stream while parsing Johannes Schindelin
2018-04-06 21:22     ` Jeff King
2018-04-09  7:35       ` Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 09/15] config: avoid using the global variable `store` Johannes Schindelin
2018-04-06 21:23     ` Jeff King
2018-04-09  7:36       ` Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 10/15] config_set_store: rename some fields for consistency Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 11/15] git_config_set: do not use a state machine Johannes Schindelin
2018-04-06 21:28     ` Jeff King
2018-04-09  7:50       ` Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 12/15] git_config_set: make use of the config parser's event stream Johannes Schindelin
2018-04-03 16:28   ` [PATCH v2 13/15] git config --unset: remove empty sections (in the common case) Johannes Schindelin
2018-04-03 16:29   ` [PATCH v2 14/15] git_config_set: reuse empty sections Johannes Schindelin
2018-04-03 16:30   ` [PATCH v2 00/15] Assorted fixes for `git config` (including the "empty sections" bug) Johannes Schindelin
2018-04-06 21:33   ` Jeff King
2018-04-09  8:19     ` Johannes Schindelin
2018-04-09  8:31   ` [PATCH v3 " Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 01/15] git_config_set: fix off-by-two Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 02/15] t1300: rename it to reflect that `repo-config` was deprecated Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 03/15] t1300: demonstrate that --replace-all can "invent" newlines Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 04/15] config --replace-all: avoid extra line breaks Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 05/15] t1300: avoid relying on a bug Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 06/15] t1300: remove unreasonable expectation from TODO Johannes Schindelin
2018-04-09  8:31     ` [PATCH v3 07/15] t1300: add a few more hairy examples of sections becoming empty Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 08/15] t1300: `--unset-all` can leave an empty section behind (bug) Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 09/15] config: introduce an optional event stream while parsing Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 10/15] config: avoid using the global variable `store` Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 11/15] config_set_store: rename some fields for consistency Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 12/15] git_config_set: do not use a state machine Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 13/15] git_config_set: make use of the config parser's event stream Johannes Schindelin
2018-05-08 13:42       ` Jeff King
2018-05-08 14:00         ` Jeff King
2018-04-09  8:32     ` [PATCH v3 14/15] git config --unset: remove empty sections (in the common case) Johannes Schindelin
2018-04-09  8:32     ` [PATCH v3 15/15] git_config_set: reuse empty sections Johannes Schindelin

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

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

  git send-email \
    --in-reply-to=nycvar.QRO.7.76.6.1804031150030.5026@qfpub.tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=haacked@gmail.com \
    --cc=jfrey@redhat.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.org \
    --cc=sbeller@google.com \
    --cc=tr@thomasrast.ch \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.