All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jonas Bernoulli <jonas@bernoul.li>
Cc: git@vger.kernel.org
Subject: Re: "submodule foreach" much slower than removed "submodule--helper --list"
Date: Mon, 17 Oct 2022 18:50:06 +0200	[thread overview]
Message-ID: <221017.86h702jsiq.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <87a65xrnwz.fsf@bernoul.li>


Replying to both this & the parent post:

On Sat, Oct 15 2022, Jonas Bernoulli wrote:

> In v2.38.0 (31955475d1c283120d5d84247eb3fd55d9f5fdd9)
> "submodule--helper --list" was remove because
>
>> We're not getting anything useful from the "list | cut -f2"
>> invocation that we couldn't get from "foreach 'echo $sm_path'".
>
> But we get speed (this is with about one hundred modules):
>
> $ time git submodule foreach -q 'echo $sm_path' > /dev/null
>
> real    0m0.585s
> user    0m0.413s
> sys     0m0.182s
>
> $ time git submodule--helper list > /dev/null
>
> real    0m0.008s
> user    0m0.004s
> sys     0m0.004s
>
> Please consider restoring this subcommand or providing something
> equivalent that is just as fast.

Sorry about the slowdown, the removal of "list" was just an in-between
step to migrating "submodule" to a full built-in.

I can't reproduce anything like the 8ms v.s. ~600ms difference you note
here, but for me migrating it to a built-in is around 10% slower with
"foreach" than the old "list". I wonder what results you get?

I sent in a topic to migrate it since you sent this report. I was going
to do it in this development cycle, but this prompted me to do it
earlier:

	https://lore.kernel.org/git/cover-00.10-00000000000-20221017T115544Z-avarab@gmail.com/

On Sat, Oct 15 2022, Jonas Bernoulli wrote:

> I just noticed that "submodule--helper name" was also removed, which I
> also found useful in scripts.  Please tell me if I am missing something,
> but it seems I now have to do something like this instead:
>
>   git config -f .gitmodules --list |
>       sed -n "s|^submodule.\([^.]*\).path=$path\$|\1|p"
>
> The old way was nicer:
>
>   git submodule--helper name $path
>
> I realize submodule--helper is for internal use and using it anyway
> comes with the risk of such removals and other changes, but again,
> please consider restoring that or providing something similar in the
> public interface.

This however is another case, I removed "name" along with "list" and
other leftover code we weren't using anymore for the internal-only
"submodule--helper" (which is at turns out, was not as internal-only as
we'd hoped).

For "list" it's clear how to use "foreach" instead, but for "name" then
AFAICT the "best" replacement is to do a:

	git submodule foreach 'echo $displaypath $name'

And pipe that into grep/sed. If that's fast enough would it satisfy your
use-case, or would a "name" equivalent be handy?

I think the best way to prove that would be e.g.:

	git submodule foreach-format '%{name}' -- <pathspec>

Which, due to the "foreach" taking N number of arguments isn't easy to
add to "foreach" without the interface becoming somewhat tortured (we
could add a [---pathspec=<pathspec>]...), but "-- <pathspec>" with a
different subcommand name seems better.

  parent reply	other threads:[~2022-10-17 16:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-15 16:50 "submodule foreach" much slower than removed "submodule--helper --list" Jonas Bernoulli
2022-10-15 17:34 ` Jonas Bernoulli
2022-10-15 18:13   ` Jeff King
2022-10-15 18:16     ` Junio C Hamano
2022-10-15 18:23       ` Jeff King
2022-10-15 18:37         ` Junio C Hamano
2022-10-15 22:40     ` Junio C Hamano
2022-10-17 17:02       ` Jeff King
2022-11-07 11:01       ` Matti Möll
2022-11-08  2:50         ` Taylor Blau
2022-11-08  3:37           ` Ævar Arnfjörð Bjarmason
2022-10-17 16:50   ` Ævar Arnfjörð Bjarmason [this message]
2022-10-17 17:46     ` Jeff King
2022-10-21 14:58     ` Jonas Bernoulli

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=221017.86h702jsiq.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jonas@bernoul.li \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.