Re: [PATCH v4 00/22] Add configuration options for split-index

[Junio C Hamano]

Christian Couder  writes:

> Highlevel view of the patches in the series
> ~~~
>
> Except for patch 1/22 and 1/22, there are 3 big steps, one for each
> new configuration variable introduced.
>
> There only small differences between this patch series and the v3
> patch series sent a few months ago.
>
> - Patch 1/22 marks a message for translation. It is not new and
>   can be applied separately.
>
> - Patch 2/22 improves the existing indentation style of t1700 by
>   using different here document style. It is a new preparatory
>   patch suggested by Junio.

OK.  I read interdiff against the previous round carefully, and
skimmed all patches less carefully.  

I may have comments on individual patches later, but this round
looked mostly sensible.

Thanks for following it through.

[PATCH v4 00/22] Add configuration options for split-index

[Christian Couder]

Goal


We want to make it possible to use the split-index feature
automatically by just setting a new "core.splitIndex" configuration
variable to true.

This can be valuable as split-index can help significantly speed up
`git rebase` especially along with the work to libify `git apply`
that has been merged to master
(see https://github.com/git/git/commit/81358dc238372793b1590efa149cc1581d1fbd98)
and is now in v2.11.

Design
~~

The design is similar as the previous work that introduced
"core.untrackedCache". 

The new "core.splitIndex" configuration option can be either true,
false or undefined which is the default.

When it is true, the split index is created, if it does not already
exists, when the index is read. When it is false, the split index is
removed if it exists, when the index is read. Otherwise it is left as
is.

Along with this new configuration variable, the two following options
are also introduced:

- splitIndex.maxPercentChange

This is to avoid having too many changes accumulating in the split
index while in split index mode. The git-update-index
documentation says:

If split-index mode is already enabled and `--split-index` is
given again, all changes in $GIT_DIR/index are pushed back to
the shared index file.

but it is probably better to not expect the user to think about it
and to have a mechanism that pushes back all changes to the shared
index file automatically when some threshold is reached.

The default threshold is when the number of entries in the split
index file reaches 20% of the number of entries in the shared
index file. The new "splitIndex.maxPercentChange" config option
lets people tweak this value.

- splitIndex.sharedIndexExpire

To make sure that old sharedindex files are eventually removed
when a new one has been created, we "touch" the shared index file
every time a split index file using the shared index file is
either created or read from. Then we can delete shared indexes
with an mtime older than one week (by default), when we create a
new shared index file. The new "splitIndex.sharedIndexExpire"
config option lets people tweak this grace period.

This idea was suggested by Duy in:


https://public-inbox.org/git/cacsjy8bqmfashf5kjguh+bd7xg98cafnyde964vjypxz-em...@mail.gmail.com/

and after some experiments, I agree that it is much simpler than
what I thought could be done during our discussion.

Junio also thinks that we have to do "time-based GC" in:
 
https://public-inbox.org/git/xmqqeg33ccjj@gitster.mtv.corp.google.com/

Note that this patch series doesn't address a leak when removing a
split-index, but Duy wrote that he has a patch to fix this leak:

https://public-inbox.org/git/CACsJy8AisF2ZVs7JdnVFp5wdskkbVQQQ=dbq5uze1moscfb...@mail.gmail.com/

Highlevel view of the patches in the series
~~~

Except for patch 1/22 and 1/22, there are 3 big steps, one for each
new configuration variable introduced.

There only small differences between this patch series and the v3
patch series sent a few months ago.

- Patch 1/22 marks a message for translation. It is not new and
  can be applied separately.

- Patch 2/22 improves the existing indentation style of t1700 by
  using different here document style. It is a new preparatory
  patch suggested by Junio.

Step 1 is:

- Patches 3/22 to 6/22 introduce the functions that are reading
  the "core.splitIndex" configuration variable and tweaking the
  split index depending on its value.

- Patch 7/22 adds a few tests for the new feature.

- Patches 8/22 and 9/22 add some documentation for the new
  feature.

There is no change since v3 in this step.

Step 2 is:

- Patches 10/22 and 11/22 introduce the functions that are reading
  the "splitIndex.maxPercentChange" configuration variable and
  regenerating a new shared index file depending on its value.

  Patch 11/12 has a few changes suggested by Junio since v3, see
  
https://public-inbox.org/git/CAP8UFD3_1EN=0esd12cew1muw8yhtpazw0m_g3wmvkfk-ug...@mail.gmail.com/

- Patch 12/22 adds a few tests for the new feature.

- Patch 13/22 add some documentation for the new feature. The
  added documentation has been reworded a little bit since v3 as
  suggested by Junio.

Step 3 is:

- Patches 14/22 to 17/22 introduce the functions that are reading
  the "splitIndex.sharedIndexExpire" configuration variable and
  expiring old shared index files depending on its value.

  Patch 15/22 has a few changes suggested by Ramsay and Junio in
  
http://public-inbox.org/git/a1a44640-ff6c-2294-72ac-46322eff8...@ramsayjones.plus.com/
  http://public-inbox.org/git/xmqqbmunq6mg@gitster.mtv.corp.google.com/

  The main change is that the new freshen_shared_index() will now
  warn if the split-index file