Re: [PATCH v6 00/10] The final building block for a faster rebase -i

From: Stefan Beller <sbeller@google.com>
Date: Jul 14, 2017, 17:27 UTC
Message-ID: <CAGZ79kbGqSadRDSQbg3N6pj04igFQG1x15S_vDUD4A6sXRSzXA@mail.gmail.com>
In-Reply-To: <cover.1500043436.git.johannes.schindelin@gmx.de>

On Fri, Jul 14, 2017 at 7:44 AM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 8 quoted lines

> This patch series reimplements the expensive pre- and post-processing of
> the todo script in C.
>
> And it concludes the work I did to accelerate rebase -i so far.
>
> I am still unwilling to replace a compile-time safe way to pass the
> options to the revision machinery by the alternative (which I am still
> flabbergasted about) proposed by Junio. This will not change.

There is new input for this discussion, though. :)

https://public-inbox.org/git/20170706202739.6056-1-sbeller@google.com/ was sent out to gauge interest if we want to pull through with removing all the submodule hack, such as 'add_submodule_odb' in submodule.c which just adds the submodule objects as an alternate object store, such that we can do some things in-process (check if a submodule has certain commits, merge_submodule).

For one of these ('merge_submodule') I would have to add the 'struct repository' as another parameter to pass around to the diff and revision walking machinery. But the API for these subsystems is traditionally only operated via an array of strings instead of passing assigning a member field to a value.

So If I were to follow the "use arrays of strings only to operate the
revision machinery" I would:
* pass a string indicating which repo to use
  (probably the path to git dir?)
* the repository objects are cached, so we can lookup e.g.
  "the_repository" via the correct string.
* use that repo object inside the revision machinery.

That sounds cumbersome and I would prefer to pass in the repository object directory. So maybe we want to have some other way opposed to "an array of strings" to operate the revision machinery.

>  -static int subject2item_cmp(const struct subject2item_entry *a,
>  -      const struct subject2item_entry *b, const void *key)
>  +static int subject2item_cmp(const void *fndata,

This could also be named unused_fndata. Please see origin/sb/hashmap-cleanup, if that makes sense as well (have all arguments const void and cast them inside the function, such that we can avoid the cast to hashmap_cmp_fn in hashmap_init)

Thanks, Stefan