Merge branch 'ew/cat-file-optim' into jch

"git cat-file --batch" has been optimized.

* ew/cat-file-optim:
  cat-file: use writev(2) if available
  cat-file: batch_write: use size_t for length
  cat-file: batch-command uses content_limit
  object_info: content_limit only applies to blobs
  packfile: packed_object_info avoids packed_to_object_type
  cat-file: use delta_base_cache entries directly
  packfile: inline cache_or_unpack_entry
  packfile: fix off-by-one in content_limit comparison
  packfile: allow content-limit for cat-file
  packfile: move sizep computation
This commit is contained in:
Taylor Blau
2024-11-01 15:40:25 -04:00
13 changed files with 312 additions and 83 deletions

View File

@@ -1870,6 +1870,9 @@ ifdef NO_PREAD
COMPAT_CFLAGS += -DNO_PREAD
COMPAT_OBJS += compat/pread.o
endif
ifdef HAVE_WRITEV
COMPAT_CFLAGS += -DHAVE_WRITEV
endif
ifdef NO_FAST_WORKING_DIRECTORY
BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
endif

View File

@@ -286,6 +286,7 @@ struct expand_data {
off_t disk_size;
const char *rest;
struct object_id delta_base_oid;
struct git_iovec iov[3];
/*
* If mark_query is true, we do not expand anything, but rather
@@ -374,7 +375,7 @@ static void expand_format(struct strbuf *sb, const char *start,
}
}
static void batch_write(struct batch_options *opt, const void *data, int len)
static void batch_write(struct batch_options *opt, const void *data, size_t len)
{
if (opt->buffer_output) {
if (fwrite(data, 1, len, stdout) != len)
@@ -383,15 +384,72 @@ static void batch_write(struct batch_options *opt, const void *data, int len)
write_or_die(1, data, len);
}
static void print_object_or_die(struct batch_options *opt, struct expand_data *data)
static void batch_writev(struct batch_options *opt, struct expand_data *data,
const struct strbuf *hdr, size_t size)
{
data->iov[0].iov_base = hdr->buf;
data->iov[0].iov_len = hdr->len;
data->iov[1].iov_len = size;
/*
* Copying a (8|16)-byte iovec for a single byte is gross, but my
* attempt to stuff output_delim into the trailing NUL byte of
* iov[1].iov_base (and restoring it after writev(2) for the
* OI_DBCACHED case) to drop iovcnt from 3->2 wasn't faster.
*/
data->iov[2].iov_base = &opt->output_delim;
data->iov[2].iov_len = 1;
if (opt->buffer_output)
fwritev_or_die(stdout, data->iov, 3);
else
writev_or_die(1, data->iov, 3);
/* writev_or_die may move iov[1].iov_base, so it's invalid */
data->iov[1].iov_base = NULL;
}
static void print_object_or_die(struct batch_options *opt,
struct expand_data *data, struct strbuf *hdr)
{
const struct object_id *oid = &data->oid;
assert(data->info.typep);
if (data->type == OBJ_BLOB) {
if (opt->buffer_output)
fflush(stdout);
if (data->iov[1].iov_base) {
void *content = data->iov[1].iov_base;
unsigned long size = data->size;
if (use_mailmap && (data->type == OBJ_COMMIT ||
data->type == OBJ_TAG)) {
size_t s = size;
if (data->info.whence == OI_DBCACHED) {
content = xmemdupz(content, s);
data->info.whence = OI_PACKED;
}
content = replace_idents_using_mailmap(content, &s);
data->iov[1].iov_base = content;
size = cast_size_t_to_ulong(s);
}
batch_writev(opt, data, hdr, size);
switch (data->info.whence) {
case OI_CACHED:
/*
* only blame uses OI_CACHED atm, so it's unlikely
* we'll ever hit this path
*/
BUG("TODO OI_CACHED support not done");
case OI_LOOSE:
case OI_PACKED:
free(content);
break;
case OI_DBCACHED:
unlock_delta_base_cache();
}
} else {
assert(data->type == OBJ_BLOB);
if (opt->transform_mode) {
char *contents;
unsigned long size;
@@ -418,36 +476,17 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
oid_to_hex(oid), data->rest);
} else
BUG("invalid transform_mode: %c", opt->transform_mode);
batch_write(opt, contents, size);
data->iov[1].iov_base = contents;
batch_writev(opt, data, hdr, size);
free(contents);
} else {
batch_write(opt, hdr->buf, hdr->len);
if (opt->buffer_output)
fflush(stdout);
stream_blob(oid);
batch_write(opt, &opt->output_delim, 1);
}
}
else {
enum object_type type;
unsigned long size;
void *contents;
contents = repo_read_object_file(the_repository, oid, &type,
&size);
if (!contents)
die("object %s disappeared", oid_to_hex(oid));
if (use_mailmap) {
size_t s = size;
contents = replace_idents_using_mailmap(contents, &s);
size = cast_size_t_to_ulong(s);
}
if (type != data->type)
die("object %s changed type!?", oid_to_hex(oid));
if (data->info.sizep && size != data->size && !use_mailmap)
die("object %s changed size!?", oid_to_hex(oid));
batch_write(opt, contents, size);
free(contents);
}
}
static void print_default_format(struct strbuf *scratch, struct expand_data *data,
@@ -514,12 +553,10 @@ static void batch_object_write(const char *obj_name,
strbuf_addch(scratch, opt->output_delim);
}
batch_write(opt, scratch->buf, scratch->len);
if (opt->batch_mode == BATCH_MODE_CONTENTS) {
print_object_or_die(opt, data);
batch_write(opt, &opt->output_delim, 1);
}
if (opt->batch_mode == BATCH_MODE_CONTENTS)
print_object_or_die(opt, data, scratch);
else
batch_write(opt, scratch->buf, scratch->len);
}
static void batch_one_object(const char *obj_name,
@@ -714,6 +751,7 @@ static void parse_cmd_contents(struct batch_options *opt,
struct expand_data *data)
{
opt->batch_mode = BATCH_MODE_CONTENTS;
data->info.contentp = &data->iov[1].iov_base;
batch_one_object(line, output, opt, data);
}
@@ -723,6 +761,7 @@ static void parse_cmd_info(struct batch_options *opt,
struct expand_data *data)
{
opt->batch_mode = BATCH_MODE_INFO;
data->info.contentp = NULL;
batch_one_object(line, output, opt, data);
}
@@ -902,9 +941,20 @@ static int batch_objects(struct batch_options *opt)
/*
* If we are printing out the object, then always fill in the type,
* since we will want to decide whether or not to stream.
*
* Likewise, grab the content in the initial request if it's small
* and we're not planning to filter it.
*/
if (opt->batch_mode == BATCH_MODE_CONTENTS)
if ((opt->batch_mode == BATCH_MODE_CONTENTS) ||
(opt->batch_mode == BATCH_MODE_QUEUE_AND_DISPATCH)) {
data.info.typep = &data.type;
if (!opt->transform_mode) {
data.info.sizep = &data.size;
data.info.contentp = &data.iov[1].iov_base;
data.info.content_limit = big_file_threshold;
data.info.direct_cache = 1;
}
}
if (opt->all_objects) {
struct object_cb_data cb;

View File

@@ -68,6 +68,7 @@ ifeq ($(uname_S),Linux)
BASIC_CFLAGS += -std=c99
endif
LINK_FUZZ_PROGRAMS = YesPlease
HAVE_WRITEV = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_ALLOCA_H = YesPlease
@@ -76,6 +77,7 @@ ifeq ($(uname_S),GNU/kFreeBSD)
DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
LIBC_CONTAINS_LIBINTL = YesPlease
FREAD_READS_DIRECTORIES = UnfortunatelyYes
HAVE_WRITEV = YesPlease
endif
ifeq ($(uname_S),UnixWare)
CC = cc
@@ -292,6 +294,7 @@ ifeq ($(uname_S),FreeBSD)
PAGER_ENV = LESS=FRX LV=-c MORE=FRX
FREAD_READS_DIRECTORIES = UnfortunatelyYes
FILENO_IS_A_MACRO = UnfortunatelyYes
HAVE_WRITEV = YesPlease
endif
ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
@@ -307,6 +310,7 @@ ifeq ($(uname_S),OpenBSD)
PROCFS_EXECUTABLE_PATH = /proc/curproc/file
FREAD_READS_DIRECTORIES = UnfortunatelyYes
FILENO_IS_A_MACRO = UnfortunatelyYes
HAVE_WRITEV = YesPlease
endif
ifeq ($(uname_S),MirBSD)
NO_STRCASESTR = YesPlease
@@ -329,6 +333,7 @@ ifeq ($(uname_S),NetBSD)
HAVE_BSD_KERN_PROC_SYSCTL = YesPlease
CSPRNG_METHOD = arc4random
PROCFS_EXECUTABLE_PATH = /proc/curproc/exe
HAVE_WRITEV = YesPlease
endif
ifeq ($(uname_S),AIX)
DEFAULT_PAGER = more

View File

@@ -401,6 +401,16 @@ static inline int git_setitimer(int which UNUSED,
#define setitimer(which,value,ovalue) git_setitimer(which,value,ovalue)
#endif
#ifdef HAVE_WRITEV
#include <sys/uio.h>
#define git_iovec iovec
#else /* !HAVE_WRITEV */
struct git_iovec {
void *iov_base;
size_t iov_len;
};
#endif /* !HAVE_WRITEV */
#ifndef NO_LIBGEN_H
#include <libgen.h>
#else

View File

@@ -1567,6 +1567,13 @@ static int loose_object_info(struct repository *r,
if (!oi->contentp)
break;
if (oi->content_limit && *oi->typep == OBJ_BLOB &&
*oi->sizep > oi->content_limit) {
git_inflate_end(&stream);
oi->contentp = NULL;
goto cleanup;
}
*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);
if (*oi->contentp)
goto cleanup;
@@ -1655,6 +1662,11 @@ static int do_oid_object_info_extended(struct repository *r,
oidclr(oi->delta_base_oid, the_repository->hash_algo);
if (oi->type_name)
strbuf_addstr(oi->type_name, type_name(co->type));
/*
* Currently `blame' is the only command which creates
* OI_CACHED, and direct_cache is only used by `cat-file'.
*/
assert(!oi->direct_cache);
if (oi->contentp)
*oi->contentp = xmemdupz(co->buf, co->size);
oi->whence = OI_CACHED;

View File

@@ -304,6 +304,7 @@ struct object_info {
struct object_id *delta_base_oid;
struct strbuf *type_name;
void **contentp;
size_t content_limit;
/* Response */
enum {
@@ -312,6 +313,14 @@ struct object_info {
OI_PACKED,
OI_DBCACHED
} whence;
/*
* Set if caller is able to use OI_DBCACHED entries without copying.
* This only applies to OI_DBCACHED entries at the moment,
* not OI_CACHED or any other type of entry.
*/
unsigned direct_cache:1;
union {
/*
* struct {

View File

@@ -1372,6 +1372,14 @@ unwind:
static struct hashmap delta_base_cache;
static size_t delta_base_cached;
/*
* Ensures only a single object is used at-a-time via oi->direct_cache.
* Using two objects directly at once (e.g. diff) would cause corruption
* since populating the cache may invalidate existing entries.
* This lock has nothing to do with parallelism at the moment.
*/
static int delta_base_cache_lock;
static LIST_HEAD(delta_base_cache_lru);
struct delta_base_cache_key {
@@ -1454,21 +1462,16 @@ static void detach_delta_base_cache_entry(struct delta_base_cache_entry *ent)
free(ent);
}
static void *cache_or_unpack_entry(struct repository *r, struct packed_git *p,
off_t base_offset, unsigned long *base_size,
enum object_type *type)
static void lock_delta_base_cache(void)
{
struct delta_base_cache_entry *ent;
delta_base_cache_lock++;
assert(delta_base_cache_lock == 1);
}
ent = get_delta_base_cache_entry(p, base_offset);
if (!ent)
return unpack_entry(r, p, base_offset, type, base_size);
if (type)
*type = ent->type;
if (base_size)
*base_size = ent->size;
return xmemdupz(ent->data, ent->size);
void unlock_delta_base_cache(void)
{
delta_base_cache_lock--;
assert(delta_base_cache_lock == 0);
}
static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
@@ -1480,6 +1483,7 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
void clear_delta_base_cache(void)
{
struct list_head *lru, *tmp;
assert(!delta_base_cache_lock);
list_for_each_safe(lru, tmp, &delta_base_cache_lru) {
struct delta_base_cache_entry *entry =
list_entry(lru, struct delta_base_cache_entry, lru);
@@ -1493,6 +1497,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
struct delta_base_cache_entry *ent;
struct list_head *lru, *tmp;
assert(!delta_base_cache_lock);
/*
* Check required to avoid redundant entries when more than one thread
* is unpacking the same object, in unpack_entry() (since its phases I
@@ -1531,39 +1536,74 @@ int packed_object_info(struct repository *r, struct packed_git *p,
off_t obj_offset, struct object_info *oi)
{
struct pack_window *w_curs = NULL;
unsigned long size;
off_t curpos = obj_offset;
enum object_type type;
enum object_type type, final_type = OBJ_BAD;
struct delta_base_cache_entry *ent;
/*
* We always get the representation type, but only convert it to
* a "real" type later if the caller is interested.
*/
if (oi->contentp) {
*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,
&type);
oi->whence = OI_PACKED;
ent = get_delta_base_cache_entry(p, obj_offset);
if (ent) {
oi->whence = OI_DBCACHED;
final_type = type = ent->type;
if (oi->sizep)
*oi->sizep = ent->size;
if (oi->contentp) {
/* ignore content_limit if avoiding copy from cache */
if (oi->direct_cache) {
lock_delta_base_cache();
*oi->contentp = ent->data;
} else if (type != OBJ_BLOB || !oi->content_limit ||
ent->size <= oi->content_limit) {
*oi->contentp = xmemdupz(ent->data, ent->size);
} else {
*oi->contentp = NULL; /* caller must stream */
}
}
} else if (oi->contentp && !oi->content_limit) {
*oi->contentp = unpack_entry(r, p, obj_offset, &type,
oi->sizep);
final_type = type;
if (!*oi->contentp)
type = OBJ_BAD;
} else {
unsigned long size;
type = unpack_object_header(p, &w_curs, &curpos, &size);
}
if (!oi->contentp && oi->sizep) {
if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
off_t tmp_pos = curpos;
off_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,
type, obj_offset);
if (!base_offset) {
type = OBJ_BAD;
goto out;
if (oi->sizep) {
if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
off_t tmp_pos = curpos;
off_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,
type, obj_offset);
if (!base_offset) {
type = OBJ_BAD;
goto out;
}
*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);
if (*oi->sizep == 0) {
type = OBJ_BAD;
goto out;
}
} else {
*oi->sizep = size;
}
*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);
if (*oi->sizep == 0) {
type = OBJ_BAD;
goto out;
}
if (oi->contentp) {
final_type = packed_to_object_type(r, p, obj_offset,
type, &w_curs, curpos);
if (final_type != OBJ_BLOB || (oi->sizep &&
*oi->sizep <= oi->content_limit)) {
*oi->contentp = unpack_entry(r, p, obj_offset,
&type, oi->sizep);
if (!*oi->contentp)
type = OBJ_BAD;
} else {
*oi->contentp = NULL;
}
} else {
*oi->sizep = size;
}
}
@@ -1580,17 +1620,17 @@ int packed_object_info(struct repository *r, struct packed_git *p,
}
if (oi->typep || oi->type_name) {
enum object_type ptot;
ptot = packed_to_object_type(r, p, obj_offset,
type, &w_curs, curpos);
if (final_type < 0)
final_type = packed_to_object_type(r, p, obj_offset,
type, &w_curs, curpos);
if (oi->typep)
*oi->typep = ptot;
*oi->typep = final_type;
if (oi->type_name) {
const char *tn = type_name(ptot);
const char *tn = type_name(final_type);
if (tn)
strbuf_addstr(oi->type_name, tn);
}
if (ptot < 0) {
if (final_type < 0) {
type = OBJ_BAD;
goto out;
}
@@ -1607,10 +1647,6 @@ int packed_object_info(struct repository *r, struct packed_git *p,
} else
oidclr(oi->delta_base_oid, the_repository->hash_algo);
}
oi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :
OI_PACKED;
out:
unuse_pack(&w_curs);
return type;

View File

@@ -212,4 +212,8 @@ int is_promisor_object(const struct object_id *oid);
int load_idx(const char *path, const unsigned int hashsz, void *idx_map,
size_t idx_size, struct packed_git *p);
/*
* release lock acquired via oi->direct_cache
*/
void unlock_delta_base_cache(void);
#endif

View File

@@ -614,20 +614,33 @@ test_expect_success 'confirm that neither loose blob is a delta' '
test_cmp expect actual
'
test_expect_success 'setup delta base tests' '
foo="$(git rev-parse HEAD:foo)" &&
foo_plus="$(git rev-parse HEAD:foo-plus)" &&
git repack -ad
'
# To avoid relying too much on the current delta heuristics,
# we will check only that one of the two objects is a delta
# against the other, but not the order. We can do so by just
# asking for the base of both, and checking whether either
# oid appears in the output.
test_expect_success '%(deltabase) reports packed delta bases' '
git repack -ad &&
git cat-file --batch-check="%(deltabase)" <blobs >actual &&
{
grep "$(git rev-parse HEAD:foo)" actual ||
grep "$(git rev-parse HEAD:foo-plus)" actual
grep "$foo" actual || grep "$foo_plus" actual
}
'
test_expect_success 'delta base direct cache use succeeds w/o asserting' '
commands="info $foo
info $foo_plus
contents $foo_plus
contents $foo" &&
echo "$commands" >in &&
git cat-file --batch-command <in >out
'
test_expect_success 'setup bogus data' '
bogus_short_type="bogus" &&
bogus_short_content="bogus" &&

View File

@@ -262,6 +262,24 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
#ifdef HAVE_WRITEV
ssize_t xwritev(int fd, const struct iovec *iov, int iovcnt)
{
while (1) {
ssize_t nr = writev(fd, iov, iovcnt);
if (nr < 0) {
if (errno == EINTR)
continue;
if (handle_nonblock(fd, POLLOUT, errno))
continue;
}
return nr;
}
}
#endif /* !HAVE_WRITEV */
/*
* xpread() is the same as pread(), but it automatically restarts pread()
* operations with a recoverable error (EAGAIN and EINTR). xpread() DOES

View File

@@ -16,6 +16,7 @@ void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_
int xopen(const char *path, int flags, ...);
ssize_t xread(int fd, void *buf, size_t len);
ssize_t xwrite(int fd, const void *buf, size_t len);
ssize_t xwritev(int fd, const struct git_iovec *, int iovcnt);
ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
int xdup(int fd);
FILE *xfopen(const char *path, const char *mode);

View File

@@ -107,3 +107,69 @@ void fflush_or_die(FILE *f)
if (fflush(f))
die_errno("fflush error");
}
void fwritev_or_die(FILE *fp, const struct git_iovec *iov, int iovcnt)
{
int i;
for (i = 0; i < iovcnt; i++) {
size_t n = iov[i].iov_len;
if (fwrite(iov[i].iov_base, 1, n, fp) != n)
die_errno("unable to write to FD=%d", fileno(fp));
}
}
/*
* note: we don't care about atomicity from writev(2) right now.
* The goal is to avoid allocations+copies in the writer and
* reduce wakeups+syscalls in the reader.
* n.b. @iov is not const since we modify it to avoid allocating
* on partial write.
*/
#ifdef HAVE_WRITEV
void writev_or_die(int fd, struct git_iovec *iov, int iovcnt)
{
int i;
while (iovcnt > 0) {
ssize_t n = xwritev(fd, iov, iovcnt);
/* EINVAL happens when sum of iov_len exceeds SSIZE_MAX */
if (n < 0 && errno == EINVAL)
n = xwrite(fd, iov[0].iov_base, iov[0].iov_len);
if (n < 0) {
check_pipe(errno);
die_errno("writev error");
} else if (!n) {
errno = ENOSPC;
die_errno("writev_error");
}
/* skip fully written iovs, retry from the first partial iov */
for (i = 0; i < iovcnt; i++) {
if (n >= iov[i].iov_len) {
n -= iov[i].iov_len;
} else {
iov[i].iov_len -= n;
iov[i].iov_base = (char *)iov[i].iov_base + n;
break;
}
}
iovcnt -= i;
iov += i;
}
}
#else /* !HAVE_WRITEV */
/*
* n.b. don't use stdio fwrite here even if it's faster, @fd may be
* non-blocking and stdio isn't equipped for EAGAIN
*/
void writev_or_die(int fd, struct git_iovec *iov, int iovcnt)
{
int i;
for (i = 0; i < iovcnt; i++)
write_or_die(fd, iov[i].iov_base, iov[i].iov_len);
}
#endif /* !HAVE_WRITEV */

View File

@@ -7,6 +7,8 @@ void fprintf_or_die(FILE *, const char *fmt, ...);
void fwrite_or_die(FILE *f, const void *buf, size_t count);
void fflush_or_die(FILE *f);
void write_or_die(int fd, const void *buf, size_t count);
void writev_or_die(int fd, struct git_iovec *, int iovcnt);
void fwritev_or_die(FILE *, const struct git_iovec *, int iovcnt);
/*
* These values are used to help identify parts of a repository to fsync.