Re: [PATCH 7/9] fetch: fetch submodules in parallel

Jonathan Nieder [off-list ref] writes:

Junio C Hamano wrote:

quoted

My preference is still (1) leave standard error output all connected
to the same fd without multiplexing, and (2) line buffer standard
output so that the output is at least readable as a text, in a
similar way a log of an irc channel where everybody is talking at
the same time.

There is something nice about the immediacy of seeing output from all
the subprocesses at the same time in that model.

But for commands that show progress like "git clone", "git checkout",
and "git fetch", it does not work well at all.  They provide output
that updates itself by putting a carriage return at the end of each
chunk of output, like this:

 remote: Finding sources:  11% (18/155)           \r
 remote: Finding sources:  12% (19/155)           \r

With multiple commands producing such output, they will overwrite each
other's lines, producing a mixture that is confusing and unuseful.

That example also illustrates why it is not a useful to buffer all
of these lines and showing them once.

Ideally what I as a user want to see is something like what "prove"
writes, showing progress on the multiple tasks that are taking place
at once:

 ===(     103;1  0/?  8/?  3/?  11/?  6/?  16/?  1/?  1/? )==============

Tell me how that "buffer all and show them once" helps us to get
near that ideal.

That would require more sophisticated inter-process communication than
seems necessary for the first version of parallel "git submodule
update".

Exactly.  Why waste memory to buffer and stall the entire output
from other processes in the interim solution, then?

On Fri, Aug 28, 2015 at 11:27:04AM -0700, Junio C Hamano wrote:

quoted

But for commands that show progress like "git clone", "git checkout",
and "git fetch", it does not work well at all.  They provide output
that updates itself by putting a carriage return at the end of each
chunk of output, like this:

 remote: Finding sources:  11% (18/155)           \r
 remote: Finding sources:  12% (19/155)           \r

With multiple commands producing such output, they will overwrite each
other's lines, producing a mixture that is confusing and unuseful.

That example also illustrates why it is not a useful to buffer all
of these lines and showing them once.

I think Jonathan's point is that you could pick _one_ active child to
show without buffering, while simultaneously buffering everybody else's
output. When that finishes, pick a new active child, show its buffer,
and then start showing its output in realtime. And so on.

So to an observer, it would look like a serial operation, but subsequent
operations after the first would magically go much faster (because
they'd been working and buffering in the background).

And that doesn't require any additional IPC magic (though I am not sure
how we get progress in the first place if the child stderr is a
pipe...).

-Peff

On Fri, Aug 28, 2015 at 11:27 AM, Junio C Hamano [off-list ref] wrote:

Jonathan Nieder [off-list ref] writes:

quoted

Junio C Hamano wrote:

quoted

My preference is still (1) leave standard error output all connected
to the same fd without multiplexing, and (2) line buffer standard
output so that the output is at least readable as a text, in a
similar way a log of an irc channel where everybody is talking at
the same time.

There is something nice about the immediacy of seeing output from all
the subprocesses at the same time in that model.

But for commands that show progress like "git clone", "git checkout",
and "git fetch", it does not work well at all.  They provide output
that updates itself by putting a carriage return at the end of each
chunk of output, like this:

 remote: Finding sources:  11% (18/155)           \r
 remote: Finding sources:  12% (19/155)           \r

With multiple commands producing such output, they will overwrite each
other's lines, producing a mixture that is confusing and unuseful.

That example also illustrates why it is not a useful to buffer all
of these lines and showing them once.

quoted

Ideally what I as a user want to see is something like what "prove"
writes, showing progress on the multiple tasks that are taking place
at once:

 ===(     103;1  0/?  8/?  3/?  11/?  6/?  16/?  1/?  1/? )==============

Tell me how that "buffer all and show them once" helps us to get
near that ideal.

It doesn't, but it looks better than the irc like everybody speaks at
the same time
solution IMHO.

At the very the user may not even be interested in the details similar
to what prove
provides. The client can estimate its own total progress by weighting
the progress
from all tasks.

quoted

That would require more sophisticated inter-process communication than
seems necessary for the first version of parallel "git submodule
update".

Exactly.  Why waste memory to buffer and stall the entire output
from other processes in the interim solution, then?

On Fri, Aug 28, 2015 at 11:35 AM, Jeff King [off-list ref] wrote:

On Fri, Aug 28, 2015 at 11:27:04AM -0700, Junio C Hamano wrote:

quoted

quoted

But for commands that show progress like "git clone", "git checkout",
and "git fetch", it does not work well at all.  They provide output
that updates itself by putting a carriage return at the end of each
chunk of output, like this:

 remote: Finding sources:  11% (18/155)           \r
 remote: Finding sources:  12% (19/155)           \r

With multiple commands producing such output, they will overwrite each
other's lines, producing a mixture that is confusing and unuseful.

That example also illustrates why it is not a useful to buffer all
of these lines and showing them once.

I think Jonathan's point is that you could pick _one_ active child to
show without buffering, while simultaneously buffering everybody else's
output. When that finishes, pick a new active child, show its buffer,
and then start showing its output in realtime. And so on.

or better yet, pick that child with the most progress (i.e. flush all finished
children and then pick the next active child), that would approximate
the progress in the output best, as it would reduce the hidden
buffered progress.

So to an observer, it would look like a serial operation, but subsequent
operations after the first would magically go much faster (because
they'd been working and buffering in the background).

And that doesn't require any additional IPC magic (though I am not sure
how we get progress in the first place if the child stderr is a
pipe...).

Moving the contents from the pipe to a strbuf buffer which we can grow
indefinitely
(way larger than pipe limits, but the output of a git fetch should be
small enough for that).

-Peff

Junio C Hamano wrote:

Jonathan Nieder [off-list ref] writes:

quoted

 remote: Finding sources:  11% (18/155)           \r
 remote: Finding sources:  12% (19/155)           \r

With multiple commands producing such output, they will overwrite each
other's lines, producing a mixture that is confusing and unuseful.

That example also illustrates why it is not a useful to buffer all
of these lines and showing them once.

I don't completely follow.  Are you referring to the wasted memory to
store the line that is going to be written and rewritten or some other
aspect?

Today (without parallelism), if I clone a repository with multiple
submodules and walk away from the terminal, then when I get back I see

	Cloning into 'plugins/cookbook-plugin'...
	remote: Counting objects: 36, done
	remote: Finding sources: 100% (36/36)
	remote: Total 1192 (delta 196), reused 1192 (delta 196)
	Receiving objects: 100% (1192/1192), 239.46 KiB | 0 bytes/s, done.
	Resolving deltas: 100% (196/196), done.
	Checking connectivity... done.
	Submodule path 'plugins/cookbook-plugin': checked out 'b9d3ca8a65030071e28be19296ba867ab424fbbf'
	Cloning into 'plugins/download-commands'...
	remote: Counting objects: 37, done
	remote: Finding sources: 100% (37/37)
	remote: Total 448 (delta 46), reused 448 (delta 46)
	Receiving objects: 100% (448/448), 96.13 KiB | 0 bytes/s, done.
	Resolving deltas: 100% (46/46), done.
	Checking connectivity... done.
	Submodule path 'plugins/download-commands': checked out '99e61fb06a4505a9558c23a56213cb32ceaa9cca'
	...

The output for each submodule is in one chunk and I can understand what
happened in each.

By contrast, with inter-mixing the speed of output tells you something
about whether the process is stalled while you stare at the screen and
wait for it to finish (good) but the result on the screen is
unintelligible when the process finishes (bad).

Showing interactive output for one task still provides the real-time
feedback about whether it is completely stuck and needs to be cancelled.
It is easier to make sense of even from the point of view of real-time
progress than the intermixed output.

What problem is the intermixed output meant to solve?

In other words, what is your objection about?  Perhaps there is a way to
both satisfy that objection and to have output clumped per task, but it
is hard to know without knowing what the objection is.

Hope that helps,
Jonathan

On Fri, Aug 28, 2015 at 11:41:17AM -0700, Stefan Beller wrote:

quoted

So to an observer, it would look like a serial operation, but subsequent
operations after the first would magically go much faster (because
they'd been working and buffering in the background).

And that doesn't require any additional IPC magic (though I am not sure
how we get progress in the first place if the child stderr is a
pipe...).

Moving the contents from the pipe to a strbuf buffer which we can grow
indefinitely
(way larger than pipe limits, but the output of a git fetch should be
small enough for that).

Right, clearly we can't rely on pipe buffers to be large enough here
(though we _may_ want to rely on tempfiles if we aren't sure that the
stdout is bounded in a reasonable way).

But what I meant was: the child will only show progress if stderr is a
tty, but here it is not.

I wonder if we need to set GIT_STDERR_IS_TTY=1 in the parent process,
and then respect it in the children (this is similar to what
GIT_PAGER_IN_USE does for stdout).

-Peff

Jeff King wrote:

I think Jonathan's point is that you could pick _one_ active child to
show without buffering, while simultaneously buffering everybody else's
output.

Yep.  Thanks for interpreting.

[...]

So to an observer, it would look like a serial operation, but subsequent
operations after the first would magically go much faster (because
they'd been working and buffering in the background).

Yes.

Thanks,
Jonathan

Jeff King wrote:

Right, clearly we can't rely on pipe buffers to be large enough here
(though we _may_ want to rely on tempfiles if we aren't sure that the
stdout is bounded in a reasonable way).

But what I meant was: the child will only show progress if stderr is a
tty, but here it is not.

For clone / fetch, we can pass --progress explicitly.

For some reason 'git checkout' doesn't support a --progress option.  I
suppose it should. ;-)

Thanks,
Jonathan

On Fri, Aug 28, 2015 at 11:50:50AM -0700, Jonathan Nieder wrote:

quoted

But what I meant was: the child will only show progress if stderr is a
tty, but here it is not.

For clone / fetch, we can pass --progress explicitly.

For some reason 'git checkout' doesn't support a --progress option.  I
suppose it should. ;-)

Yeah, that will work for those tools, but I thought you could pass
arbitrary shell commands.  It would be nice if git sub-commands run
through those just magically worked, even though we don't have an
opportunity to change their command-line parameters.

-Peff

On Fri, Aug 28, 2015 at 11:44 AM, Jeff King [off-list ref] wrote:

On Fri, Aug 28, 2015 at 11:41:17AM -0700, Stefan Beller wrote:

quoted

quoted

So to an observer, it would look like a serial operation, but subsequent
operations after the first would magically go much faster (because
they'd been working and buffering in the background).

And that doesn't require any additional IPC magic (though I am not sure
how we get progress in the first place if the child stderr is a
pipe...).

Moving the contents from the pipe to a strbuf buffer which we can grow
indefinitely
(way larger than pipe limits, but the output of a git fetch should be
small enough for that).

Right, clearly we can't rely on pipe buffers to be large enough here
(though we _may_ want to rely on tempfiles if we aren't sure that the
stdout is bounded in a reasonable way).

But what I meant was: the child will only show progress if stderr is a
tty, but here it is not.

Oh, I forgot about that.

I wonder if we need to set GIT_STDERR_IS_TTY=1 in the parent process,
and then respect it in the children (this is similar to what
GIT_PAGER_IN_USE does for stdout).

The use of GIT_PAGER_IN_USE looks straightforward to me. I'll try to add
GIT_STDERR_IS_TTY then.

-Peff

On Fri, Aug 28, 2015 at 11:53 AM, Jeff King [off-list ref] wrote:

On Fri, Aug 28, 2015 at 11:50:50AM -0700, Jonathan Nieder wrote:

quoted

quoted

But what I meant was: the child will only show progress if stderr is a
tty, but here it is not.

For clone / fetch, we can pass --progress explicitly.

For some reason 'git checkout' doesn't support a --progress option.  I
suppose it should. ;-)

Yeah, that will work for those tools, but I thought you could pass
arbitrary shell commands.  It would be nice if git sub-commands run
through those just magically worked, even though we don't have an
opportunity to change their command-line parameters.

Technically speaking this discusses the patch for fetch only and we'd
want to have that discussion at the previous patch. ;)
But as both of them use the same code, the sync feature of run-command,
which is in patch 5/9, we want to have git commands just work anyway.

-Peff