Re: [PATCH] perf: accommodate for MacOSX

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 21, 2016, 11:55 UTC
Message-ID: <alpine.DEB.2.20.1606211350470.22630@virtualbox>
In-Reply-To: <F67587B5-0EA8-4F2F-AADB-4343B4FEEA21@gmail.com>

Hi Lars,

On Tue, 21 Jun 2016, Lars Schneider wrote:

Show 26 quoted lines

> > On 20 Jun 2016, at 21:48, Junio C Hamano <gitster@pobox.com> wrote:
> > 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> >> On Sun, 19 Jun 2016, Lars Schneider wrote:
> >> 
> >>>> On 18 Jun 2016, at 15:03, Johannes Schindelin
> >>>> <johannes.schindelin@gmx.de> wrote:
> >>>> 
> >>>> As this developer has no access to MacOSX developer setups anymore,
> >>>> Travis becomes the best bet to run performance tests on that OS.
> >>> 
> >>> We don't run the performance tests on Travis CI right now.
> >>> Maybe we should? With your patch below it should work, right?
> >> ...
> >> 
> >> Yeah, well, I should have been clearer in my commit message: this patch
> >> allows the perf tests to *run*, not to *pass*... ;-)
> > 
> > OK, Lars, do we still want to take this patch?  I am leaning towards
> > taking it, if only to motivate interested others with OS X to look
> > into making the perf tests to actually run.
> 
> I think we definitively should take the "perf-lib.sh" part of the patch
> as this makes the perf test run on OSX and therefore is a strict
> improvement.

Yes, it was meant as the starting point to get more things to run on MacOSX.

> If we don't run any perf tests by default on Travis CI then I wouldn't
> take the ".travis.yml" part of the patch just to keep our Travis CI
> setup as lean as possible.

Maybe commented-out, so that people like me have a chance to use Travis for MacOSX perf testing?

> Running perf tests on Travis CI is probably bogus anyways because we
> never know on what hardware our jobs run and what other jobs run in
> parallel on that hardware.

While I agree that the absolute timings cannot be trusted, I have to point out that the relative timings on Linux at least are consistent with what I could test locally.

Could you let me know whether a commented-out

	# Uncomment this if you want to run perf tests:
	# brew install gnu-time

would be acceptable? I will reroll the patch accordingly.

Ciao, Dscho