Merge branch 'nd/split-index-unshare'

Plug some leaks and updates internal API used to implement the
split index feature to make it easier to avoid such a leak in the
future.

* nd/split-index-unshare:
p3400: add perf tests for rebasing many changes
split-index: add and use unshare_split_index()

diff --git a/read-cache.c b/read-cache.c
index 3339de812460c903db085dc2be39e4c8b491478f..22ab8b5b61636cbc3bf8bcaf5a2ef964bf69c4c3 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -1877,15 +1877,9 @@ int discard_index(struct index_state *istate)
 {
        int i;
 
-       for (i = 0; i < istate->cache_nr; i++) {
-               if (istate->cache[i]->index &&
-                   istate->split_index &&
-                   istate->split_index->base &&
-                   istate->cache[i]->index <= istate->split_index->base->cache_nr &&
-                   istate->cache[i] == istate->split_index->base->cache[istate->cache[i]->index - 1])
-                       continue;
+       unshare_split_index(istate, 1);
+       for (i = 0; i < istate->cache_nr; i++)
                free(istate->cache[i]);
-       }
        resolve_undo_clear_index(istate);
        istate->cache_nr = 0;
        istate->cache_changed = 0;

diff --git a/split-index.c b/split-index.c
index f519e60f87578386ec9b4ea764bf530be0f64836..49bd197f715ae783d7fdbe8e1d638a1bcac0073c 100644 (file)
--- a/split-index.c
+++ b/split-index.c
@@ -73,10 +73,17 @@ void move_cache_to_base_index(struct index_state *istate)
        int i;
 
        /*
-        * do not delete old si->base, its index entries may be shared
-        * with istate->cache[]. Accept a bit of leaking here because
-        * this code is only used by short-lived update-index.
+        * If "si" is shared with another index_state (e.g. by
+        * unpack-trees code), we will need to duplicate split_index
+        * struct. It's not happening now though, luckily.
         */
+       assert(si->refcount <= 1);
+
+       unshare_split_index(istate, 0);
+       if (si->base) {
+               discard_index(si->base);
+               free(si->base);
+       }
        si->base = xcalloc(1, sizeof(*si->base));
        si->base->version = istate->version;
        /* zero timestamp disables racy test in ce_write_index() */
@@ -275,11 +282,41 @@ void finish_writing_split_index(struct index_state *istate)
        istate->cache_nr = si->saved_cache_nr;
 }
 
+void unshare_split_index(struct index_state *istate, int discard)
+{
+       struct split_index *si = istate->split_index;
+       int i;
+
+       if (!si || !si->base)
+               return;
+
+       for (i = 0; i < istate->cache_nr; i++) {
+               struct cache_entry *ce = istate->cache[i];
+               struct cache_entry *new = NULL;
+
+               if (!ce->index ||
+                   ce->index > si->base->cache_nr ||
+                   ce != si->base->cache[ce->index - 1])
+                       continue;
+
+               if (!discard) {
+                       int len = ce_namelen(ce);
+                       new = xcalloc(1, cache_entry_size(len));
+                       copy_cache_entry(new, ce);
+                       memcpy(new->name, ce->name, len);
+                       new->index = 0;
+               }
+               istate->cache[i] = new;
+       }
+}
+
+
 void discard_split_index(struct index_state *istate)
 {
        struct split_index *si = istate->split_index;
        if (!si)
                return;
+       unshare_split_index(istate, 0);
        istate->split_index = NULL;
        si->refcount--;
        if (si->refcount)
@@ -328,14 +365,8 @@ void add_split_index(struct index_state *istate)
 
 void remove_split_index(struct index_state *istate)
 {
-       if (istate->split_index) {
-               /*
-                * can't discard_split_index(&the_index); because that
-                * will destroy split_index->base->cache[], which may
-                * be shared with the_index.cache[]. So yeah we're
-                * leaking a bit here.
-                */
-               istate->split_index = NULL;
-               istate->cache_changed |= SOMETHING_CHANGED;
-       }
+       if (!istate->split_index)
+               return;
+       discard_split_index(istate);
+       istate->cache_changed |= SOMETHING_CHANGED;
 }

diff --git a/split-index.h b/split-index.h
index df91c1bda8117fe7d0f25a0aaedce79370801ce0..65c0f09b2bd413e091cc65ef2f0d3a2d92aeb858 100644 (file)
--- a/split-index.h
+++ b/split-index.h
@@ -33,5 +33,6 @@ void finish_writing_split_index(struct index_state *istate);
 void discard_split_index(struct index_state *istate);
 void add_split_index(struct index_state *istate);
 void remove_split_index(struct index_state *istate);
+void unshare_split_index(struct index_state *istate, int discard);
 
 #endif

diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh
index b3e7d525d277c339b4f9d821fb6d18dfb2aa2621..ce271ca4c1a07f620f25c5f4dd6febe020308c64 100755 (executable)
--- a/t/perf/p3400-rebase.sh
+++ b/t/perf/p3400-rebase.sh
@@ -5,7 +5,7 @@ test_description='Tests rebase performance'
 
 test_perf_default_repo
 
-test_expect_success 'setup' '
+test_expect_success 'setup rebasing on top of a lot of changes' '
        git checkout -f -b base &&
        git checkout -b to-rebase &&
        git checkout -b upstream &&
@@ -33,4 +33,24 @@ test_perf 'rebase on top of a lot of unrelated changes' '
        git rebase --onto base HEAD^
 '
 
+test_expect_success 'setup rebasing many changes without split-index' '
+       git config core.splitIndex false &&
+       git checkout -b upstream2 to-rebase &&
+       git checkout -b to-rebase2 upstream
+'
+
+test_perf 'rebase a lot of unrelated changes without split-index' '
+       git rebase --onto upstream2 base &&
+       git rebase --onto base upstream2
+'
+
+test_expect_success 'setup rebasing many changes with split-index' '
+       git config core.splitIndex true
+'
+
+test_perf 'rebase a lot of unrelated changes with split-index' '
+       git rebase --onto upstream2 base &&
+       git rebase --onto base upstream2
+'
+
 test_done