Skip to content

Commit 9540ce5

Browse files
peffgitster
authored andcommitted
refs: write packed_refs file using stdio
We write each line of a new packed-refs file individually using a write() syscall (and sometimes 2, if the ref is peeled). Since each line is only about 50-100 bytes long, this creates a lot of system call overhead. We can instead open a stdio handle around our descriptor and use fprintf to write to it. The extra buffering is not a problem for us, because nobody will read our new packed-refs file until we call commit_lock_file (by which point we have flushed everything). On a pathological repository with 8.5 million refs, this dropped the time to run `git pack-refs` from 20s to 6s. Signed-off-by: Jeff King <[email protected]> Reviewed-by: Michael Haggerty <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0c72b98 commit 9540ce5

File tree

3 files changed

+33
-23
lines changed

3 files changed

+33
-23
lines changed

cache.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,6 +1395,8 @@ extern const char *git_mailmap_blob;
13951395

13961396
/* IO helper functions */
13971397
extern void maybe_flush_or_die(FILE *, const char *);
1398+
__attribute__((format (printf, 2, 3)))
1399+
extern void fprintf_or_die(FILE *, const char *fmt, ...);
13981400
extern int copy_fd(int ifd, int ofd);
13991401
extern int copy_file(const char *dst, const char *src, int mode);
14001402
extern int copy_file_with_time(const char *dst, const char *src, int mode);

refs.c

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,39 +2191,25 @@ struct ref_lock *lock_any_ref_for_update(const char *refname,
21912191
* Write an entry to the packed-refs file for the specified refname.
21922192
* If peeled is non-NULL, write it as the entry's peeled value.
21932193
*/
2194-
static void write_packed_entry(int fd, char *refname, unsigned char *sha1,
2194+
static void write_packed_entry(FILE *fh, char *refname, unsigned char *sha1,
21952195
unsigned char *peeled)
21962196
{
2197-
char line[PATH_MAX + 100];
2198-
int len;
2199-
2200-
len = snprintf(line, sizeof(line), "%s %s\n",
2201-
sha1_to_hex(sha1), refname);
2202-
/* this should not happen but just being defensive */
2203-
if (len > sizeof(line))
2204-
die("too long a refname '%s'", refname);
2205-
write_or_die(fd, line, len);
2206-
2207-
if (peeled) {
2208-
if (snprintf(line, sizeof(line), "^%s\n",
2209-
sha1_to_hex(peeled)) != PEELED_LINE_LENGTH)
2210-
die("internal error");
2211-
write_or_die(fd, line, PEELED_LINE_LENGTH);
2212-
}
2197+
fprintf_or_die(fh, "%s %s\n", sha1_to_hex(sha1), refname);
2198+
if (peeled)
2199+
fprintf_or_die(fh, "^%s\n", sha1_to_hex(peeled));
22132200
}
22142201

22152202
/*
22162203
* An each_ref_entry_fn that writes the entry to a packed-refs file.
22172204
*/
22182205
static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
22192206
{
2220-
int *fd = cb_data;
22212207
enum peel_status peel_status = peel_entry(entry, 0);
22222208

22232209
if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
22242210
error("internal error: %s is not a valid packed reference!",
22252211
entry->name);
2226-
write_packed_entry(*fd, entry->name, entry->u.value.sha1,
2212+
write_packed_entry(cb_data, entry->name, entry->u.value.sha1,
22272213
peel_status == PEEL_PEELED ?
22282214
entry->u.value.peeled : NULL);
22292215
return 0;
@@ -2259,15 +2245,22 @@ int commit_packed_refs(void)
22592245
get_packed_ref_cache(&ref_cache);
22602246
int error = 0;
22612247
int save_errno = 0;
2248+
FILE *out;
22622249

22632250
if (!packed_ref_cache->lock)
22642251
die("internal error: packed-refs not locked");
2265-
write_or_die(packed_ref_cache->lock->fd,
2266-
PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
22672252

2253+
out = fdopen(packed_ref_cache->lock->fd, "w");
2254+
if (!out)
2255+
die_errno("unable to fdopen packed-refs descriptor");
2256+
2257+
fprintf_or_die(out, "%s", PACKED_REFS_HEADER);
22682258
do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
2269-
0, write_packed_entry_fn,
2270-
&packed_ref_cache->lock->fd);
2259+
0, write_packed_entry_fn, out);
2260+
if (fclose(out))
2261+
die_errno("write error");
2262+
packed_ref_cache->lock->fd = -1;
2263+
22712264
if (commit_lock_file(packed_ref_cache->lock)) {
22722265
save_errno = errno;
22732266
error = -1;

write_or_die.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,21 @@ void maybe_flush_or_die(FILE *f, const char *desc)
4949
}
5050
}
5151

52+
void fprintf_or_die(FILE *f, const char *fmt, ...)
53+
{
54+
va_list ap;
55+
int ret;
56+
57+
va_start(ap, fmt);
58+
ret = vfprintf(f, fmt, ap);
59+
va_end(ap);
60+
61+
if (ret < 0) {
62+
check_pipe(errno);
63+
die_errno("write error");
64+
}
65+
}
66+
5267
void fsync_or_die(int fd, const char *msg)
5368
{
5469
if (fsync(fd) < 0) {

0 commit comments

Comments
 (0)