Skip to content

Commit f382b75

Browse files
gitstergitster
committed
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()
2 parents a531ecf + de950c5 commit f382b75

File tree

4 files changed

+68
-22
lines changed

4 files changed

+68
-22
lines changed

‎read-cache.c‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,15 +1877,9 @@ int discard_index(struct index_state *istate)
18771877
{
18781878
int i;
18791879

1880-
for (i = 0; i < istate->cache_nr; i++) {
1881-
if (istate->cache[i]->index &&
1882-
istate->split_index &&
1883-
istate->split_index->base &&
1884-
istate->cache[i]->index <= istate->split_index->base->cache_nr &&
1885-
istate->cache[i] == istate->split_index->base->cache[istate->cache[i]->index - 1])
1886-
continue;
1880+
unshare_split_index(istate, 1);
1881+
for (i = 0; i < istate->cache_nr; i++)
18871882
free(istate->cache[i]);
1888-
}
18891883
resolve_undo_clear_index(istate);
18901884
istate->cache_nr = 0;
18911885
istate->cache_changed = 0;

‎split-index.c‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,17 @@ void move_cache_to_base_index(struct index_state *istate)
7373
int i;
7474

7575
/*
76-
* do not delete old si->base, its index entries may be shared
77-
* with istate->cache[]. Accept a bit of leaking here because
78-
* this code is only used by short-lived update-index.
76+
* If "si" is shared with another index_state (e.g. by
77+
* unpack-trees code), we will need to duplicate split_index
78+
* struct. It's not happening now though, luckily.
7979
*/
80+
assert(si->refcount <= 1);
81+
82+
unshare_split_index(istate, 0);
83+
if (si->base) {
84+
discard_index(si->base);
85+
free(si->base);
86+
}
8087
si->base = xcalloc(1, sizeof(*si->base));
8188
si->base->version = istate->version;
8289
/* zero timestamp disables racy test in ce_write_index() */
@@ -275,11 +282,41 @@ void finish_writing_split_index(struct index_state *istate)
275282
istate->cache_nr = si->saved_cache_nr;
276283
}
277284

285+
void unshare_split_index(struct index_state *istate, int discard)
286+
{
287+
struct split_index *si = istate->split_index;
288+
int i;
289+
290+
if (!si || !si->base)
291+
return;
292+
293+
for (i = 0; i < istate->cache_nr; i++) {
294+
struct cache_entry *ce = istate->cache[i];
295+
struct cache_entry *new = NULL;
296+
297+
if (!ce->index ||
298+
ce->index > si->base->cache_nr ||
299+
ce != si->base->cache[ce->index - 1])
300+
continue;
301+
302+
if (!discard) {
303+
int len = ce_namelen(ce);
304+
new = xcalloc(1, cache_entry_size(len));
305+
copy_cache_entry(new, ce);
306+
memcpy(new->name, ce->name, len);
307+
new->index = 0;
308+
}
309+
istate->cache[i] = new;
310+
}
311+
}
312+
313+
278314
void discard_split_index(struct index_state *istate)
279315
{
280316
struct split_index *si = istate->split_index;
281317
if (!si)
282318
return;
319+
unshare_split_index(istate, 0);
283320
istate->split_index = NULL;
284321
si->refcount--;
285322
if (si->refcount)
@@ -328,14 +365,8 @@ void add_split_index(struct index_state *istate)
328365

329366
void remove_split_index(struct index_state *istate)
330367
{
331-
if (istate->split_index) {
332-
/*
333-
* can't discard_split_index(&the_index); because that
334-
* will destroy split_index->base->cache[], which may
335-
* be shared with the_index.cache[]. So yeah we're
336-
* leaking a bit here.
337-
*/
338-
istate->split_index = NULL;
339-
istate->cache_changed |= SOMETHING_CHANGED;
340-
}
368+
if (!istate->split_index)
369+
return;
370+
discard_split_index(istate);
371+
istate->cache_changed |= SOMETHING_CHANGED;
341372
}

‎split-index.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,6 @@ void finish_writing_split_index(struct index_state *istate);
3333
void discard_split_index(struct index_state *istate);
3434
void add_split_index(struct index_state *istate);
3535
void remove_split_index(struct index_state *istate);
36+
void unshare_split_index(struct index_state *istate, int discard);
3637

3738
#endif

‎t/perf/p3400-rebase.sh‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ test_description='Tests rebase performance'
55

66
test_perf_default_repo
77

8-
test_expect_success 'setup' '
8+
test_expect_success 'setup rebasing on top of a lot of changes' '
99
git checkout -f -b base &&
1010
git checkout -b to-rebase &&
1111
git checkout -b upstream &&
@@ -33,4 +33,24 @@ test_perf 'rebase on top of a lot of unrelated changes' '
3333
git rebase --onto base HEAD^
3434
'
3535

36+
test_expect_success 'setup rebasing many changes without split-index' '
37+
git config core.splitIndex false &&
38+
git checkout -b upstream2 to-rebase &&
39+
git checkout -b to-rebase2 upstream
40+
'
41+
42+
test_perf 'rebase a lot of unrelated changes without split-index' '
43+
git rebase --onto upstream2 base &&
44+
git rebase --onto base upstream2
45+
'
46+
47+
test_expect_success 'setup rebasing many changes with split-index' '
48+
git config core.splitIndex true
49+
'
50+
51+
test_perf 'rebase a lot of unrelated changes with split-index' '
52+
git rebase --onto upstream2 base &&
53+
git rebase --onto base upstream2
54+
'
55+
3656
test_done

0 commit comments

Comments
 (0)