git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH v2 1/5] merge-tree --stdin: flush stdout to avoid deadlock
Date: Tue, 18 Feb 2025 16:24:35 +0000	[thread overview]
Message-ID: <3b3179785098580f3336bb24bdbaf0aa1366bfcd.1739895879.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1862.v2.git.1739895879.gitgitgadget@gmail.com>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If a process tries to read the output from "git merge-tree --stdin"
before it closes merge-tree's stdin then it deadlocks. This happens
because merge-tree does not flush its output before trying to read
another line of input and means that it is not possible to cherry-pick a
sequence of commits using "git merge-tree --stdin". Fix this by calling
maybe_flush_or_die() before trying to read the next line of
input. Flushing the output after each merge does not seem to affect the
performance, any difference is lost in the noise even after increasing
the number of runs.

$ git rev-list --merges --parents -n100 origin/master |
	sed 's/^[^ ]* //' >/tmp/merges
$ hyperfine -L flush 0,1 --warmup 1 --runs 30 \
	'GIT_FLUSH={flush} ./git merge-tree --stdin </tmp/merges'
Benchmark 1: GIT_FLUSH=0 ./git merge-tree --stdin </tmp/merges
  Time (mean ± σ):     546.6 ms ±  11.7 ms    [User: 503.2 ms, System: 40.9 ms]
  Range (min … max):   535.9 ms … 567.7 ms    30 runs

Benchmark 2: GIT_FLUSH=1 ./git merge-tree --stdin </tmp/merges
  Time (mean ± σ):     546.9 ms ±  12.0 ms    [User: 505.9 ms, System: 38.9 ms]
  Range (min … max):   529.8 ms … 570.0 ms    30 runs

Summary
  'GIT_FLUSH=0 ./git merge-tree --stdin </tmp/merges' ran
    1.00 ± 0.03 times faster than 'GIT_FLUSH=1 ./git merge-tree --stdin </tmp/merges'

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/merge-tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 9a6c8b4e4cf..57f4340faba 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -18,6 +18,7 @@
 #include "tree.h"
 #include "config.h"
 #include "strvec.h"
+#include "write-or-die.h"
 
 static int line_termination = '\n';
 
@@ -623,6 +624,7 @@ int cmd_merge_tree(int argc,
 			} else {
 				die(_("malformed input line: '%s'."), buf.buf);
 			}
+			maybe_flush_or_die(stdout, "stdout");
 
 			if (result < 0)
 				die(_("merging cannot continue; got unclean result of %d"), result);
-- 
gitgitgadget


  reply	other threads:[~2025-02-18 16:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-16 16:37 [PATCH 0/5] merge-tree --stdin: flush stdout Phillip Wood via GitGitGadget
2025-02-16 16:37 ` [PATCH 1/5] merge-tree --stdin: flush stdout to avoid deadlock Phillip Wood via GitGitGadget
2025-02-17 20:01   ` Elijah Newren
2025-02-16 16:37 ` [PATCH 2/5] merge-tree: remove redundant code Phillip Wood via GitGitGadget
2025-02-17 20:15   ` Elijah Newren
2025-02-18 10:01     ` Phillip Wood
2025-02-16 16:37 ` [PATCH 3/5] merge-tree: only use basic merge config Phillip Wood via GitGitGadget
2025-02-16 16:37 ` [PATCH 4/5] merge-tree: improve docs for --stdin Phillip Wood via GitGitGadget
2025-02-17 20:26   ` Elijah Newren
2025-02-18 10:02     ` Phillip Wood
2025-02-18 15:56       ` Elijah Newren
2025-02-16 16:37 ` [PATCH 5/5] merge-tree: fix link formatting in html docs Phillip Wood via GitGitGadget
2025-02-17 20:30   ` Elijah Newren
2025-02-18 16:24 ` [PATCH v2 0/5] merge-tree --stdin: flush stdout Phillip Wood via GitGitGadget
2025-02-18 16:24   ` Phillip Wood via GitGitGadget [this message]
2025-02-19  6:23     ` [PATCH v2 1/5] merge-tree --stdin: flush stdout to avoid deadlock Patrick Steinhardt
2025-02-19 14:55       ` Phillip Wood
2025-02-19 16:02       ` Junio C Hamano
2025-02-18 16:24   ` [PATCH v2 2/5] merge-tree: remove redundant code Phillip Wood via GitGitGadget
2025-02-18 16:24   ` [PATCH v2 3/5] merge-tree: only use basic merge config Phillip Wood via GitGitGadget
2025-02-18 16:24   ` [PATCH v2 4/5] merge-tree: improve docs for --stdin Phillip Wood via GitGitGadget
2025-02-18 16:24   ` [PATCH v2 5/5] merge-tree: fix link formatting in html docs Phillip Wood via GitGitGadget
2025-02-18 16:46   ` [PATCH v2 0/5] merge-tree --stdin: flush stdout Elijah Newren
2025-02-18 16:54     ` Phillip Wood
2025-02-18 19:35     ` 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=3b3179785098580f3336bb24bdbaf0aa1366bfcd.1739895879.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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).