Skip to content

Commit 030faf2

Browse files
gitstergitster
committed
Merge branch 'kw/write-index-reduce-alloc'
We used to spend more than necessary cycles allocating and freeing piece of memory while writing each index entry out. This has been optimized. * kw/write-index-reduce-alloc: read-cache: avoid allocating every ondisk entry when writing read-cache: fix memory leak in do_write_index perf: add test for writing the index
2 parents 614ea03 + ce012de commit 030faf2

File tree

4 files changed

+87
-28
lines changed

4 files changed

+87
-28
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,7 @@ TEST_PROGRAMS_NEED_X += test-parse-options
655655
TEST_PROGRAMS_NEED_X += test-path-utils
656656
TEST_PROGRAMS_NEED_X += test-prio-queue
657657
TEST_PROGRAMS_NEED_X += test-read-cache
658+
TEST_PROGRAMS_NEED_X += test-write-cache
658659
TEST_PROGRAMS_NEED_X += test-ref-store
659660
TEST_PROGRAMS_NEED_X += test-regex
660661
TEST_PROGRAMS_NEED_X += test-revision-walking

‎read-cache.c‎

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,6 +1499,7 @@ struct ondisk_cache_entry_extended {
14991499
};
15001500

15011501
/* These are only used for v3 or lower */
1502+
#define align_padding_size(size, len) ((size + (len) + 8) & ~7) - (size + len)
15021503
#define align_flex_name(STRUCT,len) ((offsetof(struct STRUCT,name) + (len) + 8) & ~7)
15031504
#define ondisk_cache_entry_size(len) align_flex_name(ondisk_cache_entry,len)
15041505
#define ondisk_cache_entry_extended_size(len) align_flex_name(ondisk_cache_entry_extended,len)
@@ -2032,7 +2033,7 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)
20322033
}
20332034

20342035
/* Copy miscellaneous fields but not the name */
2035-
static char *copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,
2036+
static void copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,
20362037
struct cache_entry *ce)
20372038
{
20382039
short flags;
@@ -2056,32 +2057,35 @@ static char *copy_cache_entry_to_ondisk(struct ondisk_cache_entry *ondisk,
20562057
struct ondisk_cache_entry_extended *ondisk2;
20572058
ondisk2 = (struct ondisk_cache_entry_extended *)ondisk;
20582059
ondisk2->flags2 = htons((ce->ce_flags & CE_EXTENDED_FLAGS) >> 16);
2059-
return ondisk2->name;
2060-
}
2061-
else {
2062-
return ondisk->name;
20632060
}
20642061
}
20652062

20662063
static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
2067-
struct strbuf *previous_name)
2064+
struct strbuf *previous_name, struct ondisk_cache_entry *ondisk)
20682065
{
20692066
int size;
2070-
struct ondisk_cache_entry *ondisk;
20712067
int saved_namelen = saved_namelen; /* compiler workaround */
2072-
char *name;
20732068
int result;
2069+
static unsigned char padding[8] = { 0x00 };
20742070

20752071
if (ce->ce_flags & CE_STRIP_NAME) {
20762072
saved_namelen = ce_namelen(ce);
20772073
ce->ce_namelen = 0;
20782074
}
20792075

2076+
if (ce->ce_flags & CE_EXTENDED)
2077+
size = offsetof(struct ondisk_cache_entry_extended, name);
2078+
else
2079+
size = offsetof(struct ondisk_cache_entry, name);
2080+
20802081
if (!previous_name) {
2081-
size = ondisk_ce_size(ce);
2082-
ondisk = xcalloc(1, size);
2083-
name = copy_cache_entry_to_ondisk(ondisk, ce);
2084-
memcpy(name, ce->name, ce_namelen(ce));
2082+
int len = ce_namelen(ce);
2083+
copy_cache_entry_to_ondisk(ondisk, ce);
2084+
result = ce_write(c, fd, ondisk, size);
2085+
if (!result)
2086+
result = ce_write(c, fd, ce->name, len);
2087+
if (!result)
2088+
result = ce_write(c, fd, padding, align_padding_size(size, len));
20852089
} else {
20862090
int common, to_remove, prefix_size;
20872091
unsigned char to_remove_vi[16];
@@ -2094,16 +2098,12 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
20942098
to_remove = previous_name->len - common;
20952099
prefix_size = encode_varint(to_remove, to_remove_vi);
20962100

2097-
if (ce->ce_flags & CE_EXTENDED)
2098-
size = offsetof(struct ondisk_cache_entry_extended, name);
2099-
else
2100-
size = offsetof(struct ondisk_cache_entry, name);
2101-
size += prefix_size + (ce_namelen(ce) - common + 1);
2102-
2103-
ondisk = xcalloc(1, size);
2104-
name = copy_cache_entry_to_ondisk(ondisk, ce);
2105-
memcpy(name, to_remove_vi, prefix_size);
2106-
memcpy(name + prefix_size, ce->name + common, ce_namelen(ce) - common);
2101+
copy_cache_entry_to_ondisk(ondisk, ce);
2102+
result = ce_write(c, fd, ondisk, size);
2103+
if (!result)
2104+
result = ce_write(c, fd, to_remove_vi, prefix_size);
2105+
if (!result)
2106+
result = ce_write(c, fd, ce->name + common, ce_namelen(ce) - common + 1);
21072107

21082108
strbuf_splice(previous_name, common, to_remove,
21092109
ce->name + common, ce_namelen(ce) - common);
@@ -2113,8 +2113,6 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce,
21132113
ce->ce_flags &= ~CE_STRIP_NAME;
21142114
}
21152115

2116-
result = ce_write(c, fd, ondisk, size);
2117-
free(ondisk);
21182116
return result;
21192117
}
21202118

@@ -2192,10 +2190,11 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
21922190
int newfd = tempfile->fd;
21932191
git_SHA_CTX c;
21942192
struct cache_header hdr;
2195-
int i, err, removed, extended, hdr_version;
2193+
int i, err = 0, removed, extended, hdr_version;
21962194
struct cache_entry **cache = istate->cache;
21972195
int entries = istate->cache_nr;
21982196
struct stat st;
2197+
struct ondisk_cache_entry_extended ondisk;
21992198
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
22002199
int drop_cache_tree = 0;
22012200

@@ -2232,6 +2231,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
22322231
return -1;
22332232

22342233
previous_name = (hdr_version == 4) ? &previous_name_buf : NULL;
2234+
22352235
for (i = 0; i < entries; i++) {
22362236
struct cache_entry *ce = cache[i];
22372237
if (ce->ce_flags & CE_REMOVE)
@@ -2247,15 +2247,21 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
22472247
if (allow)
22482248
warning(msg, ce->name);
22492249
else
2250-
return error(msg, ce->name);
2250+
err = error(msg, ce->name);
22512251

22522252
drop_cache_tree = 1;
22532253
}
2254-
if (ce_write_entry(&c, newfd, ce, previous_name) < 0)
2255-
return -1;
2254+
if (ce_write_entry(&c, newfd, ce, previous_name, (struct ondisk_cache_entry *)&ondisk) < 0)
2255+
err = -1;
2256+
2257+
if (err)
2258+
break;
22562259
}
22572260
strbuf_release(&previous_name_buf);
22582261

2262+
if (err)
2263+
return err;
2264+
22592265
/* Write extension data here */
22602266
if (!strip_extensions && istate->split_index) {
22612267
struct strbuf sb = STRBUF_INIT;

‎t/helper/test-write-cache.c‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#include "cache.h"
2+
#include "lockfile.h"
3+
4+
static struct lock_file index_lock;
5+
6+
int cmd_main(int argc, const char **argv)
7+
{
8+
int i, cnt = 1, lockfd;
9+
if (argc == 2)
10+
cnt = strtol(argv[1], NULL, 0);
11+
setup_git_directory();
12+
read_cache();
13+
for (i = 0; i < cnt; i++) {
14+
lockfd = hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
15+
if (0 <= lockfd) {
16+
write_locked_index(&the_index, &index_lock, COMMIT_LOCK);
17+
} else {
18+
rollback_lock_file(&index_lock);
19+
}
20+
}
21+
22+
return 0;
23+
}

‎t/perf/p0007-write-cache.sh‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#!/bin/sh
2+
3+
test_description="Tests performance of writing the index"
4+
5+
. ./perf-lib.sh
6+
7+
test_perf_default_repo
8+
9+
test_expect_success "setup repo" '
10+
if git rev-parse --verify refs/heads/p0006-ballast^{commit}
11+
then
12+
echo Assuming synthetic repo from many-files.sh
13+
git config --local core.sparsecheckout 1
14+
cat >.git/info/sparse-checkout <<-EOF
15+
/*
16+
!ballast/*
17+
EOF
18+
else
19+
echo Assuming non-synthetic repo...
20+
fi &&
21+
nr_files=$(git ls-files | wc -l)
22+
'
23+
24+
count=3
25+
test_perf "write_locked_index $count times ($nr_files files)" "
26+
test-write-cache $count
27+
"
28+
29+
test_done

0 commit comments

Comments
 (0)