git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Couder <christian.couder@gmail.com>
Cc: git@vger.kernel.org, "Jeff King" <peff@peff.net>,
	"Thomas Rast" <tr@thomasrast.ch>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: Re: [PATCH 3/8] perf/aggregate: implement codespeed JSON output
Date: Wed, 13 Dec 2017 12:33:15 -0800	[thread overview]
Message-ID: <xmqqfu8ewepg.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171213151344.2138-4-chriscool@tuxfamily.org> (Christian Couder's message of "Wed, 13 Dec 2017 16:13:39 +0100")

Christian Couder <christian.couder@gmail.com> writes:

>  my $resultsdir = "test-results";
> +my $results_section = "";
>  if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") {
>  	$resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION};
> +	$results_section = $ENV{GIT_PERF_SUBSECTION};
>  }

...

> +	my $executable;
> +	if ($results_section eq "") {
> +		$executable = `uname -o -p`;
> +	} else {
> +		$executable = $results_section;
> +		chomp $executable;
> +	}

Aside from portability of 'uname -o' Eric raised, I wonder if the
platform information is still useful even when perf-subsection is
specified.  With the above code, we can identify that a single
result is for (say) MacOS only when we are not limiting to a single
subsection, but wouldn't it be equally a valid desire to be able to
track performance figures for a single subsection over time and
being able to say "On MacOS, subsection A's performance dropped
between release X and X+1 quite a bit, but on Linux x86-64, there
was no such change" or somesuch?

IOW, shouldn't the "executable" label always contain the platform
information, plus an optional subsection info when (and only when)
the result is limited to a subsection?

By the way, $results_section that is not an empty string at this
point must have come from $ENV{GIT_PERF_SUBSECTION}, and from the
way the environment variable is used in t/perf/run, e.g.

		(
			GIT_PERF_SUBSECTION="$subsec"
			export GIT_PERF_SUBSECTION
			echo "======== Run for subsection '$GIT_PERF_SUBSECTION' ========"
			run_subsection "$@"
		)

I do not think we expect it to have a trailing LF.  What's that
chomp doing there?

  parent reply	other threads:[~2017-12-13 20:33 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13 15:13 [PATCH 0/8] Codespeed perf results Christian Couder
2017-12-13 15:13 ` [PATCH 1/8] perf/aggregate: fix checking ENV{GIT_PERF_SUBSECTION} Christian Couder
2017-12-13 15:13 ` [PATCH 2/8] perf/aggregate: refactor printing results Christian Couder
2017-12-13 15:13 ` [PATCH 3/8] perf/aggregate: implement codespeed JSON output Christian Couder
2017-12-13 18:25   ` Eric Sunshine
2017-12-14  8:47     ` Christian Couder
2017-12-13 20:33   ` Junio C Hamano [this message]
2017-12-14  8:57     ` Christian Couder
2017-12-14 17:31       ` Junio C Hamano
2017-12-13 15:13 ` [PATCH 4/8] perf/run: use $default_value instead of $4 Christian Couder
2017-12-13 20:40   ` Junio C Hamano
2017-12-13 20:54     ` Junio C Hamano
2017-12-14  9:00       ` Christian Couder
2017-12-14  8:59     ` Christian Couder
2017-12-13 15:13 ` [PATCH 5/8] perf/run: add conf_opts argument to get_var_from_env_or_config() Christian Couder
2017-12-13 15:13 ` [PATCH 6/8] perf/run: learn about perf.codespeedOutput Christian Couder
2017-12-13 15:13 ` [PATCH 7/8] perf/run: learn to send output to codespeed server Christian Couder
2017-12-13 15:13 ` [PATCH 8/8] perf/run: read GIT_TEST_REPO_NAME from perf.repoName Christian Couder
2017-12-13 16:02 ` [PATCH 0/8] Codespeed perf results Philip Oakley
2017-12-13 16:18   ` Christian Couder

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=xmqqfu8ewepg.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --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.
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).