Subject: Re: [PATCH] submodule: implement `module_name` as a builtin helper

From: Junio C Hamano <[email-masked]>
Date: Fri, 07 Aug 2015 14:32:46 -0700

Stefan Beller <[email-masked]> writes:

> On Fri, Aug 7, 2015 at 2:14 PM, Junio C Hamano <[email-masked]> wrote:
>> Stefan Beller <[email-masked]> 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_.