Re: [PATCH v4 5/5] diff-lib: parallelize run_diff_files for submodules

From: Glen Choo <chooglen@google.com>
Date: Nov 29, 2022, 22:29 UTC
Message-ID: <kl6lr0xlqtrc.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <20221128210125.2751300-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 11 quoted lines

> Calvin Wan <calvinwan@google.com> writes:
>>  submodule.c                        | 154 +++++++++++++++++++++++++++++
>
> I think the way to implement this is to have a parallel implementation,
> and then have the serial implementation call the parallel implementation's
> functions, or have a common set of functions that both the parallel
> implementation and the serial implementation call. Here, it seems that
> the parallel implementation exists completely separate from the serial
> implementation, with no code shared. That makes it both more difficult to
> review, and also makes it difficult to make changes to how we diff submodules
> in the future (since we would have to make changes in two parts of the code).

It seems that most of the code is copied from is_submodule_modified(), so a possible way to do this would be:

- Split is_submodule_modified() into 2 functions, one that sets up the
  "git status --porcelain=2" process (named something like
  setup_status_porcelain()) and one that parses its output (this is
  parse_status_porcelain() in Patch 2). The serial implementation
  (is_submodule_modified()) uses both of these and has some extra logic
  to run the child process.

- Refactor get_next_submodule_status() (from this patch) to use
  setup_status_porcelain().