From 7c07385d902f6d8c177d533dc2faa36ef4a52a66 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 7 Jul 2013 06:03:29 -0400 Subject: [PATCH 01/10] zero-initialize object_info structs The sha1_object_info_extended function expects the caller to provide a "struct object_info" which contains pointers to "query" items that will be filled in. The purpose of providing pointers rather than storing the response directly in the struct is so that callers can choose not to incur the expense in finding particular fields that they do not care about. Right now the only query item is "sizep", and all callers set it explicitly to choose whether or not to query it; they can then leave the rest of the struct uninitialized. However, as we add new query items, each caller will have to be updated to explicitly turn off the new ones (by setting them to NULL). Instead, let's teach each caller to zero-initialize the struct, so that they do not have to learn about each new query item added. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_file.c | 2 +- streaming.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 67e815b2db..79ef052b11 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2409,7 +2409,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) { - struct object_info oi; + struct object_info oi = {0}; oi.sizep = sizep; return sha1_object_info_extended(sha1, &oi); diff --git a/streaming.c b/streaming.c index cabcd9d157..cac282f06b 100644 --- a/streaming.c +++ b/streaming.c @@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1, struct stream_filter *filter) { struct git_istream *st; - struct object_info oi; + struct object_info oi = {0}; const unsigned char *real = lookup_replace_object(sha1); enum input_source src = istream_source(real, type, &oi); From 161f00e708874bac646da2ac05c66a18ade2c074 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 7 Jul 2013 06:04:00 -0400 Subject: [PATCH 02/10] teach sha1_object_info_extended a "disk_size" query Using sha1_object_info_extended, a caller can find out the type of an object, its size, and information about where it is stored. In addition to the object's "true" size, it can also be useful to know the size that the object takes on disk (e.g., to generate statistics about which refs consume space). This patch adds a "disk_sizep" field to "struct object_info", and fills it in during sha1_object_info_extended if it is non-NULL. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 1 + sha1_file.c | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/cache.h b/cache.h index 94ca1acf70..f2915509a6 100644 --- a/cache.h +++ b/cache.h @@ -1099,6 +1099,7 @@ extern int unpack_object_header(struct packed_git *, struct pack_window **, off_ struct object_info { /* Request */ unsigned long *sizep; + unsigned long *disk_sizep; /* Response */ enum { diff --git a/sha1_file.c b/sha1_file.c index 79ef052b11..6baed676dc 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1694,7 +1694,8 @@ static int retry_bad_packed_offset(struct packed_git *p, off_t obj_offset) #define POI_STACK_PREALLOC 64 static int packed_object_info(struct packed_git *p, off_t obj_offset, - unsigned long *sizep, int *rtype) + unsigned long *sizep, int *rtype, + unsigned long *disk_sizep) { struct pack_window *w_curs = NULL; unsigned long size; @@ -1728,6 +1729,11 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset, } } + if (disk_sizep) { + struct revindex_entry *revidx = find_pack_revindex(p, obj_offset); + *disk_sizep = revidx[1].offset - obj_offset; + } + while (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) { off_t base_offset; /* Push the object we're going to leave behind */ @@ -2338,7 +2344,8 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1, } -static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *sizep) +static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *sizep, + unsigned long *disk_sizep) { int status; unsigned long mapsize, size; @@ -2349,6 +2356,8 @@ static int sha1_loose_object_info(const unsigned char *sha1, unsigned long *size map = map_sha1_file(sha1, &mapsize); if (!map) return error("unable to find %s", sha1_to_hex(sha1)); + if (disk_sizep) + *disk_sizep = mapsize; if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0) status = error("unable to unpack %s header", sha1_to_hex(sha1)); @@ -2372,13 +2381,15 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) if (co) { if (oi->sizep) *(oi->sizep) = co->size; + if (oi->disk_sizep) + *(oi->disk_sizep) = 0; oi->whence = OI_CACHED; return co->type; } if (!find_pack_entry(sha1, &e)) { /* Most likely it's a loose object. */ - status = sha1_loose_object_info(sha1, oi->sizep); + status = sha1_loose_object_info(sha1, oi->sizep, oi->disk_sizep); if (status >= 0) { oi->whence = OI_LOOSE; return status; @@ -2390,7 +2401,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi) return status; } - status = packed_object_info(e.p, e.offset, oi->sizep, &rtype); + status = packed_object_info(e.p, e.offset, oi->sizep, &rtype, + oi->disk_sizep); if (status < 0) { mark_bad_packed_object(e.p, sha1); status = sha1_object_info_extended(sha1, oi); From 03c893cbf9ad2e9a5be43382bcabdb2af2a49a4a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Jul 2013 07:36:43 -0400 Subject: [PATCH 03/10] t1006: modernize output comparisons In modern tests, we typically put output into a file and compare it with test_cmp. This is nicer than just comparing via "test", and much shorter than comparing via "test" and printing a custom message. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t1006-cat-file.sh | 61 +++++++++++++-------------------------------- 1 file changed, 18 insertions(+), 43 deletions(-) diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 9cc5c6bf4d..c2f25039e8 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -36,66 +36,41 @@ $content" ' test_expect_success "Type of $type is correct" ' - test $type = "$(git cat-file -t $sha1)" + echo $type >expect && + git cat-file -t $sha1 >actual && + test_cmp expect actual ' test_expect_success "Size of $type is correct" ' - test $size = "$(git cat-file -s $sha1)" + echo $size >expect && + git cat-file -s $sha1 >actual && + test_cmp expect actual ' test -z "$content" || test_expect_success "Content of $type is correct" ' - expect="$(maybe_remove_timestamp "$content" $no_ts)" - actual="$(maybe_remove_timestamp "$(git cat-file $type $sha1)" $no_ts)" - - if test "z$expect" = "z$actual" - then - : happy - else - echo "Oops: expected $expect" - echo "but got $actual" - false - fi + maybe_remove_timestamp "$content" $no_ts >expect && + maybe_remove_timestamp "$(git cat-file $type $sha1)" $no_ts >actual && + test_cmp expect actual ' test_expect_success "Pretty content of $type is correct" ' - expect="$(maybe_remove_timestamp "$pretty_content" $no_ts)" - actual="$(maybe_remove_timestamp "$(git cat-file -p $sha1)" $no_ts)" - if test "z$expect" = "z$actual" - then - : happy - else - echo "Oops: expected $expect" - echo "but got $actual" - false - fi + maybe_remove_timestamp "$pretty_content" $no_ts >expect && + maybe_remove_timestamp "$(git cat-file -p $sha1)" $no_ts >actual && + test_cmp expect actual ' test -z "$content" || test_expect_success "--batch output of $type is correct" ' - expect="$(maybe_remove_timestamp "$batch_output" $no_ts)" - actual="$(maybe_remove_timestamp "$(echo $sha1 | git cat-file --batch)" $no_ts)" - if test "z$expect" = "z$actual" - then - : happy - else - echo "Oops: expected $expect" - echo "but got $actual" - false - fi + maybe_remove_timestamp "$batch_output" $no_ts >expect && + maybe_remove_timestamp "$(echo $sha1 | git cat-file --batch)" $no_ts >actual && + test_cmp expect actual ' test_expect_success "--batch-check output of $type is correct" ' - expect="$sha1 $type $size" - actual="$(echo_without_newline $sha1 | git cat-file --batch-check)" - if test "z$expect" = "z$actual" - then - : happy - else - echo "Oops: expected $expect" - echo "but got $actual" - false - fi + echo "$sha1 $type $size" >expect && + echo_without_newline $sha1 | git cat-file --batch-check >actual && + test_cmp expect actual ' } From 98e2092b5027fde9dbe68cb49a196ad2184c02c3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Jul 2013 07:38:24 -0400 Subject: [PATCH 04/10] cat-file: teach --batch to stream blob objects The regular "git cat-file -p" and "git cat-file blob" code paths already learned to stream large blobs. Let's do the same here. Note that this means we look up the type and size before making a decision of whether to load the object into memory or stream (just like the "-p" code path does). That can lead to extra work, but it should be dwarfed by the cost of actually accessing the object itself. In my measurements, there was a 1-2% slowdown when using "--batch" on a large number of objects. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 045cee7bce..70dd8c8a10 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -117,12 +117,36 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) return 0; } +static void print_object_or_die(int fd, const unsigned char *sha1, + enum object_type type, unsigned long size) +{ + if (type == OBJ_BLOB) { + if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0) + die("unable to stream %s to stdout", sha1_to_hex(sha1)); + } + else { + enum object_type rtype; + unsigned long rsize; + void *contents; + + contents = read_sha1_file(sha1, &rtype, &rsize); + if (!contents) + die("object %s disappeared", sha1_to_hex(sha1)); + if (rtype != type) + die("object %s changed type!?", sha1_to_hex(sha1)); + if (rsize != size) + die("object %s change size!?", sha1_to_hex(sha1)); + + write_or_die(fd, contents, size); + free(contents); + } +} + static int batch_one_object(const char *obj_name, int print_contents) { unsigned char sha1[20]; enum object_type type = 0; unsigned long size; - void *contents = NULL; if (!obj_name) return 1; @@ -133,16 +157,10 @@ static int batch_one_object(const char *obj_name, int print_contents) return 0; } - if (print_contents == BATCH) - contents = read_sha1_file(sha1, &type, &size); - else - type = sha1_object_info(sha1, &size); - + type = sha1_object_info(sha1, &size); if (type <= 0) { printf("%s missing\n", obj_name); fflush(stdout); - if (print_contents == BATCH) - free(contents); return 0; } @@ -150,12 +168,9 @@ static int batch_one_object(const char *obj_name, int print_contents) fflush(stdout); if (print_contents == BATCH) { - write_or_die(1, contents, size); - printf("\n"); - fflush(stdout); - free(contents); + print_object_or_die(1, sha1, type, size); + write_or_die(1, "\n", 1); } - return 0; } From b71bd48017981f7257d8c0a5096825321b4fb49b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Jul 2013 07:38:58 -0400 Subject: [PATCH 05/10] cat-file: refactor --batch option parsing We currently use an int to tell us whether --batch parsing is on, and if so, whether we should print the full object contents. Let's instead factor this into a struct, filled in by callback, which will make further batch-related options easy to add. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 56 +++++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 70dd8c8a10..5254fe8956 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -13,9 +13,6 @@ #include "userdiff.h" #include "streaming.h" -#define BATCH 1 -#define BATCH_CHECK 2 - static int cat_one_file(int opt, const char *exp_type, const char *obj_name) { unsigned char sha1[20]; @@ -142,7 +139,12 @@ static void print_object_or_die(int fd, const unsigned char *sha1, } } -static int batch_one_object(const char *obj_name, int print_contents) +struct batch_options { + int enabled; + int print_contents; +}; + +static int batch_one_object(const char *obj_name, struct batch_options *opt) { unsigned char sha1[20]; enum object_type type = 0; @@ -167,19 +169,19 @@ static int batch_one_object(const char *obj_name, int print_contents) printf("%s %s %lu\n", sha1_to_hex(sha1), typename(type), size); fflush(stdout); - if (print_contents == BATCH) { + if (opt->print_contents) { print_object_or_die(1, sha1, type, size); write_or_die(1, "\n", 1); } return 0; } -static int batch_objects(int print_contents) +static int batch_objects(struct batch_options *opt) { struct strbuf buf = STRBUF_INIT; while (strbuf_getline(&buf, stdin, '\n') != EOF) { - int error = batch_one_object(buf.buf, print_contents); + int error = batch_one_object(buf.buf, opt); if (error) return error; } @@ -201,10 +203,28 @@ static int git_cat_file_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } +static int batch_option_callback(const struct option *opt, + const char *arg, + int unset) +{ + struct batch_options *bo = opt->value; + + if (unset) { + memset(bo, 0, sizeof(*bo)); + return 0; + } + + bo->enabled = 1; + bo->print_contents = !strcmp(opt->long_name, "batch"); + + return 0; +} + int cmd_cat_file(int argc, const char **argv, const char *prefix) { - int opt = 0, batch = 0; + int opt = 0; const char *exp_type = NULL, *obj_name = NULL; + struct batch_options batch = {0}; const struct option options[] = { OPT_GROUP(N_(" can be one of: blob, tree, commit, tag")), @@ -215,12 +235,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'), OPT_SET_INT(0, "textconv", &opt, N_("for blob objects, run textconv on object's content"), 'c'), - OPT_SET_INT(0, "batch", &batch, - N_("show info and content of objects fed from the standard input"), - BATCH), - OPT_SET_INT(0, "batch-check", &batch, - N_("show info about objects fed from the standard input"), - BATCH_CHECK), + { OPTION_CALLBACK, 0, "batch", &batch, NULL, + N_("show info and content of objects fed from the standard input"), + PARSE_OPT_NOARG, batch_option_callback }, + { OPTION_CALLBACK, 0, "batch-check", &batch, NULL, + N_("show info about objects fed from the standard input"), + PARSE_OPT_NOARG, batch_option_callback }, OPT_END() }; @@ -237,19 +257,19 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) else usage_with_options(cat_file_usage, options); } - if (!opt && !batch) { + if (!opt && !batch.enabled) { if (argc == 2) { exp_type = argv[0]; obj_name = argv[1]; } else usage_with_options(cat_file_usage, options); } - if (batch && (opt || argc)) { + if (batch.enabled && (opt || argc)) { usage_with_options(cat_file_usage, options); } - if (batch) - return batch_objects(batch); + if (batch.enabled) + return batch_objects(&batch); return cat_one_file(opt, exp_type, obj_name); } From 93d2a607ba05fba31442941d0425019120993846 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Jul 2013 07:45:47 -0400 Subject: [PATCH 06/10] cat-file: add --batch-check= The `cat-file --batch-check` command can be used to quickly get information about a large number of objects. However, it provides a fixed set of information. This patch adds an optional option to --batch-check to allow a caller to specify which items they are interested in, and in which order to output them. This is not very exciting for now, since we provide the same limited set that you could already get. However, it opens the door to adding new format items in the future without breaking backwards compatibility (or forcing callers to pay the cost to calculate uninteresting items). Since the --batch option shares code with --batch-check, it receives the same feature, though it is less likely to be of interest there. The format atom names are chosen to match their counterparts in for-each-ref. Though we do not (yet) share any code with for-each-ref's formatter, this keeps the interface as consistent as possible, and may help later on if the implementations are unified. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-cat-file.txt | 55 +++++++++++++---- builtin/cat-file.c | 107 ++++++++++++++++++++++++++++----- t/t1006-cat-file.sh | 6 ++ 3 files changed, 142 insertions(+), 26 deletions(-) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index 30d585af5d..7c57eca9b0 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -58,12 +58,16 @@ OPTIONS to apply the filter to the content recorded in the index at . --batch:: - Print the SHA-1, type, size, and contents of each object provided on - stdin. May not be combined with any other options or arguments. +--batch=:: + Print object information and contents for each object provided + on stdin. May not be combined with any other options or arguments. + See the section `BATCH OUTPUT` below for details. --batch-check:: - Print the SHA-1, type, and size of each object provided on stdin. May not - be combined with any other options or arguments. +--batch-check=:: + Print object information for each object provided on stdin. May + not be combined with any other options or arguments. See the + section `BATCH OUTPUT` below for details. OUTPUT ------ @@ -78,23 +82,52 @@ If '-p' is specified, the contents of are pretty-printed. If is specified, the raw (though uncompressed) contents of the will be returned. -If '--batch' is specified, output of the following form is printed for each -object specified on stdin: +BATCH OUTPUT +------------ + +If `--batch` or `--batch-check` is given, `cat-file` will read objects +from stdin, one per line, and print information about them. + +Each line is considered as a whole object name, and is parsed as if +given to linkgit:git-rev-parse[1]. + +You can specify the information shown for each object by using a custom +``. The `` is copied literally to stdout for each +object, with placeholders of the form `%(atom)` expanded, followed by a +newline. The available atoms are: + +`objectname`:: + The 40-hex object name of the object. + +`objecttype`:: + The type of of the object (the same as `cat-file -t` reports). + +`objectsize`:: + The size, in bytes, of the object (the same as `cat-file -s` + reports). + +If no format is specified, the default format is `%(objectname) +%(objecttype) %(objectsize)`. + +If `--batch` is specified, the object information is followed by the +object contents (consisting of `%(objectsize)` bytes), followed by a +newline. + +For example, `--batch` without a custom format would produce: ------------ SP SP LF LF ------------ -If '--batch-check' is specified, output of the following form is printed for -each object specified on stdin: +Whereas `--batch-check='%(objectname) %(objecttype)'` would produce: ------------ - SP SP LF + SP LF ------------ -For both '--batch' and '--batch-check', output of the following form is printed -for each object specified on stdin that does not exist in the repository: +If a name is specified on stdin that cannot be resolved to an object in +the repository, then `cat-file` will ignore any custom format and print: ------------ SP missing LF diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 5254fe8956..b43a0c5040 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -114,6 +114,66 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) return 0; } +struct expand_data { + unsigned char sha1[20]; + enum object_type type; + unsigned long size; + + /* + * If mark_query is true, we do not expand anything, but rather + * just mark the object_info with items we wish to query. + */ + int mark_query; + + /* + * After a mark_query run, this object_info is set up to be + * passed to sha1_object_info_extended. It will point to the data + * elements above, so you can retrieve the response from there. + */ + struct object_info info; +}; + +static int is_atom(const char *atom, const char *s, int slen) +{ + int alen = strlen(atom); + return alen == slen && !memcmp(atom, s, alen); +} + +static void expand_atom(struct strbuf *sb, const char *atom, int len, + void *vdata) +{ + struct expand_data *data = vdata; + + if (is_atom("objectname", atom, len)) { + if (!data->mark_query) + strbuf_addstr(sb, sha1_to_hex(data->sha1)); + } else if (is_atom("objecttype", atom, len)) { + if (!data->mark_query) + strbuf_addstr(sb, typename(data->type)); + } else if (is_atom("objectsize", atom, len)) { + if (data->mark_query) + data->info.sizep = &data->size; + else + strbuf_addf(sb, "%lu", data->size); + } else + die("unknown format element: %.*s", len, atom); +} + +static size_t expand_format(struct strbuf *sb, const char *start, void *data) +{ + const char *end; + + if (*start != '(') + return 0; + end = strchr(start + 1, ')'); + if (!end) + die("format element '%s' does not end in ')'", start); + + expand_atom(sb, start + 1, end - start - 1, data); + + return end - start + 1; +} + static void print_object_or_die(int fd, const unsigned char *sha1, enum object_type type, unsigned long size) { @@ -142,35 +202,37 @@ static void print_object_or_die(int fd, const unsigned char *sha1, struct batch_options { int enabled; int print_contents; + const char *format; }; -static int batch_one_object(const char *obj_name, struct batch_options *opt) +static int batch_one_object(const char *obj_name, struct batch_options *opt, + struct expand_data *data) { - unsigned char sha1[20]; - enum object_type type = 0; - unsigned long size; + struct strbuf buf = STRBUF_INIT; if (!obj_name) return 1; - if (get_sha1(obj_name, sha1)) { + if (get_sha1(obj_name, data->sha1)) { printf("%s missing\n", obj_name); fflush(stdout); return 0; } - type = sha1_object_info(sha1, &size); - if (type <= 0) { + data->type = sha1_object_info_extended(data->sha1, &data->info); + if (data->type <= 0) { printf("%s missing\n", obj_name); fflush(stdout); return 0; } - printf("%s %s %lu\n", sha1_to_hex(sha1), typename(type), size); - fflush(stdout); + strbuf_expand(&buf, opt->format, expand_format, data); + strbuf_addch(&buf, '\n'); + write_or_die(1, buf.buf, buf.len); + strbuf_release(&buf); if (opt->print_contents) { - print_object_or_die(1, sha1, type, size); + print_object_or_die(1, data->sha1, data->type, data->size); write_or_die(1, "\n", 1); } return 0; @@ -179,9 +241,23 @@ static int batch_one_object(const char *obj_name, struct batch_options *opt) static int batch_objects(struct batch_options *opt) { struct strbuf buf = STRBUF_INIT; + struct expand_data data; + + if (!opt->format) + opt->format = "%(objectname) %(objecttype) %(objectsize)"; + + /* + * Expand once with our special mark_query flag, which will prime the + * object_info to be handed to sha1_object_info_extended for each + * object. + */ + memset(&data, 0, sizeof(data)); + data.mark_query = 1; + strbuf_expand(&buf, opt->format, expand_format, &data); + data.mark_query = 0; while (strbuf_getline(&buf, stdin, '\n') != EOF) { - int error = batch_one_object(buf.buf, opt); + int error = batch_one_object(buf.buf, opt, &data); if (error) return error; } @@ -216,6 +292,7 @@ static int batch_option_callback(const struct option *opt, bo->enabled = 1; bo->print_contents = !strcmp(opt->long_name, "batch"); + bo->format = arg; return 0; } @@ -235,12 +312,12 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'), OPT_SET_INT(0, "textconv", &opt, N_("for blob objects, run textconv on object's content"), 'c'), - { OPTION_CALLBACK, 0, "batch", &batch, NULL, + { OPTION_CALLBACK, 0, "batch", &batch, "format", N_("show info and content of objects fed from the standard input"), - PARSE_OPT_NOARG, batch_option_callback }, - { OPTION_CALLBACK, 0, "batch-check", &batch, NULL, + PARSE_OPT_OPTARG, batch_option_callback }, + { OPTION_CALLBACK, 0, "batch-check", &batch, "format", N_("show info about objects fed from the standard input"), - PARSE_OPT_NOARG, batch_option_callback }, + PARSE_OPT_OPTARG, batch_option_callback }, OPT_END() }; diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index c2f25039e8..4e911fb43d 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -72,6 +72,12 @@ $content" echo_without_newline $sha1 | git cat-file --batch-check >actual && test_cmp expect actual ' + + test_expect_success "custom --batch-check format" ' + echo "$type $sha1" >expect && + echo $sha1 | git cat-file --batch-check="%(objecttype) %(objectname)" >actual && + test_cmp expect actual + ' } hello_content="Hello World" From a4ac1061783d25db4253309d2b58b9c2b89401d7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Jul 2013 07:46:25 -0400 Subject: [PATCH 07/10] cat-file: add %(objectsize:disk) format atom This atom is just like %(objectsize), except that it shows the on-disk size of the object rather than the object's true size. In other words, it makes the "disk_size" query of sha1_object_info_extended available via the command-line. This can be used for rough attribution of disk usage to particular refs, though see the caveats in the documentation. This patch does not include any tests, as the exact numbers returned are volatile and subject to zlib and packing decisions. We cannot even reliably guarantee that the on-disk size is smaller than the object content (though in general this should be the case for non-trivial objects). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-cat-file.txt | 18 ++++++++++++++++++ builtin/cat-file.c | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index 7c57eca9b0..10fbc6a373 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -106,6 +106,10 @@ newline. The available atoms are: The size, in bytes, of the object (the same as `cat-file -s` reports). +`objectsize:disk`:: + The size, in bytes, that the object takes up on disk. See the + note about on-disk sizes in the `CAVEATS` section below. + If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`. @@ -133,6 +137,20 @@ the repository, then `cat-file` will ignore any custom format and print: SP missing LF ------------ + +CAVEATS +------- + +Note that the sizes of objects on disk are reported accurately, but care +should be taken in drawing conclusions about which refs or objects are +responsible for disk usage. The size of a packed non-delta object may be +much larger than the size of objects which delta against it, but the +choice of which object is the base and which is the delta is arbitrary +and is subject to change during a repack. Note also that multiple copies +of an object may be present in the object database; in this case, it is +undefined which copy's size will be reported. + + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/cat-file.c b/builtin/cat-file.c index b43a0c5040..11fa8c08bc 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -118,6 +118,7 @@ struct expand_data { unsigned char sha1[20]; enum object_type type; unsigned long size; + unsigned long disk_size; /* * If mark_query is true, we do not expand anything, but rather @@ -155,6 +156,11 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, data->info.sizep = &data->size; else strbuf_addf(sb, "%lu", data->size); + } else if (is_atom("objectsize:disk", atom, len)) { + if (data->mark_query) + data->info.disk_sizep = &data->disk_size; + else + strbuf_addf(sb, "%lu", data->disk_size); } else die("unknown format element: %.*s", len, atom); } From c334b87b30c1464a1ab563fe1fb8de5eaf0e5bac Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Jul 2013 16:45:59 -0400 Subject: [PATCH 08/10] cat-file: split --batch input lines on whitespace If we get an input line to --batch or --batch-check that looks like "HEAD foo bar", we will currently feed the whole thing to get_sha1(). This means that to use --batch-check with `rev-list --objects`, one must pre-process the input, like: git rev-list --objects HEAD | cut -d' ' -f1 | git cat-file --batch-check Besides being more typing and slightly less efficient to invoke `cut`, the result loses information: we no longer know which path each object was found at. This patch teaches cat-file to split input lines at the first whitespace. Everything to the left of the whitespace is considered an object name, and everything to the right is made available as the %(reset) atom. So you can now do: git rev-list --objects HEAD | git cat-file --batch-check='%(objectsize) %(rest)' to collect object sizes at particular paths. Even if %(rest) is not used, we always do the whitespace split (which means you can simply eliminate the `cut` command from the first example above). This whitespace split is backwards compatible for any reasonable input. Object names cannot contain spaces, so any input with spaces would have resulted in a "missing" line. The only input hurt is if somebody really expected input of the form "HEAD is a fine-looking ref!" to fail; it will now parse HEAD, and make "is a fine-looking ref!" available as %(rest). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-cat-file.txt | 10 ++++++++-- builtin/cat-file.c | 20 +++++++++++++++++++- t/t1006-cat-file.sh | 7 +++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index 10fbc6a373..3ddec0b65b 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -88,8 +88,10 @@ BATCH OUTPUT If `--batch` or `--batch-check` is given, `cat-file` will read objects from stdin, one per line, and print information about them. -Each line is considered as a whole object name, and is parsed as if -given to linkgit:git-rev-parse[1]. +Each line is split at the first whitespace boundary. All characters +before that whitespace are considered as a whole object name, and are +parsed as if given to linkgit:git-rev-parse[1]. Characters after that +whitespace can be accessed using the `%(rest)` atom (see below). You can specify the information shown for each object by using a custom ``. The `` is copied literally to stdout for each @@ -110,6 +112,10 @@ newline. The available atoms are: The size, in bytes, that the object takes up on disk. See the note about on-disk sizes in the `CAVEATS` section below. +`rest`:: + The text (if any) found after the first run of whitespace on the + input line (i.e., the "rest" of the line). + If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`. diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 11fa8c08bc..0e64b4159c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -119,6 +119,7 @@ struct expand_data { enum object_type type; unsigned long size; unsigned long disk_size; + const char *rest; /* * If mark_query is true, we do not expand anything, but rather @@ -161,6 +162,9 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, data->info.disk_sizep = &data->disk_size; else strbuf_addf(sb, "%lu", data->disk_size); + } else if (is_atom("rest", atom, len)) { + if (!data->mark_query && data->rest) + strbuf_addstr(sb, data->rest); } else die("unknown format element: %.*s", len, atom); } @@ -263,7 +267,21 @@ static int batch_objects(struct batch_options *opt) data.mark_query = 0; while (strbuf_getline(&buf, stdin, '\n') != EOF) { - int error = batch_one_object(buf.buf, opt, &data); + char *p; + int error; + + /* + * Split at first whitespace, tying off the beginning of the + * string and saving the remainder (or NULL) in data.rest. + */ + p = strpbrk(buf.buf, " \t"); + if (p) { + while (*p && strchr(" \t", *p)) + *p++ = '\0'; + } + data.rest = p; + + error = batch_one_object(buf.buf, opt, &data); if (error) return error; } diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 4e911fb43d..d499d02a29 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -78,6 +78,13 @@ $content" echo $sha1 | git cat-file --batch-check="%(objecttype) %(objectname)" >actual && test_cmp expect actual ' + + test_expect_success '--batch-check with %(rest)' ' + echo "$type this is some extra content" >expect && + echo "$sha1 this is some extra content" | + git cat-file --batch-check="%(objecttype) %(rest)" >actual && + test_cmp expect actual + ' } hello_content="Hello World" From 012b32bb46459f96509669c2f5be0a93a95a2b43 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 10 Jul 2013 07:50:26 -0400 Subject: [PATCH 09/10] pack-revindex: use unsigned to store number of objects A packfile may have up to 2^32-1 objects in it, so the "right" data type to use is uint32_t. We currently use a signed int, which means that we may behave incorrectly for packfiles with more than 2^31-1 objects on 32-bit systems. Nobody has noticed because having 2^31 objects is pretty insane. The linux.git repo has on the order of 2^22 objects, which is hundreds of times smaller than necessary to trigger the bug. Let's bump this up to an "unsigned". On 32-bit systems, this gives us the correct data-type, and on 64-bit systems, it is probably more efficient to use the native "unsigned" than a true uint32_t. While we're at it, we can fix the binary search not to overflow in such a case if our unsigned is 32 bits. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pack-revindex.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pack-revindex.c b/pack-revindex.c index 77a0465be6..1aa9754384 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -72,8 +72,8 @@ static int cmp_offset(const void *a_, const void *b_) static void create_pack_revindex(struct pack_revindex *rix) { struct packed_git *p = rix->p; - int num_ent = p->num_objects; - int i; + unsigned num_ent = p->num_objects; + unsigned i; const char *index = p->index_data; rix->revindex = xmalloc(sizeof(*rix->revindex) * (num_ent + 1)); @@ -114,7 +114,7 @@ static void create_pack_revindex(struct pack_revindex *rix) struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs) { int num; - int lo, hi; + unsigned lo, hi; struct pack_revindex *rix; struct revindex_entry *revindex; @@ -132,7 +132,7 @@ struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs) lo = 0; hi = p->num_objects + 1; do { - int mi = (lo + hi) / 2; + unsigned mi = lo + (hi - lo) / 2; if (revindex[mi].offset == ofs) { return revindex + mi; } else if (ofs < revindex[mi].offset) From 8b8dfd5132ce91f632b5303c39cda2dfe30790f1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 11 Jul 2013 08:16:00 -0400 Subject: [PATCH 10/10] pack-revindex: radix-sort the revindex The pack revindex stores the offsets of the objects in the pack in sorted order, allowing us to easily find the on-disk size of each object. To compute it, we populate an array with the offsets from the sha1-sorted idx file, and then use qsort to order it by offsets. That does O(n log n) offset comparisons, and profiling shows that we spend most of our time in cmp_offset. However, since we are sorting on a simple off_t, we can use numeric sorts that perform better. A radix sort can run in O(k*n), where k is the number of "digits" in our number. For a 64-bit off_t, using 16-bit "digits" gives us k=4. On the linux.git repo, with about 3M objects to sort, this yields a 400% speedup. Here are the best-of-five numbers for running echo HEAD | git cat-file --batch-check="%(objectsize:disk) on a fully packed repository, which is dominated by time spent building the pack revindex: before after real 0m0.834s 0m0.204s user 0m0.788s 0m0.164s sys 0m0.040s 0m0.036s This matches our algorithmic expectations. log(3M) is ~21.5, so a traditional sort is ~21.5n. Our radix sort runs in k*n, where k is the number of radix digits. In the worst case, this is k=4 for a 64-bit off_t, but we can quit early when the largest value to be sorted is smaller. For any repository under 4G, k=2. Our algorithm makes two passes over the list per radix digit, so we end up with 4n. That should yield ~5.3x speedup. We see 4x here; the difference is probably due to the extra bucket book-keeping the radix sort has to do. On a smaller repo, the difference is less impressive, as log(n) is smaller. For git.git, with 173K objects (but still k=2), we see a 2.7x improvement: before after real 0m0.046s 0m0.017s user 0m0.036s 0m0.012s sys 0m0.008s 0m0.000s On even tinier repos (e.g., a few hundred objects), the speedup goes away entirely, as the small advantage of the radix sort gets erased by the book-keeping costs (and at those sizes, the cost to generate the the rev-index gets lost in the noise anyway). Signed-off-by: Jeff King Reviewed-by: Brandon Casey Signed-off-by: Junio C Hamano --- pack-revindex.c | 100 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 95 insertions(+), 5 deletions(-) diff --git a/pack-revindex.c b/pack-revindex.c index 1aa9754384..b4d2b35bb3 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -59,11 +59,101 @@ static void init_pack_revindex(void) /* revindex elements are lazily initialized */ } -static int cmp_offset(const void *a_, const void *b_) +/* + * This is a least-significant-digit radix sort. + * + * It sorts each of the "n" items in "entries" by its offset field. The "max" + * parameter must be at least as large as the largest offset in the array, + * and lets us quit the sort early. + */ +static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max) { - const struct revindex_entry *a = a_; - const struct revindex_entry *b = b_; - return (a->offset < b->offset) ? -1 : (a->offset > b->offset) ? 1 : 0; + /* + * We use a "digit" size of 16 bits. That keeps our memory + * usage reasonable, and we can generally (for a 4G or smaller + * packfile) quit after two rounds of radix-sorting. + */ +#define DIGIT_SIZE (16) +#define BUCKETS (1 << DIGIT_SIZE) + /* + * We want to know the bucket that a[i] will go into when we are using + * the digit that is N bits from the (least significant) end. + */ +#define BUCKET_FOR(a, i, bits) (((a)[(i)].offset >> (bits)) & (BUCKETS-1)) + + /* + * We need O(n) temporary storage. Rather than do an extra copy of the + * partial results into "entries", we sort back and forth between the + * real array and temporary storage. In each iteration of the loop, we + * keep track of them with alias pointers, always sorting from "from" + * to "to". + */ + struct revindex_entry *tmp = xmalloc(n * sizeof(*tmp)); + struct revindex_entry *from = entries, *to = tmp; + int bits; + unsigned *pos = xmalloc(BUCKETS * sizeof(*pos)); + + /* + * If (max >> bits) is zero, then we know that the radix digit we are + * on (and any higher) will be zero for all entries, and our loop will + * be a no-op, as everybody lands in the same zero-th bucket. + */ + for (bits = 0; max >> bits; bits += DIGIT_SIZE) { + struct revindex_entry *swap; + unsigned i; + + memset(pos, 0, BUCKETS * sizeof(*pos)); + + /* + * We want pos[i] to store the index of the last element that + * will go in bucket "i" (actually one past the last element). + * To do this, we first count the items that will go in each + * bucket, which gives us a relative offset from the last + * bucket. We can then cumulatively add the index from the + * previous bucket to get the true index. + */ + for (i = 0; i < n; i++) + pos[BUCKET_FOR(from, i, bits)]++; + for (i = 1; i < BUCKETS; i++) + pos[i] += pos[i-1]; + + /* + * Now we can drop the elements into their correct buckets (in + * our temporary array). We iterate the pos counter backwards + * to avoid using an extra index to count up. And since we are + * going backwards there, we must also go backwards through the + * array itself, to keep the sort stable. + * + * Note that we use an unsigned iterator to make sure we can + * handle 2^32-1 objects, even on a 32-bit system. But this + * means we cannot use the more obvious "i >= 0" loop condition + * for counting backwards, and must instead check for + * wrap-around with UINT_MAX. + */ + for (i = n - 1; i != UINT_MAX; i--) + to[--pos[BUCKET_FOR(from, i, bits)]] = from[i]; + + /* + * Now "to" contains the most sorted list, so we swap "from" and + * "to" for the next iteration. + */ + swap = from; + from = to; + to = swap; + } + + /* + * If we ended with our data in the original array, great. If not, + * we have to move it back from the temporary storage. + */ + if (from != entries) + memcpy(entries, tmp, n * sizeof(*entries)); + free(tmp); + free(pos); + +#undef BUCKET_FOR +#undef BUCKETS +#undef DIGIT_SIZE } /* @@ -108,7 +198,7 @@ static void create_pack_revindex(struct pack_revindex *rix) */ rix->revindex[num_ent].offset = p->pack_size - 20; rix->revindex[num_ent].nr = -1; - qsort(rix->revindex, num_ent, sizeof(*rix->revindex), cmp_offset); + sort_revindex(rix->revindex, num_ent, p->pack_size); } struct revindex_entry *find_pack_revindex(struct packed_git *p, off_t ofs)