RFC patch, 8 partsAdd configuration options for split-index

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 current work to libify `git apply`.

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.

Highlevel view of the patches in the series ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    - Patches 1/8 to 4/8 introduce the functions that are reading the
      configuration variable and tweaking the split index depending on
      its value.

    - Patches 5/8 and 6/8 add some documentation for the new feature.

    - Patch 7/8 adds a few simple tests.

    - Patch 8/8 cleans up the old "sharedindex.XXXX" previously
      created by split-index.

Future work ~~~~~~~~~~~

One thing that is probably missing is a mechanism 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 threshold could be for example when $GIT_DIR/index size is larger than 25% of the shared index size. Opinions, test results or test ideas are welcome on this.

Links ~~~~~

The last iteration of the git apply libification patch series is there:

http://thread.gmane.org/gmane.comp.version-control.git/298344/

This patch series is also available here:

https://github.com/chriscool/git/commits/config-split-index

Christian Couder (8):
  config: add git_config_get_split_index()
  split-index: add {add,remove}_split_index() functions
  read-cache: add and then use tweak_split_index()
  update-index: warn in case of split-index incoherency
  Documentation/config: add information for core.splitIndex
  Documentation/git-update-index: talk about core.splitIndex config var
  t1700: add tests for core.splitIndex
  read-cache: unlink old sharedindex files

 Documentation/config.txt           |  4 ++++
 Documentation/git-update-index.txt |  6 ++++++
 builtin/update-index.c             | 20 ++++++++---------
 cache.h                            |  1 +
 config.c                           | 10 +++++++++
 read-cache.c                       | 44 +++++++++++++++++++++++++++++++++++++-
 split-index.c                      | 18 ++++++++++++++++
 split-index.h                      |  2 ++
 t/t1700-split-index.sh             | 44 ++++++++++++++++++++++++++++++++++++++
 9 files changed, 138 insertions(+), 11 deletions(-)

Also use the functions in cmd_update_index() in builtin/update-index.c.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/update-index.c | 17 ++++-------------
 split-index.c          | 18 ++++++++++++++++++
 split-index.h          |  2 ++
 3 files changed, 24 insertions(+), 13 deletions(-)

This will make us use the split-index feature or not depending on the value of the core.splitIndex config variable.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 read-cache.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/config.txt | 4 ++++
 1 file changed, 4 insertions(+)

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-update-index.txt | 6 ++++++
 1 file changed, 6 insertions(+)

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t1700-split-index.sh | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/update-index.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Everytime split index is turned on, it creates a "sharedindex.XXXX" file in the git directory. This makes sure that old sharedindex files are removed after a new one has been created.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 read-cache.c           | 27 ++++++++++++++++++++++++++-
 t/t1700-split-index.sh |  7 +++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder <christian.couder@gmail.com> wrote:

Hmm it's one-way link, we don't know how many index files use this shared index file, how can you be sure nobody else will need it? I'm thinking about temporary indexes. If a temp index is created, saved on disk, and use delete the shared index file, the real, main index may become useless. Temp index will most likely replace the main index (git commit) but if a failure happens, we can't fall back.

A safer approach is "touch" the shared index every time a linked index is used, then we can delete shared indexes with old mtime, older than a grace period, in git-prune (or here).

On Mon, Jul 11, 2016 at 8:27 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Isn't there a way to scan all the current indexes (temp or not) to see which shared indexes they need?

Maybe old linked indexes could be converted after some time to use a newer shared index, so that we can get rid of the old shared indexes. That seems safer than just deleting old linked indexes.

Thanks for your review, Christian.

On Tue, Jul 12, 2016 at 9:04 AM, Christian Couder <christian.couder@gmail.com> wrote:

No. People could create an index file anywhere in theory. So you don't know how many index files there are.

It really depends. If the shared part is too small for old indexes, we might as well unsplit them. In practice though, the only long-term index file is $GIT_DIR/index. If we don't delete old shared index files too close to their creation time, temp index files will go away.

On Tue, Jul 12, 2016 at 5:12 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Maybe when an index file is created, its path and its sharedindex file could be appended into a log file. We could check this log file to see if we can remove sharedindex files. We would need to remove the entries in this log file for the indexes that are no longer there.

Or instead of one log file we could have a file for each index file in a special directory called for example "indexinfo". So we could just delete the file if its related index is no longer there.

We could treat $GIT_DIR/index specially so that if there are no temp index files, there should be nothing in "indexinfo".

On Tue, Jul 12, 2016 at 9:45 PM, Christian Couder <christian.couder@gmail.com> wrote:

New files will require locking so people don't append at the same time. And maybe another new host of problems. I think we can just go with the garbage collection way that we have done for unreachable objects.

Your indexinfo idea looks very close to multiple locking, an index would lock the shared index it's linked to, preventing it from being removed. For single locking, we can just create a file named $X.lock, but for multiple locks, maybe we can go with $X.lock-$index_trailing_sha1. Will it work? I don't know. Just thinking out loud.

No, temp index files are needed. And it will be hard to treat $GIT_DIR/index specially because updating it involves a temp index: you first prepare a new index in $GIT_DIR/index.lock. If everything goes well, you atomically rename it to $GIT_DIR/index. You may be able to treat $GIT_DIR/index.lock special too, but that's starting to get out of hand.

On Wed, Jul 13, 2016 at 5:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:

If we create a new file for each index file with a name that depends on the index path (for example maybe the sha1 of the index path) then as many index files with the same path are not possible we should be safe.

Isn't is possible that the same index (so with the same trailing sha1) be created in two different places?

If the new index is always prepared in $GIT_DIR/index.lock, then there is no problem in the first place because when my original patch calls clean_shared_index_files() the $GIT_DIR/index.lock is taken. Or maybe I am missing something?

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 cache.h  |  1 +
 config.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

On Mon, Jul 11, 2016 at 7:22 PM, Christian Couder <christian.couder@gmail.com> wrote:

Oh yes I would like something like this. I stuck to the basics because as you see you need to define some criteria to re-split again, but without experimenting on real repos, I could just have gone the wrong way. Index file size or the percentage of entries in linked/shared indexes are two good candidates. You can also just re-split on commands that likely lead to increasing linked index size a lot (maybe git-reset), or run long enough that some extra processing won't get noticed. For example git-gc should definitely re-split if this feature is on, but I can't say if it's often enough.

On Tue, Jul 12, 2016 at 6:01 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Ok, I started working on automatically pushing back all changes to the shared index when the percentage of entries in linked vs shared indexes is greater than 25% (maybe I will make it configurable later). It is very basic and doesn't work well for now (for one thing it is missing added entries), see:

https://github.com/chriscool/git/commits/config-split-index8

Basically I would like a way to count then entries that are only in the linked index without modifying them to be safe, but I have a hard time seeing how I could modify prepare_to_write_split_index() to get that.

I'd like to avoid re-split only on some specific commands as I feel it could lead to bad behavior when none of these specific commands are called but the linked index is growing because of other commands.

On Sat, Jul 23, 2016 at 6:11 PM, Christian Couder <christian.couder@gmail.com> wrote:

Hmm.. can you do the counting separately? A shared cache_entry must have its field "index" greater than zero. By counting the number of entries whose index is zero (i.e. not shared) against the total number of real (*) entries, you should have a decent estimate when to split. Then you can do exactly what "git update-index --no-split-index" and "git update-index --split-index" sequence does, but in write_index(). It's easier than messing inside split-index.c. If we hit performance problem, then we can look into changing split-index.c

(*) remember that some entries may be marked CE_REMOVE, which are dead entries and should not be counted because they will never be written down on disk.

On Mon, Jul 25, 2016 at 6:04 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Yeah, thanks for the suggestion. I will try it.