From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: "git\@vger.kernel.org" <git@vger•kernel.org>,
Ramsay Jones <ramsay@ramsayjones•plus.com>,
Jacob Keller <jacob.keller@gmail•com>, Jeff King <peff@peff•net>,
Jonathan Nieder <jrnieder@gmail•com>,
Johannes Schindelin <johannes.schindelin@gmail•com>,
Jens Lehmann <Jens.Lehmann@web•de>,
Eric Sunshine <ericsunshine@gmail•com>,
Johannes Sixt <j6t@kdbg•org>
Subject: Re: [PATCHv3 02/11] run-command: report failure for degraded output just once
Date: Wed, 04 Nov 2015 12:42:15 -0800 [thread overview]
Message-ID: <xmqq8u6da448.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAGZ79kaiRKHd2RS9eNeZt_VZqqBF0HS0D=x1HbOTPXYOphu8pg@mail.gmail.com> (Stefan Beller's message of "Wed, 4 Nov 2015 12:14:44 -0800")
Stefan Beller <sbeller@google•com> writes:
> Another approach would be to test if we can set to non blocking and if
> that is not possible, do not buffer it, but redirect the subcommand
> directly to stderr of the calling process.
>
> if (set_nonblocking(pp->children[i].process.err) < 0) {
> pp->children[i].process.err = 2;
> degraded_parallelism = 1;
> }
>
> and once we observe the degraded_parallelism flag, we can only
> schedule a maximum of one job at a time, having direct output?
I would even say that on a platform that is _capable_ of setting fd
non-blocking, we should signal a grave error and die if an attempt
to do so fails, period.
On the other hand, on a platform that is known to be incapable
(e.g. lacks SETFL or NONBLOCK), we have two options.
1. If we can arrange to omit the intermediary buffer processing
without butchering the flow of the main logic with many
#ifdef..#endif, then that would make a lot of sense to do so, and
running the processes in parallel with mixed output might be OK.
It may not be very nice, but should be an acceptable compromise.
2. If we need to sprinkle conditional compilation all over the place
to do so, then I do not think it is worth it. Instead, we should
keep a single code structure, and forbid setting numtasks to more
than one, which would also remove the need for nonblock IO.
Either way, bringing "parallelism with sequential output" to
platforms without nonblock IO can be left for a later day, when we
find either (1) a good approach that does not require nonblock IO to
do this, or (2) a good approach to do a nonblock IO on these
platforms (we know about Windows, but there may be others; I dunno).
next prev parent reply other threads:[~2015-11-04 20:42 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-04 0:37 [PATCHv3 00/11] Expose the submodule parallelism to the user Stefan Beller
2015-11-04 0:37 ` [PATCHv3 01/11] run_processes_parallel: delimit intermixed task output Stefan Beller
2015-11-04 0:37 ` [PATCHv3 02/11] run-command: report failure for degraded output just once Stefan Beller
2015-11-04 18:14 ` Junio C Hamano
2015-11-04 20:14 ` Stefan Beller
2015-11-04 20:36 ` Johannes Sixt
2015-11-04 21:01 ` Junio C Hamano
2015-11-04 22:56 ` Jeff King
2015-11-05 2:05 ` Junio C Hamano
2015-11-05 6:51 ` Jeff King
2015-11-05 7:32 ` Junio C Hamano
2015-11-05 17:37 ` Stefan Beller
2015-11-04 20:42 ` Junio C Hamano [this message]
2015-11-04 21:04 ` Stefan Beller
2015-11-04 21:19 ` Junio C Hamano
2015-11-04 21:41 ` Stefan Beller
2015-11-04 0:37 ` [PATCHv3 03/11] run-command: omit setting file descriptors to non blocking in Windows Stefan Beller
2015-11-04 0:37 ` [PATCHv3 04/11] submodule-config: keep update strategy around Stefan Beller
2015-11-04 0:37 ` [PATCHv3 05/11] submodule-config: drop check against NULL Stefan Beller
2015-11-04 0:37 ` [PATCHv3 06/11] submodule-config: remove name_and_item_from_var Stefan Beller
2015-11-04 0:37 ` [PATCHv3 07/11] submodule-config: introduce parse_generic_submodule_config Stefan Beller
2015-11-04 0:37 ` [PATCHv3 08/11] fetching submodules: respect `submodule.jobs` config option Stefan Beller
2015-11-10 22:21 ` Jens Lehmann
2015-11-10 22:29 ` Stefan Beller
2015-11-11 19:55 ` Jens Lehmann
2015-11-11 23:34 ` Stefan Beller
2015-11-13 20:47 ` Jens Lehmann
2015-11-13 21:29 ` Stefan Beller
2015-11-04 0:37 ` [PATCHv3 09/11] git submodule update: have a dedicated helper for cloning Stefan Beller
2015-11-04 0:37 ` [PATCHv3 10/11] submodule update: expose parallelism to the user Stefan Beller
2015-11-04 0:37 ` [PATCHv3 11/11] clone: allow an explicit argument for parallel submodule clones Stefan Beller
2015-11-04 17:54 ` [PATCHv3 00/11] Expose the submodule parallelism to the user Junio C Hamano
2015-11-04 18:08 ` Stefan Beller
2015-11-04 18:17 ` Junio C Hamano
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=xmqq8u6da448.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=Jens.Lehmann@web$(echo .)de \
--cc=ericsunshine@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=j6t@kdbg$(echo .)org \
--cc=jacob.keller@gmail$(echo .)com \
--cc=johannes.schindelin@gmail$(echo .)com \
--cc=jrnieder@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=ramsay@ramsayjones$(echo .)plus.com \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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