All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Stefan Beller <sbeller@google.com>
Cc: Jens Lehmann <Jens.Lehmann@web.de>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	"git\@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH] submodule: implement `module_name` as a builtin helper
Date: Fri, 07 Aug 2015 14:32:46 -0700	[thread overview]
Message-ID: <xmqq614qyebl.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAGZ79kYjaXtGurWgPk47FauLhC=k-gBjLYhepuz4gJE6Rm_8DA@mail.gmail.com> (Stefan Beller's message of "Fri, 7 Aug 2015 14:21:52 -0700")

Stefan Beller <sbeller@google.com> writes:

> On Fri, Aug 7, 2015 at 2:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>> ...
>>> We can drop that hunk as it only uses the new method
>>> `submodule_name_for_path` but doesn't change functionality.
>>> So if you want to keep Heikos work, I'll just resend the patch
>>> without that hunk.
>>
>> Does such a result even make sense?  Note that I wasn't talking
>> about textual conflict.
>>
>> If we followed what you just said, that patch will try to directly
>> read the data in config_name_for_path string list, which is removed
>> by Heiko's series, if I am reading it right.

By the way, the above is more important part of the message you are
responding to.  The result does not simply link, because your
unsorted_string_list_lookup() will no longer have the string list in
the first place X-<.

>>> 2) Come up with a good thread pool abstraction
>>>    (Started as "[RFC/PATCH 0/4] parallel fetch for submodules" )
>>>    This abstraction (if done right) will allow us to use it in different places
>>>    easily. I started it as part of "git fetch --recurse-submodules" because
>>>    it is submodule related and reasonably sized
>>
>> I personally think this gives the most bang-for-buck.  Write that
>> and expose it as "git submodule for-each-parallel", which takes the
>> shell scriptlet that currently is the loop body of "while read mode
>> sha1 stage sm_path" in update and clone.  You will have immediate
>> and large payback.
>
> You said that before. I feel like this is a bit to narrow.

That depends on how good a design you make for the internal
"parallel execution" engine.  If it is made to take an arbitrary C
function with list of arguments to call it with, for-each-parallel
would be just a degenerate and narrow case where that arbitrary C
function happens to be exec'ing a shell and feed a shell scriptlet
to it.

The internal parallel execution engine would be reusable without any
change to call C native functions, allowing you to do everything
inside the process in the future.  And the "narrow" case is a good
first step to validate your design actually _works_.

  reply	other threads:[~2015-08-07 21:32 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-05  0:04 [PATCH 1/4] submodule: implement `module_list` as a builtin helper Stefan Beller
2015-08-05  0:04 ` [PATCH 2/4] submodule: implement `module_name` " Stefan Beller
2015-08-05  0:05   ` Stefan Beller
2015-08-05  0:58   ` Eric Sunshine
2015-08-05 16:29     ` Stefan Beller
2015-08-05 19:06   ` Jens Lehmann
2015-08-05 19:55     ` Stefan Beller
2015-08-05 21:08       ` [PATCH] " Stefan Beller
2015-08-06 19:49         ` Jens Lehmann
2015-08-06 21:38           ` Stefan Beller
2015-08-07 20:03             ` Junio C Hamano
2015-08-07 20:54               ` Stefan Beller
2015-08-07 20:17           ` Junio C Hamano
2015-08-07 20:49             ` Stefan Beller
2015-08-07 21:14               ` Junio C Hamano
2015-08-07 21:21                 ` Stefan Beller
2015-08-07 21:32                   ` Junio C Hamano [this message]
2015-08-07 22:04                     ` Stefan Beller
2015-08-07 22:18                       ` Junio C Hamano
2015-08-07 23:12                         ` Stefan Beller
2015-08-07 22:42                   ` Junio C Hamano
2015-08-07 23:19                     ` Stefan Beller
2015-08-08  0:21                       ` Junio C Hamano
2015-08-08  6:20                         ` Junio C Hamano
2015-08-10 17:03                           ` Stefan Beller
2015-08-05 18:31 ` [PATCH 1/4] submodule: implement `module_list` " Jens Lehmann
2015-08-07 19:53 ` 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=xmqq614qyebl.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    --cc=sbeller@google.com \
    /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.