Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Christian Couder <christian.couder@gmail.com>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Patrick Steinhardt <ps@pks.im>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>, John Cai <johncai86@gmail.com>,
	Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH 00/14] Introduce new `git replay` command
Date: Fri, 14 Apr 2023 11:39:04 -0600	[thread overview]
Message-ID: <64398fb87548d_5d9f72942d@chronos.notmuch> (raw)
In-Reply-To: <20230407072415.1360068-1-christian.couder@gmail.com>

Christian Couder wrote:
> # Quick Overview (from Elijah)
> 
> `git replay`, at a basic level, can perhaps be thought of as a
> "default-to-dry-run rebase" -- meaning no updates to the working tree,
> or to the index, or to any references.

Interesting, I just ran into this problem trying to cleanup my personal git
branches.

Simply checking which branches can be cleanly rebased on top of master takes a
significant amount of time without any tricks, and using `git merge-tree` still
takes some time.

But the biggest offender is checking which patches have not yet been merged
into master, which takes 52 seconds on my machine which is by no means old.

> # Reasons for diverging from cherry-pick & rebase (from Elijah)
> 
> * Server side needs

I personally don't care about the server side, but...

>   * Both cherry-pick and rebase, via the sequencer, are heavily tied
>     to updating the working tree, index, some refs, and a lot of
>     control files with every commit replayed, and invoke a mess of
>     hooks[1] that might be hard to avoid for backward compatibility
>     reasons (at least, that's been brought up a few times on the
>     list).

This is important as an end user as well.

Since day 1 one of the important selling points of git was that operations that
could be done in milliseconds did take milliseconds.

If it can be done faster, why wouldn't I want it to be done faster?

> * Decapitate HEAD-centric assumptions

That's good, but not particularly important at the moment IMO.

> * Performance
> 
>   * jj is slaughtering us on rebase speed[2].  I would like us to become
>     competitive.  (I dropped a few comments in the link at [2] about why
>     git is currently so bad.)

Indeed.

>   * From [3], there was a simple 4-patch series in linux.git that took
>     53 seconds to rebase.

I did participate in that discussion, but Uwe Kleine-König never responded back.

In [1] he clearly noticed the problem was *before* attempting to apply any
patch. Other people mentioned the fork-point detection, but I don't think that
was the issue, my guess was that checking for the possibility of a fast-forward
was the issue.

The code was clearly doing the wrong thing for that case, but I believe it
should have been fixed by d42c9ffa0f (rebase: factor out branch_base
calculation, 2022-10-17).

It would be interesting to see if this issue can be reproduced somehow.

>     Switching to ort dropped it to 16 seconds.

No, it dropped to 16 seconds it for Elijah, not Uwe. Uwe (who had the real
repository) noticed a big reduction of around 70%, but the discrepancy of using
--onto versus not always remained.

>     While that sounds great, only 11 *milliseconds* were needed to do
>     the actual merges.  That means almost *all* the time (>99%) was
>     overhead!  Big offenders:
> 
>     * --reapply-cherry-picks should be the default
> 
>     * can_fast_forward() should be ripped out, and perhaps other extraneous
>       revision walks

Doesn't d42c9ffa0f (rebase: factor out branch_base calculation, 2022-10-17)
deal with that?

---

I think something like this is defeinitely needed, when I rewrote `git rebase`
to use `git cherry-pick` I noticed many areas of improvement, and I'm of the
opinion that `git rebase` should be rewritten from scratch.

But precisely because git focuses too much on backwards compatibility (and
often in the wrong areas), I think `git replay` should be thoroughly discussed
before accepting something we could quickly realize can be substantially
improved.

Cheers.

[1] https://lore.kernel.org/git/20210528214024.vw4huojcklrm6d27@pengutronix.de/

-- 
Felipe Contreras

  parent reply	other threads:[~2023-04-14 17:39 UTC|newest]

Thread overview: 208+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-07  7:24 [PATCH 00/14] Introduce new `git replay` command Christian Couder
2023-04-07  7:24 ` [PATCH 01/14] replay: introduce new builtin Christian Couder
2023-04-07  7:24 ` [PATCH 02/14] replay: start using parse_options API Christian Couder
2023-04-07  7:24 ` [PATCH 03/14] replay: die() instead of failing assert() Christian Couder
2023-04-07  7:24 ` [PATCH 04/14] replay: introduce pick_regular_commit() Christian Couder
2023-04-07  7:24 ` [PATCH 05/14] replay: don't simplify history Christian Couder
2023-04-07  7:24 ` [PATCH 06/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-04-07  7:24 ` [PATCH 07/14] replay: remove progress and info output Christian Couder
2023-04-07  7:24 ` [PATCH 08/14] replay: remove HEAD related sanity check Christian Couder
2023-04-07  7:24 ` [PATCH 09/14] replay: very coarse worktree updating Christian Couder
2023-04-07  7:24 ` [PATCH 10/14] replay: make it a minimal server side command Christian Couder
2023-04-07  7:24 ` [PATCH 11/14] replay: use standard revision ranges Christian Couder
2023-04-14 14:09   ` Derrick Stolee
2023-04-14 14:23     ` Derrick Stolee
2023-04-15 19:07       ` Elijah Newren
2023-04-16  5:28         ` Elijah Newren
2023-04-17 14:05         ` Derrick Stolee
2023-04-18  5:54           ` Elijah Newren
2023-04-18 13:10             ` Derrick Stolee
2023-04-20  4:53               ` Elijah Newren
2023-04-20 13:44                 ` Derrick Stolee
2023-04-23  1:18                   ` Elijah Newren
2023-04-24 15:23                     ` Derrick Stolee
2023-04-30  6:45                       ` Elijah Newren
2023-09-03 15:47                         ` Johannes Schindelin
2023-09-07  8:39                           ` Christian Couder
2023-09-07 10:22                             ` Johannes Schindelin
2023-04-17 15:45         ` Junio C Hamano
2023-04-18  5:58           ` Elijah Newren
2023-04-18  4:58       ` Elijah Newren
2023-04-15 18:30     ` Elijah Newren
2023-04-07  7:24 ` [PATCH 12/14] replay: introduce guess_new_base() Christian Couder
2023-04-07  7:24 ` [PATCH 13/14] replay: add different modes Christian Couder
2023-04-07  7:24 ` [PATCH 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-04-14 10:12 ` [PATCH 00/14] Introduce new `git replay` command Phillip Wood
2023-04-15 17:18   ` Elijah Newren
2023-04-14 17:39 ` Felipe Contreras [this message]
2023-04-15  6:44 ` Elijah Newren
2023-05-09 17:53 ` [PATCH v2 00/15] " Christian Couder
2023-05-09 17:53   ` [PATCH v2 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-05-09 17:53   ` [PATCH v2 02/15] replay: introduce new builtin Christian Couder
2023-05-09 17:53   ` [PATCH v2 03/15] replay: start using parse_options API Christian Couder
2023-05-09 17:53   ` [PATCH v2 04/15] replay: die() instead of failing assert() Christian Couder
2023-05-09 17:53   ` [PATCH v2 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-05-09 17:53   ` [PATCH v2 06/15] replay: don't simplify history Christian Couder
2023-05-09 17:53   ` [PATCH v2 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-05-09 17:53   ` [PATCH v2 08/15] replay: remove progress and info output Christian Couder
2023-05-09 17:53   ` [PATCH v2 09/15] replay: remove HEAD related sanity check Christian Couder
2023-05-09 17:53   ` [PATCH v2 10/15] replay: make it a minimal server side command Christian Couder
2023-05-09 17:53   ` [PATCH v2 11/15] replay: use standard revision ranges Christian Couder
2023-05-09 17:53   ` [PATCH v2 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-05-16  4:25     ` Elijah Newren
2023-05-09 17:53   ` [PATCH v2 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-05-09 17:53   ` [PATCH v2 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-05-16  4:26     ` Elijah Newren
2023-05-09 17:53   ` [PATCH v2 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-05-16  4:26     ` Elijah Newren
2023-05-09 22:28   ` [PATCH v2 00/15] Introduce new `git replay` command Junio C Hamano
2023-05-10  7:33     ` Christian Couder
2023-05-16  4:42   ` Elijah Newren
2023-06-02 10:25   ` [PATCH v3 " Christian Couder
2023-06-02 10:25     ` [PATCH v3 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-06-02 10:25     ` [PATCH v3 02/15] replay: introduce new builtin Christian Couder
2023-06-02 10:25     ` [PATCH v3 03/15] replay: start using parse_options API Christian Couder
2023-06-02 10:25     ` [PATCH v3 04/15] replay: die() instead of failing assert() Christian Couder
2023-06-02 10:25     ` [PATCH v3 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-06-02 10:25     ` [PATCH v3 06/15] replay: don't simplify history Christian Couder
2023-06-02 10:25     ` [PATCH v3 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-06-02 10:25     ` [PATCH v3 08/15] replay: remove progress and info output Christian Couder
2023-06-02 10:25     ` [PATCH v3 09/15] replay: remove HEAD related sanity check Christian Couder
2023-06-02 10:25     ` [PATCH v3 10/15] replay: make it a minimal server side command Christian Couder
2023-06-22 10:01       ` Toon Claes
2023-09-07  8:32         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 11/15] replay: use standard revision ranges Christian Couder
2023-06-22 10:03       ` Toon Claes
2023-09-07  8:32         ` Christian Couder
2023-09-07 21:02           ` Dragan Simic
2023-10-10 12:44             ` Christian Couder
2023-10-10 14:02               ` Dragan Simic
2023-06-02 10:25     ` [PATCH v3 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-07-25 21:16       ` Junio C Hamano
2023-09-07  8:33         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-06-22 10:05       ` Toon Claes
2023-09-07  8:35         ` Christian Couder
2023-07-25 21:41       ` Junio C Hamano
2023-09-07  8:35         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-06-22 10:10       ` Toon Claes
2023-09-07  8:37         ` Christian Couder
2023-06-02 10:25     ` [PATCH v3 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-06-03  1:42     ` [PATCH v3 00/15] Introduce new `git replay` command Junio C Hamano
2023-06-05  7:11       ` Christian Couder
2023-09-07  9:25     ` [PATCH v4 " Christian Couder
2023-09-07  9:25       ` [PATCH v4 01/15] t6429: remove switching aspects of fast-rebase Christian Couder
2023-09-07  9:25       ` [PATCH v4 02/15] replay: introduce new builtin Christian Couder
2023-09-07 10:23         ` Johannes Schindelin
2023-10-10 12:42           ` Christian Couder
2023-09-07  9:25       ` [PATCH v4 03/15] replay: start using parse_options API Christian Couder
2023-09-07  9:25       ` [PATCH v4 04/15] replay: die() instead of failing assert() Christian Couder
2023-09-07  9:25       ` [PATCH v4 05/15] replay: introduce pick_regular_commit() Christian Couder
2023-09-07  9:25       ` [PATCH v4 06/15] replay: don't simplify history Christian Couder
2023-09-07 10:23         ` Johannes Schindelin
2023-10-10 12:43           ` Christian Couder
2023-09-07  9:25       ` [PATCH v4 07/15] replay: add an important FIXME comment about gpg signing Christian Couder
2023-09-07  9:25       ` [PATCH v4 08/15] replay: remove progress and info output Christian Couder
2023-09-07  9:25       ` [PATCH v4 09/15] replay: remove HEAD related sanity check Christian Couder
2023-09-07  9:25       ` [PATCH v4 10/15] replay: make it a minimal server side command Christian Couder
2023-09-07  9:25       ` [PATCH v4 11/15] replay: use standard revision ranges Christian Couder
2023-09-07 10:24         ` Johannes Schindelin
2023-10-10 12:49           ` Christian Couder
2023-09-08 22:55         ` Linus Arver
2023-09-10  3:20           ` Linus Arver
2023-10-10 12:48             ` Christian Couder
2023-10-10 12:48           ` Christian Couder
2023-10-19 19:26             ` Linus Arver
2023-09-07  9:25       ` [PATCH v4 12/15] replay: disallow revision specific options and pathspecs Christian Couder
2023-09-07 10:24         ` Johannes Schindelin
2023-10-10 12:49           ` Christian Couder
2023-09-07  9:25       ` [PATCH v4 13/15] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-09-07  9:25       ` [PATCH v4 14/15] replay: add --contained to rebase contained branches Christian Couder
2023-09-07  9:25       ` [PATCH v4 15/15] replay: stop assuming replayed branches do not diverge Christian Couder
2023-09-07 10:25       ` [PATCH v4 00/15] Introduce new `git replay` command Johannes Schindelin
2023-10-10 12:50         ` Christian Couder
2023-10-10 12:38       ` [PATCH v5 00/14] " Christian Couder
2023-10-10 12:38         ` [PATCH v5 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-10-10 12:38         ` [PATCH v5 02/14] replay: introduce new builtin Christian Couder
2023-10-10 12:38         ` [PATCH v5 03/14] replay: start using parse_options API Christian Couder
2023-10-10 12:38         ` [PATCH v5 04/14] replay: die() instead of failing assert() Christian Couder
2023-10-10 12:38         ` [PATCH v5 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-10-10 12:38         ` [PATCH v5 06/14] replay: change rev walking options Christian Couder
2023-10-10 12:38         ` [PATCH v5 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-10-10 12:38         ` [PATCH v5 08/14] replay: remove progress and info output Christian Couder
2023-10-10 12:38         ` [PATCH v5 09/14] replay: remove HEAD related sanity check Christian Couder
2023-10-10 12:38         ` [PATCH v5 10/14] replay: make it a minimal server side command Christian Couder
2023-10-10 12:38         ` [PATCH v5 11/14] replay: use standard revision ranges Christian Couder
2023-10-19 19:49           ` Linus Arver
2023-10-10 12:38         ` [PATCH v5 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-10-10 12:38         ` [PATCH v5 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-10-10 12:38         ` [PATCH v5 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-10-26 13:44         ` [PATCH v5 00/14] Introduce new `git replay` command Johannes Schindelin
2023-10-29  6:01           ` Elijah Newren
2023-11-02 14:59             ` Christian Couder
2023-11-08 12:25               ` Johannes Schindelin
2023-11-02 15:06           ` Christian Couder
2023-11-08 12:25             ` Johannes Schindelin
2023-10-29  6:00         ` Elijah Newren
2023-10-29 14:14           ` Johannes Schindelin
2023-10-30 17:18             ` Elijah Newren
2023-11-02 14:44               ` Christian Couder
2023-11-02 14:48           ` Christian Couder
2023-11-02 13:51         ` [PATCH v6 " Christian Couder
2023-11-02 13:51           ` [PATCH v6 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-02 13:51           ` [PATCH v6 02/14] replay: introduce new builtin Christian Couder
2023-11-02 13:51           ` [PATCH v6 03/14] replay: start using parse_options API Christian Couder
2023-11-02 13:51           ` [PATCH v6 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-02 13:51           ` [PATCH v6 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-02 13:51           ` [PATCH v6 06/14] replay: change rev walking options Christian Couder
2023-11-02 13:51           ` [PATCH v6 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-02 13:51           ` [PATCH v6 08/14] replay: remove progress and info output Christian Couder
2023-11-02 13:51           ` [PATCH v6 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-02 13:51           ` [PATCH v6 10/14] replay: make it a minimal server side command Christian Couder
2023-11-02 13:51           ` [PATCH v6 11/14] replay: use standard revision ranges Christian Couder
2023-11-02 13:51           ` [PATCH v6 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-02 13:51           ` [PATCH v6 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-02 13:51           ` [PATCH v6 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-07  2:43           ` [PATCH v6 00/14] Introduce new `git replay` command Elijah Newren
2023-11-07  9:43             ` Christian Couder
2023-11-15 14:51               ` Christian Couder
2023-11-08 12:19             ` Johannes Schindelin
2023-11-08 12:47           ` Johannes Schindelin
2023-11-15 14:46             ` Christian Couder
2023-11-16  8:45               ` Johannes Schindelin
2023-11-16  8:52                 ` Christian Couder
2023-11-15 14:33           ` [PATCH v7 " Christian Couder
2023-11-15 14:33             ` [PATCH v7 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-15 14:33             ` [PATCH v7 02/14] replay: introduce new builtin Christian Couder
2023-11-15 14:33             ` [PATCH v7 03/14] replay: start using parse_options API Christian Couder
2023-11-15 14:33             ` [PATCH v7 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-15 14:33             ` [PATCH v7 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-15 14:33             ` [PATCH v7 06/14] replay: change rev walking options Christian Couder
2023-11-15 14:33             ` [PATCH v7 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-15 14:33             ` [PATCH v7 08/14] replay: remove progress and info output Christian Couder
2023-11-15 14:33             ` [PATCH v7 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-15 14:33             ` [PATCH v7 10/14] replay: make it a minimal server side command Christian Couder
2023-11-15 14:33             ` [PATCH v7 11/14] replay: use standard revision ranges Christian Couder
2023-11-15 14:33             ` [PATCH v7 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-15 14:33             ` [PATCH v7 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-15 14:33             ` [PATCH v7 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-16  8:53             ` [PATCH v7 00/14] Introduce new `git replay` command Johannes Schindelin
2023-11-23 19:32               ` Elijah Newren
2023-11-24  0:28                 ` Junio C Hamano
2023-11-24 11:10             ` [PATCH v8 " Christian Couder
2023-11-24 11:10               ` [PATCH v8 01/14] t6429: remove switching aspects of fast-rebase Christian Couder
2023-11-24 11:10               ` [PATCH v8 02/14] replay: introduce new builtin Christian Couder
2023-11-24 11:10               ` [PATCH v8 03/14] replay: start using parse_options API Christian Couder
2023-11-24 11:10               ` [PATCH v8 04/14] replay: die() instead of failing assert() Christian Couder
2023-11-24 11:10               ` [PATCH v8 05/14] replay: introduce pick_regular_commit() Christian Couder
2023-11-24 11:10               ` [PATCH v8 06/14] replay: change rev walking options Christian Couder
2023-11-24 11:10               ` [PATCH v8 07/14] replay: add an important FIXME comment about gpg signing Christian Couder
2023-11-24 11:10               ` [PATCH v8 08/14] replay: remove progress and info output Christian Couder
2023-11-24 11:10               ` [PATCH v8 09/14] replay: remove HEAD related sanity check Christian Couder
2023-11-24 11:10               ` [PATCH v8 10/14] replay: make it a minimal server side command Christian Couder
2023-11-24 11:10               ` [PATCH v8 11/14] replay: use standard revision ranges Christian Couder
2023-11-24 11:10               ` [PATCH v8 12/14] replay: add --advance or 'cherry-pick' mode Christian Couder
2023-11-24 11:10               ` [PATCH v8 13/14] replay: add --contained to rebase contained branches Christian Couder
2023-11-24 11:10               ` [PATCH v8 14/14] replay: stop assuming replayed branches do not diverge Christian Couder
2023-11-25  0:02               ` [PATCH v8 00/14] Introduce new `git replay` command Johannes Schindelin

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=64398fb87548d_5d9f72942d@chronos.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    --cc=newren@gmail.com \
    --cc=ps@pks.im \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).