git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Thomas Rast <trast@inf.ethz.ch>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: "git grep" parallelism question
Date: Tue, 30 Apr 2013 17:12:08 +0100	[thread overview]
Message-ID: <20130430161207.GQ472@serenity.lan> (raw)
In-Reply-To: <20130430155939.GA31881@sigill.intra.peff.net>

On Tue, Apr 30, 2013 at 11:59:39AM -0400, Jeff King wrote:
> On Tue, Apr 30, 2013 at 09:08:49AM +0100, John Keeping wrote:
> 
> > > With your patch, doesn't "tXXXX-*.sh --root $there" automatically
> > > use the fast $there temporary location as the result depot, too?
> > 
> > No, the current code uses:
> > 
> >     $TEST_OUTPUT_DIRECTORY/$root/trash\ directory.tXXXX
> > 
> > where we don't prepend $TEST_OUTPUT_DIRECTORY/ if $root is absolute.
> > 
> > > If it doesn't with the current code, shouldn't it?
> > 
> > I think the current behaviour is fine and the two options complement
> > each other.
> > 
> > TEST_OUTPUT_DIRECTORY is something you set once and forget about which
> > says "all of the test output should go over here", whereas --root is
> > passed to a specific test and says "put your output here" but does not
> > affect the result aggregation which is not specific to that test.
> 
> The original intent of "--root" (and how I use it) is to set and forget
> it, too, via GIT_TEST_OPTS. I intentionally didn't move test results
> with it, because to me the point was a pure optimization: put the trash
> directories on a faster disk, and leave everything else identical.  With
> "--root", any scripts which later want to look at test-results will find
> them in the usual place.
> 
> Your patch updates all of the in-tree spots which look at the results,
> but any third-party scripts would need to take it into account, too
> (though I have no idea if any such scripts even exist).
> 
> I'm curious if there is a good reason to want to move the results. Some
> possibilities I can think of are:
> 
>   1. More optimization, as results are written to the faster filesystem.
>      I doubt this is noticeable, though, as the amount of data written
>      is relatively small compared to the tests themselves (which are
>      constantly creating and deleting repos).
> 
>   2. You can run tests in a read-only git checkout. I'm not sure how
>      useful that is, though, since you would already need to compile
>      git.
> 
>   3. You could have multiple sets of test results to keep or compare.
>      I'd think you'd want to keep the built versions of git around, too,
>      though. Which would mean that a full checkout like git-new-workdir
>      would be a much simpler way to accomplish the same thing.
> 
> So I'm not against TEST_OUTPUT_DIRECTORY as a concept, but I'm having
> trouble seeing how it is more useful than "--root".

I think the original intent of TEST_OUTPUT_DIRECTORY was to allow other
users of the test framework (in contrib/ or the performance tests) to
put their output in a sensible place for those tests, like you describe
below.

The patch being discussed here [1] just makes sure that it applies
to everything - previously it was applied to test-results/
inconsistently; test-lib.sh used TEST_OUTPUT_DIRECTORY but the makefile
didn't.  So we haven't actually changed where test-results/ live as a
result of this change, just where the makefile looks in order to display
the aggregate results and clean them up.

> > Note that setting TEST_OUTPUT_DIRECTORY in config.mak affects all tests
> > no matter how you run them (via make or as ./tXXXX-yyyy.sh) whereas
> > setting --root=... in GIT_TEST_OPTS only affect tests run via make.
> 
> I actually consider that a feature of "--root". When I run "make test"
> everything happens fast. When I run the script manually (which is
> usually because I'm debugging), the trash directory appears in the
> current directory, so I can easily investigate it. And if you are
> running a single test, the performance impact is usually negligible
> (where you really notice it is when running "make -j32 test").

This confirms to me that the patch as it currently stands is correct: we
have made TEST_OUTPUT_DIRECTORY consistent and --root still works as
before.

[1] http://article.gmane.org/gmane.comp.version-control.git/222555

  reply	other threads:[~2013-04-30 16:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-26 17:31 "git grep" parallelism question Linus Torvalds
2013-04-26 18:47 ` Junio C Hamano
2013-04-26 18:54   ` Linus Torvalds
2013-04-26 19:19     ` Junio C Hamano
2013-04-26 20:31       ` Linus Torvalds
2013-04-27 13:46         ` Thomas Rast
2013-04-29 14:05         ` Ramkumar Ramachandra
2013-04-29 16:18           ` John Keeping
2013-04-29 18:04             ` Thomas Rast
2013-04-29 18:08               ` John Keeping
2013-04-29 22:22                 ` Junio C Hamano
2013-04-30  8:08                   ` John Keeping
2013-04-30 15:59                     ` Jeff King
2013-04-30 16:12                       ` John Keeping [this message]
2013-04-30 16:14                         ` Jeff King
2013-05-05 15:40         ` Pete Wyckoff

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=20130430161207.GQ472@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=torvalds@linux-foundation.org \
    --cc=trast@inf.ethz.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 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 NNTP newsgroup(s).