From: Charles Bailey <hidden> Date: 2016-06-15 23:05:23
In my team we've been looking for a fast way to check a large number of
repositories for large files, which are typically unintentionally checked in
binaries, so that we can warn repository owners and help them tidy up as
desired.
There seem to be two main approaches to scripting this. The first is to do
something revision-walk based such as `log --numstat` and the second is to scan
pack files using `verify-pack -v` and either to ensure that everything is packed
or scan loose objects separately.
The revision walking tends to be slow and parsing verify-pack -v is awkward
not only because of the need to take account of multiple packs and loose
objects, but also because it is porcelainish. For example, at some point it
gained a delta chain summary which needs to be snipped before the list of
packed objects can be sorted and used.
The third patch in this series adds a new built in which makes this simple and
fast. While implementing it, I found a couple of other improvements which I
think stand alone.
[PATCH 1/3] Correct test-parse-options to handle negative ints
I noticed that a printf in test-parse-options was using %u instead of %d for an
int with the consequence that it wouldn't ever print a negative value correctly.
I don't know that we do ever parse a negative integer as an option, but there's
no reason that it shouldn't work so I fixed it and added a trivial test.
[PATCH 2/3] Move unsigned long option parsing out of pack-objects.c
I wanted to be able to parse options like --min-size=500k in my new command so I
started to add OPT_ULONG, only to realise that it already existed but was
private to pack-objects. I added OPT_ULONG support to parse-options based on the
existing OPT_INTEGER code, added new tests and changed pack-objects to use this
instead.
@@ -2588,23 +2588,6 @@ static int option_parse_unpack_unreachable(const struct option *opt,return0;}-staticintoption_parse_ulong(conststructoption*opt,-constchar*arg,intunset)-{-if(unset)-die(_("option %s does not accept negative form"),-opt->long_name);--if(!git_parse_ulong(arg,opt->value))-die(_("unable to parse value '%s' for option %s"),-arg,opt->long_name);-return0;-}--#define OPT_ULONG(s, l, v, h) \-{OPTION_CALLBACK,(s),(l),(v),"n",(h),\-PARSE_OPT_NONEG,option_parse_ulong}-intcmd_pack_objects(intargc,constchar**argv,constchar*prefix){intuse_internal_rev_list=0;
@@ -180,6 +180,21 @@ static int get_value(struct parse_opt_ctx_t *p,returnopterror(opt,"expects a numerical value",flags);return0;+caseOPTION_ULONG:+if(unset){+*(unsignedlong*)opt->value=0;+return0;+}+if(opt->flags&PARSE_OPT_OPTARG&&!p->opt){+*(unsignedlong*)opt->value=opt->defval;+return0;+}+if(get_arg(p,opt,flags,&arg))+return-1;+if(!git_parse_ulong(arg,opt->value))+returnopterror(opt,"expects a numerical value",flags);+return0;+default:die("should not happen, someone must be hit on the forehead");}
@@ -48,6 +49,7 @@ int main(int argc, char **argv)OPT_GROUP(""),OPT_INTEGER('i',"integer",&integer,"get a integer"),OPT_INTEGER('j',NULL,&integer,"get a integer, too"),+OPT_ULONG('u',"unsigned-long",&unsigned_long,"get an unsigned long"),OPT_SET_INT(0,"set23",&integer,"set integer to 23",23),OPT_DATE('t',NULL,×tamp,"get timestamp of <time>"),OPT_CALLBACK('L',"length",&integer,"str",
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:23
From: Charles Bailey <redacted>
filter-objects is a command to scan all objects in the object database
for the repository and print the ids of those which match the given
criteria.
The current supported criteria are object type and the minimum size of
the object.
The guiding use case is to scan repositories quickly for large objects
which may cause performance issues for users. The list of objects can
then be used to guide some future remediating action.
Signed-off-by: Charles Bailey <redacted>
---
Documentation/git-filter-objects.txt | 38 +++++++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/filter-objects.c | 73 ++++++++++++++++++++++++++++++++++++
git.c | 1 +
t/t8100-filter-objects.sh | 67 +++++++++++++++++++++++++++++++++
6 files changed, 181 insertions(+)
create mode 100644 Documentation/git-filter-objects.txt
create mode 100644 builtin/filter-objects.c
create mode 100755 t/t8100-filter-objects.sh
@@ -0,0 +1,38 @@+git-filter-objects(1)+=====================++NAME+----+git-filter-objects - Scan through all objects in the repository and print those+matching a given filter+++SYNOPSIS+--------+[verse]+'git filter-objects' [-t <type> | --type=<type>] [--min-size=<size>]+ [-v|--verbose]++DESCRIPTION+-----------+Scans all objects in a repository - including any unreachable objects - and+print out the ids of all matching objects. If `--verbose` is specified then+the object type and size is printed out as well as its id.++OPTIONS+-------+-t::+--type::+ Only list objects whose type matches <type>.++--min-size::+ Only list objects whose size exceeds <size> bytes.++-v::+--verbose::+ Output in the followin format instead of just printing object ids:+ <sha1> SP <type> SP <size>++GIT+---+Part of the linkgit:git[1] suite
@@ -0,0 +1,73 @@+#include"cache.h"+#include"builtin.h"+#include"revision.h"+#include"parse-options.h"++#include<stdio.h>++staticintreq_type;+staticunsignedlongmin_size;+staticintverbose;++staticintcheck_object(constunsignedchar*sha1)+{+unsignedlongsize;+inttype=sha1_object_info(sha1,&size);++if(type<0)+return-1;++if(size>=min_size&&(!req_type||type==req_type)){+if(verbose)+printf("%s %s %lu\n",sha1_to_hex(sha1),typename(type),size);+else+printf("%s\n",sha1_to_hex(sha1));+}++return0;+}++staticintcheck_loose_object(constunsignedchar*sha1,+constchar*path,+void*data)+{+returncheck_object(sha1);+}++staticintcheck_packed_object(constunsignedchar*sha1,+structpacked_git*pack,+uint32_tpos,+void*data)+{+returncheck_object(sha1);+}++staticchar*opt_type;+staticstructoptionbuiltin_filter_objects_options[]={+OPT_ULONG(0,"min-size",&min_size,"minimum size of object to show"),+OPT_STRING('t',"type",&opt_type,NULL,"type of objects to show"),+OPT__VERBOSE(&verbose,"show object type and size"),+OPT_END()+};++intcmd_filter_objects(intargc,constchar**argv,constchar*prefix)+{+structpacked_git*p;++argc=parse_options(argc,argv,prefix,builtin_filter_objects_options,+NULL,0);++if(opt_type)+req_type=type_from_string(opt_type);++for_each_loose_object(check_loose_object,NULL,0);++prepare_packed_git();+for(p=packed_git;p;p=p->next){+open_pack_index(p);+}++for_each_packed_object(check_packed_object,NULL,0);++return0;+}
@@ -0,0 +1,67 @@+#!/bin/sh++test_description='git filter-objects'+../test-lib.sh++test_expect_success'setup''+echohello,world>file&&+gitaddfile&&+gitcommit-m"initial"+'++test_expect_success'filter by type''+gitrev-parseHEAD>expected&&+gitfilter-objects-tcommit>result&&+test_cmpexpectedresult&&+gitrev-parseHEAD:file>expected&&+gitfilter-objects-tblob>result&&+test_cmpexpectedresult&&+gitrev-parseHEAD^{tree}>expected&&+gitfilter-objects-ttree>result&&+test_cmpexpectedresult+'++test_expect_success'filter by type after pack''+gitrepack-Ad&&+gitrev-parseHEAD>expected&&+gitfilter-objects-tcommit>result&&+test_cmpexpectedresult&&+gitrev-parseHEAD:file>expected&&+gitfilter-objects-tblob>result&&+test_cmpexpectedresult&&+gitrev-parseHEAD^{tree}>expected&&+gitfilter-objects-ttree>result&&+test_cmpexpectedresult+'++test_expect_success'verbose output''+echo$(gitrev-parseHEAD)commit$(gitcat-file-sHEAD)>expected&&+gitfilter-objects-v-tcommit>result&&+test_cmpexpectedresult&&+echo$(gitrev-parseHEAD:file)blob$(gitcat-file-sHEAD:file)>expected&&+gitfilter-objects-v-tblob>result&&+test_cmpexpectedresult&&+echo$(gitrev-parseHEAD^{tree})tree$(gitcat-file-sHEAD^{tree})>expected&&+gitfilter-objects-v-ttree>result&&+test_cmpexpectedresult+'++test_expect_success'filter on size''+gitcommit-F---allow-empty<<-\EOF&&+Thisisareasonablylongcommitmessage++Itisdesignedtomakesurethatwecreateanobject+thatissubstantiallylargerthanalltheothers.++Ourtestfileblobisafewbytes,ourtreeissimilarly+smallandourfirstcommitisnottoobig.++Thismessagealoneisabout300charactersandasample+commitfromithasbeenmeasuredat562bytes.+EOF+gitrev-parseHEAD>expected&&+gitfilter-objects--min-size=500>result&&+test_cmpexpectedresult+'++test_done
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:23
From: Charles Bailey <redacted>
Fix the printf specification to treat 'integer' as the signed type that
it is and add a test that checks that we parse negative option
arguments.
Signed-off-by: Charles Bailey <redacted>
---
t/t0040-parse-options.sh | 2 ++
test-parse-options.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
From: Jeff King <hidden> Date: 2016-06-15 23:05:24
On Fri, Jun 19, 2015 at 10:10:59AM +0100, Charles Bailey wrote:
filter-objects is a command to scan all objects in the object database
for the repository and print the ids of those which match the given
criteria.
The current supported criteria are object type and the minimum size of
the object.
The guiding use case is to scan repositories quickly for large objects
which may cause performance issues for users. The list of objects can
then be used to guide some future remediating action.
I've had to perform this exact same task. You can already do the
"filtering" part pretty easily and efficiently with cat-file and a perl
script, like:
magically_generate_all_objects |
git cat-file --batch-check='%(objectsize) %(objectname)' |
perl -alne 'print $F[1] if $F[0] > 1234'
That's not as friendly as your filter-objects, but it's a lot more
flexible (since you can ask cat-file for all sorts of information).
Obviously I've glossed over the "how to get a list of objects" part.
If you truly want all objects (not just reachable ones), or if "rev-list
--objects" is too slow, the best way is:
objects() {
# loose objects
for i in objects/??/*; do
echo $i
done |
sed 's,objects/\(..\)/,\1,'
# packed objects
for i in objects/pack/*.idx; do
git show-index <$i
done |
cut -d' ' -f2
}
Certainly I'm not opposed to doing something less horrible there (and I
am happy to see my for_each_*_object interface getting more callers!).
I kind of wonder if we should make "all objects, reachable or not" an
option for rev-list. I'm not sure if it would choke on adding them all
to the "pending" list, though; it's not really made for that. But it
would enable neat things like:
git rev-list --all-the-objects --not --all
to show you what's unreachable.
-Peff
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:24
On Fri, Jun 19, 2015 at 06:10:10AM -0400, Jeff King wrote:
On Fri, Jun 19, 2015 at 10:10:59AM +0100, Charles Bailey wrote:
quoted
filter-objects is a command to scan all objects in the object database
for the repository and print the ids of those which match the given
criteria.
The current supported criteria are object type and the minimum size of
the object.
The guiding use case is to scan repositories quickly for large objects
which may cause performance issues for users. The list of objects can
then be used to guide some future remediating action.
I've had to perform this exact same task. You can already do the
"filtering" part pretty easily and efficiently with cat-file and a perl
script, like:
magically_generate_all_objects |
git cat-file --batch-check='%(objectsize) %(objectname)' |
perl -alne 'print $F[1] if $F[0] > 1234'
That's not as friendly as your filter-objects, but it's a lot more
flexible (since you can ask cat-file for all sorts of information).
Obviously I've glossed over the "how to get a list of objects" part.
If you truly want all objects (not just reachable ones), or if "rev-list
--objects" is too slow [...]
So, yes, performance is definitely an issue and I could have called this
command "git magically-generate-all-object-for-scripts" but then, as it
was so easy to provide exactly the filtering that I was looking for in
the C code, I thought I would do that as well and then "filter-objects"
("filter-all-objects"?) seemed like a better name.
It's about an order of magnitude faster on the systems I've checked to
do a parameterless filter-objects then rev-list --all --objects,
although I understand they do different things.
I am also thinking about another piece that answers the question: "which
commits introduce any of (or the first of) this list of objects?". This
can be done by parseing a diff --raw for commits but I think it should
be possible to do this faster, too.
Charles.
From: Jeff King <hidden> Date: 2016-06-15 23:05:24
On Fri, Jun 19, 2015 at 11:33:24AM +0100, Charles Bailey wrote:
quoted
Obviously I've glossed over the "how to get a list of objects" part.
If you truly want all objects (not just reachable ones), or if "rev-list
--objects" is too slow [...]
So, yes, performance is definitely an issue and I could have called this
command "git magically-generate-all-object-for-scripts" but then, as it
was so easy to provide exactly the filtering that I was looking for in
the C code, I thought I would do that as well and then "filter-objects"
("filter-all-objects"?) seemed like a better name.
Right, my point was only that it works for _your_ particular filter, but
it would be nice to have something more general. And we already have
"cat-file --batch-check". IOW, I think I would prefer the "magical" form
because it's a better scripting building block. As you note,
"filter-objects" without any filters is exactly that. Your 10 extra
lines of C code are not exactly bloat, but I just wonder if other people
will find it all that useful.
It's about an order of magnitude faster on the systems I've checked to
do a parameterless filter-objects then rev-list --all --objects,
although I understand they do different things.
Right, it's the object-opening and hash lookups that kill you in
"rev-list", because it's actually walking the graph.
I am also thinking about another piece that answers the question: "which
commits introduce any of (or the first of) this list of objects?". This
can be done by parseing a diff --raw for commits but I think it should
be possible to do this faster, too.
If you care about "introduce", I think you have to traverse and do the
diffs. If you only care about "contains" (for example, because you want
to know which path the blob is found at), you can find trees which
mention it, then trees which mention that tree, and so on. I think that
ends up slower in practice, though.
I have patches that implement a "rev-list --find=$sha1", which sets a
bit on $sha1 and then traverses with --objects until we find it (or
them; you can specify multiple). It's pretty straightforward, but it
does cost as much as "git rev-list --objects" in the worst case. Let me
know if you're interested and I can clean it up and post it.
-Peff
From: John Keeping <hidden> Date: 2016-06-15 23:05:24
On Fri, Jun 19, 2015 at 11:33:24AM +0100, Charles Bailey wrote:
So, yes, performance is definitely an issue and I could have called this
command "git magically-generate-all-object-for-scripts" but then, as it
was so easy to provide exactly the filtering that I was looking for in
the C code, I thought I would do that as well and then "filter-objects"
("filter-all-objects"?) seemed like a better name.
By analogy with "git filter-branch", I don't think "filter-objects" is a
good name here. My preference would be "ls-objects".
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:24
On Fri, Jun 19, 2015 at 11:52:28AM +0100, John Keeping wrote:
On Fri, Jun 19, 2015 at 11:33:24AM +0100, Charles Bailey wrote:
quoted
So, yes, performance is definitely an issue and I could have called this
command "git magically-generate-all-object-for-scripts" but then, as it
was so easy to provide exactly the filtering that I was looking for in
the C code, I thought I would do that as well and then "filter-objects"
("filter-all-objects"?) seemed like a better name.
By analogy with "git filter-branch", I don't think "filter-objects" is a
good name here. My preference would be "ls-objects".
I like that because it emphasises why I wrote it, the very basic
filtering is a nice additional feature.
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:25
This is a re-roll of the first two patches in my previous series which used to
include "filter-objects" which is now a separate topic.
[PATCH 1/2] Correct test-parse-options to handle negative ints
The first one has changed only in that I've moved the additional test to a more
logical place in the test file.
[PATCH 2/2] Move unsigned long option parsing out of pack-objects.c
I've made the following changes to the second commit:
- renamed this OPT_MAGNITUDE to try and convey something that is
both unsigned and might benefit from a 'scale' suffix. I'm expecting
more discussion on the name!
- fixed the enum ordering to put this close to OPT_INTEGER
- added documentation to api-parse-options.txt
- marginally improved the opterror message on failed parses
- noted the change in behavior for the error messages generated for
pack-objects' --max-pack-size and --window-memory in the commit message
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:25
From: Charles Bailey <redacted>
Fix the printf specification to treat 'integer' as the signed type that
it is and add a test that checks that we parse negative option
arguments.
Signed-off-by: Charles Bailey <redacted>
---
t/t0040-parse-options.sh | 2 ++
test-parse-options.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:25
From: Charles Bailey <redacted>
The unsigned long option parsing (including 'k'/'m'/'g' suffix parsing)
is more widely applicable. Add support for OPT_MAGNITUDE to
parse-options.h and change pack-objects.c use this support.
The error behavior on parse errors follows that of OPT_INTEGER.
The name of the option that failed to parse is reported with a brief
message describing the expect format for the option argument and then
the full usage message for the command invoked.
This is differs from the previous behavior for OPT_ULONG used in
pack-objects for --max-pack-size and --window-memory which used to
display the value supplied in the error message and did not display the
full usage message.
Signed-off-by: Charles Bailey <redacted>
---
Documentation/technical/api-parse-options.txt | 6 ++++
builtin/pack-objects.c | 25 +++------------
parse-options.c | 17 ++++++++++
parse-options.h | 3 ++
t/t0040-parse-options.sh | 45 ++++++++++++++++++++++++---
test-parse-options.c | 3 ++
6 files changed, 73 insertions(+), 26 deletions(-)
@@ -168,6 +168,12 @@ There are some macros to easily define options: Introduce an option with integer argument. The integer is put into `int_var`.+`OPT_MAGNITUDE(short, long, &unsigned_long_var, description)`::+ Introduce an option with a size argument. The argument must be a+ non-negative integer and may include a suffix of 'k', 'm' or 'g' to+ scale the provided value by 1024, 1024^2 or 1024^3 respectively.+ The scaled value is put into `unsigned_long_var`.+ `OPT_DATE(short, long, &int_var, description)`:: Introduce an option with date argument, see `approxidate()`. The timestamp is put into `int_var`.
@@ -2588,23 +2588,6 @@ static int option_parse_unpack_unreachable(const struct option *opt,return0;}-staticintoption_parse_ulong(conststructoption*opt,-constchar*arg,intunset)-{-if(unset)-die(_("option %s does not accept negative form"),-opt->long_name);--if(!git_parse_ulong(arg,opt->value))-die(_("unable to parse value '%s' for option %s"),-arg,opt->long_name);-return0;-}--#define OPT_ULONG(s, l, v, h) \-{OPTION_CALLBACK,(s),(l),(v),"n",(h),\-PARSE_OPT_NONEG,option_parse_ulong}-intcmd_pack_objects(intargc,constchar**argv,constchar*prefix){intuse_internal_rev_list=0;
@@ -2627,16 +2610,16 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix){OPTION_CALLBACK,0,"index-version",NULL,N_("version[,offset]"),N_("write the pack index file in the specified idx format version"),0,option_parse_index_version},-OPT_ULONG(0,"max-pack-size",&pack_size_limit,-N_("maximum size of each output pack file")),+OPT_MAGNITUDE(0,"max-pack-size",&pack_size_limit,+N_("maximum size of each output pack file")),OPT_BOOL(0,"local",&local,N_("ignore borrowed objects from alternate object store")),OPT_BOOL(0,"incremental",&incremental,N_("ignore packed objects")),OPT_INTEGER(0,"window",&window,N_("limit pack window by objects")),-OPT_ULONG(0,"window-memory",&window_memory_limit,-N_("limit pack window by memory in addition to object limit")),+OPT_MAGNITUDE(0,"window-memory",&window_memory_limit,+N_("limit pack window by memory in addition to object limit")),OPT_INTEGER(0,"depth",&depth,N_("maximum length of delta chain allowed in the resulting pack")),OPT_BOOL(0,"reuse-delta",&reuse_delta,
@@ -180,6 +180,23 @@ static int get_value(struct parse_opt_ctx_t *p,returnopterror(opt,"expects a numerical value",flags);return0;+caseOPTION_MAGNITUDE:+if(unset){+*(unsignedlong*)opt->value=0;+return0;+}+if(opt->flags&PARSE_OPT_OPTARG&&!p->opt){+*(unsignedlong*)opt->value=opt->defval;+return0;+}+if(get_arg(p,opt,flags,&arg))+return-1;+if(!git_parse_ulong(arg,opt->value))+returnopterror(opt,+"expects a integer value with an optional k/m/g suffix",+flags);+return0;+default:die("should not happen, someone must be hit on the forehead");}
@@ -48,6 +49,7 @@ int main(int argc, char **argv)OPT_GROUP(""),OPT_INTEGER('i',"integer",&integer,"get a integer"),OPT_INTEGER('j',NULL,&integer,"get a integer, too"),+OPT_MAGNITUDE('m',"magnitude",&magnitude,"get a magnitude"),OPT_SET_INT(0,"set23",&integer,"set integer to 23",23),OPT_DATE('t',NULL,×tamp,"get timestamp of <time>"),OPT_CALLBACK('L',"length",&integer,"str",
@@ -180,6 +180,23 @@ static int get_value(struct parse_opt_ctx_t *p,returnopterror(opt,"expects a numerical value",flags);return0;+caseOPTION_MAGNITUDE:+if(unset){+*(unsignedlong*)opt->value=0;+return0;+}+if(opt->flags&PARSE_OPT_OPTARG&&!p->opt){+*(unsignedlong*)opt->value=opt->defval;+return0;+}+if(get_arg(p,opt,flags,&arg))+return-1;+if(!git_parse_ulong(arg,opt->value))+returnopterror(opt,+"expects a integer value with an optional k/m/g suffix",+flags);+return0;+
Spotted after sending:
s/expects a integer/expects an integer/
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:25
This is a re-casting of my previous filter-objects command but without
any of the filtering so it is now just "list-all-objects".
I have retained the "--verbose" option which outputs the same format as
the default "cat-file --batch-check" as it provides a useful performance
gain to filtering though "cat-file" if this basic information is all
that is needed.
The motivating use case is to enable a script to quickly scan a large
number of repositories for any large objects.
I performed some test timings of some different commands on a clone of
the Linux kernel which was completely packed.
$ time git rev-list --all --objects |
cut -d" " -f1 |
git cat-file --batch-check |
awk '{if ($3 >= 512000) { print $1 }}' |
wc -l
958
real 0m30.823s
user 0m41.904s
sys 0m7.728s
list-all-objects gives a significant improvement:
$ time git list-all-objects |
git cat-file --batch-check |
awk '{if ($3 >= 512000) { print $1 }}' |
wc -l
958
real 0m9.585s
user 0m10.820s
sys 0m4.960s
skipping the cat-filter filter is a lesser but still significant
improvement:
$ time git list-all-objects -v |
awk '{if ($3 >= 512000) { print $1 }}' |
wc -l
958
real 0m5.637s
user 0m6.652s
sys 0m0.156s
The old filter-objects could do the size filter a little be faster, but
not by much:
$ time git filter-objects --min-size=500k |
wc -l
958
real 0m4.564s
user 0m4.496s
sys 0m0.064s
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:25
From: Charles Bailey <redacted>
list-all-objects is a command to print the ids of all objects in the
object database of a repository. It is designed as a low overhead
interface for scripts that want to analyse all objects but don't require
the ordering implied by a revision walk.
It will list all objects, loose and packed, and will include unreachable
objects.
list-all-objects is faster that "rev-list --all --objects" but there is
no guarantee as to the order in which objects will be listed.
Signed-off-by: Charles Bailey <redacted>
---
Documentation/git-list-all-objects.txt | 29 +++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/list-all-objects.c | 64 ++++++++++++++++++++++++++++++++++
git.c | 1 +
t/t8100-list-all-objects.sh | 45 ++++++++++++++++++++++++
6 files changed, 141 insertions(+)
create mode 100644 Documentation/git-list-all-objects.txt
create mode 100644 builtin/list-all-objects.c
create mode 100755 t/t8100-list-all-objects.sh
@@ -0,0 +1,29 @@+git-list-all-objects(1)+=======================++NAME+----+git-list-all-objects - List all objects in the repository.++SYNOPSIS+--------+[verse]+'git list-all-objects' [-v|--verbose]++DESCRIPTION+-----------+List the ids of all objects in a repository, including any unreachable objects.+If `--verbose` is specified then the object's type and size is printed out as+well as its id.++OPTIONS+-------++-v::+--verbose::+ Output in the followin format instead of just printing object ids:+ <sha1> SP <type> SP <size>++GIT+---+Part of the linkgit:git[1] suite
@@ -0,0 +1,64 @@+#include"cache.h"+#include"builtin.h"+#include"revision.h"+#include"parse-options.h"++#include<stdio.h>++staticintverbose;++staticintprint_object(constunsignedchar*sha1)+{+if(verbose){+unsignedlongsize;+inttype=sha1_object_info(sha1,&size);++if(type<0)+return-1;++printf("%s %s %lu\n",sha1_to_hex(sha1),typename(type),size);+}+else+printf("%s\n",sha1_to_hex(sha1));++return0;+}++staticintcheck_loose_object(constunsignedchar*sha1,+constchar*path,+void*data)+{+returnprint_object(sha1);+}++staticintcheck_packed_object(constunsignedchar*sha1,+structpacked_git*pack,+uint32_tpos,+void*data)+{+returnprint_object(sha1);+}++staticstructoptionbuiltin_filter_objects_options[]={+OPT__VERBOSE(&verbose,"show object type and size"),+OPT_END()+};++intcmd_list_all_objects(intargc,constchar**argv,constchar*prefix)+{+structpacked_git*p;++argc=parse_options(argc,argv,prefix,builtin_filter_objects_options,+NULL,0);++for_each_loose_object(check_loose_object,NULL,0);++prepare_packed_git();+for(p=packed_git;p;p=p->next){+open_pack_index(p);+}++for_each_packed_object(check_packed_object,NULL,0);++return0;+}
@@ -0,0 +1,45 @@+#!/bin/sh++test_description='git list-all-objects'+../test-lib.sh++test_expect_success'setup''+echohello,world>file&&+gitaddfile&&+gitcommit-m"initial"+'++test_basic_repo_objects(){+gitcat-file--batch-check="%(objectname)"<<-EOF>expected.unsorted&&+HEAD+HEAD:file+HEAD^{tree}+EOF+gitlist-all-objects>all-objects.unsorted&&+sortexpected.unsorted>expected&&+sortall-objects.unsorted>all-objects&&+test_cmpall-objectsexpected+}++test_expect_success'list all objects''+test_basic_repo_objects+'+test_expect_success'list all objects after pack''+gitrepack-Ad&&+test_basic_repo_objects+'++test_expect_success'verbose output''+gitcat-file--batch-check="%(objectname) %(objecttype) %(objectsize)"\+<<-EOF>expected.unsorted&&+HEAD+HEAD:file+HEAD^{tree}+EOF+gitlist-all-objects-v>all-objects.unsorted&&+sortexpected.unsorted>expected&&+sortall-objects.unsorted>all-objects&&+test_cmpall-objectsexpected+'++test_done
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
On Sun, Jun 21, 2015 at 08:20:30PM +0100, Charles Bailey wrote:
I performed some test timings of some different commands on a clone of
the Linux kernel which was completely packed.
Thanks for timing things. I think we can fairly easily improve a bit on
what you have here. I'll go through my full analysis, but see the
conclusions at the end.
$ time git rev-list --all --objects |
cut -d" " -f1 |
git cat-file --batch-check |
awk '{if ($3 >= 512000) { print $1 }}' |
wc -l
958
real 0m30.823s
user 0m41.904s
sys 0m7.728s
list-all-objects gives a significant improvement:
$ time git list-all-objects |
git cat-file --batch-check |
awk '{if ($3 >= 512000) { print $1 }}' |
wc -l
958
real 0m9.585s
user 0m10.820s
sys 0m4.960s
That makes sense; of course these two are not necessarily producing the
same answer (they do in your case because it's a fresh clone, and all of
the objects are reachable). I think that's an acceptable caveat.
You can speed up the second one by asking batch-check only for the parts
you care about:
git list-all-objects |
git cat-file --batch-check='%(objectsize) %(objectname)' |
awk '{if ($1 >= 512000) { print $2 }}' |
wc -l
That dropped my best-of-five timings for the same test down from 9.5s to
7.0s. The answer should be the same. The reason is that cat-file will
only compute the items it needs to show, and the object-type is more
expensive to get than the size[1].
Replacing awk with:
perl -alne 'print $F[0] if $F[1] > 512000'
dropped that to 6.0s. That mostly means my awk sucks, but it is
interesting to note that not all of the extra time is pipe overhead
inherent to this approach; your choice of processor matters, too.
If you're willing to get a slightly different answer, but one that is
often just as useful, you can replace the "%(objectsize)" in the
cat-file invocation with "%(objectsize:disk)". That gives you the actual
on-disk size of the object, which includes delta and zlib compression.
For 512K, that produces very different results (because files of that
size may actually be text file). But for most truly huge files, they
typically do not delta or compress at all, and the on-disk size is
roughly the same.
That only shaves off 100-200 milliseconds, though.
[1] If you are wondering why the size is cheaper than the type, it is
because of deltas. For base objects, we can get either immediately
from the pack entry's header. For a delta, to get the size we have
to open the object data; the expected size is part of the delta
data. So we pay the extra cost to zlib-inflate the first few bytes.
But finding the type works differently; the type in the pack header
is OFS_DELTA, so we have to walk back to the parent entry to find
the real type. If that parent is a delta, we walk back recursively
until we hit a base object.
You'd think that would also make %(objectsize:disk) much cheaper
than %(objectsize), too. But the disk sizes require computing a
the pack revindex on the fly, which takes a few hundred milliseconds
on linux.git.
skipping the cat-filter filter is a lesser but still significant
improvement:
$ time git list-all-objects -v |
awk '{if ($3 >= 512000) { print $1 }}' |
wc -l
958
real 0m5.637s
user 0m6.652s
sys 0m0.156s
That's a pretty nice improvement over the piped version. But we cannot
do the same custom-format optimization there, because "-v" does not
support it. It would be nice if it supported the full range of cat-file
formatters.
I did a hacky proof-of-concept, and that brought my 6.0s time down to
4.9s.
I also noticed that cat-file doesn't do any output buffering; this is
because it may be used interactively, line by line, by a caller
controlling both pipes. Replacing write_or_die() with fwrite in my
proof-of-concept dropped the time to 3.7s.
That's faster still than your original (different machines, obviously,
but your times are similar to mine):
The old filter-objects could do the size filter a little be faster, but
not by much:
$ time git filter-objects --min-size=500k |
wc -l
958
real 0m4.564s
user 0m4.496s
sys 0m0.064s
This is likely caused by your use of sha1_object_info(), which always
computes the type. Switching to the extended form would probably buy you
another 2 seconds or so.
Also, all my numbers are wall-clock times. The CPU time for my 3.7s time
is actually 6.8s. Whereas doing it all in one process would probably
require 3.0s or so of actual CPU time.
So my conclusions are:
1. Yes, the pipe/parsing overhead of a separate processor really is
measurable. That's hidden in the wall-clock time if you have
multiple cores, but you may care more about CPU time. I still think
the flexibility is worth it.
2. Cutting out the pipe to cat-file is worth doing, as it saves a few
seconds. Cutting out "%(objecttype)" saves a lot, too, and is worth
doing. We should teach "list-all-objects -v" to use cat-file's
custom formatters (alternatively, we could just teach cat-file a
"--batch-all-objects" option rather than add a new command).
3. We should teach cat-file a "--buffer" option to use fwrite. Even if
we end up with "list-all-objects --format='%(objectsize)'" for this
task, it would help all the other uses of cat-file.
-Peff
On Mon, Jun 22, 2015 at 2:20 AM, Charles Bailey [off-list ref] wrote:
From: Charles Bailey <redacted>
list-all-objects is a command to print the ids of all objects in the
object database of a repository. It is designed as a low overhead
interface for scripts that want to analyse all objects but don't require
the ordering implied by a revision walk.
It will list all objects, loose and packed, and will include unreachable
objects.
Nit picking, but perhaps we should allow to select object source:
loose, packed, alternates.. These info are available now and cheap to
get. It's ok not to do it now though.
Personally I would name this command "find-objects" (after unix
command "find") where we could still filter objects _not_ based on
object content.
--
Duy
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
On Mon, Jun 22, 2015 at 04:57:28PM +0700, Duy Nguyen wrote:
On Mon, Jun 22, 2015 at 2:20 AM, Charles Bailey [off-list ref] wrote:
quoted
From: Charles Bailey <redacted>
list-all-objects is a command to print the ids of all objects in the
object database of a repository. It is designed as a low overhead
interface for scripts that want to analyse all objects but don't require
the ordering implied by a revision walk.
It will list all objects, loose and packed, and will include unreachable
objects.
Nit picking, but perhaps we should allow to select object source:
loose, packed, alternates.. These info are available now and cheap to
get. It's ok not to do it now though.
There is already plumbing to do those individual operations if you want.
Although some of the plumbing involves "for i in objects/pack/*.pack",
which is perhaps a little less abstract than we'd like. :)
Personally I would name this command "find-objects" (after unix
command "find") where we could still filter objects _not_ based on
object content.
I like that better than "ls", too, but I propose that we actually add
this as a feature to cat-file. I'll send patches in a moment.
-Peff
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
On Mon, Jun 22, 2015 at 04:38:22AM -0400, Jeff King wrote:
quoted
+ prepare_packed_git();
+ for (p = packed_git; p; p = p->next) {
+ open_pack_index(p);
+ }
Yikes. The fact that you need to do this means that
for_each_packed_object is buggy, IMHO. I'll send a patch.
Here's that patch. And since I did not want to pile work on Charles, I
went ahead and just implemented the patches I suggested in the other
email.
We may want to take patch 1 separately for the maint-track, as it is
really a bug-fix (albeit one that I do not think actually affects anyone
in practice right now).
Patches 2-5 are useful even if we go with Charles' command, as they make
cat-file better (cleanups and he new buffer option).
Patches 6-7 implement the cat-file option that would be redundant with
list-all-objects.
By the way, in addition to not showing objects in order,
list-all-objects (and my cat-file option) may show duplicates. Do we
want to "sort -u" for the user? It might be nice for them to always get
a de-duped and sorted list. Aside from the CPU cost of sorting, it does
mean we'll allocate ~80MB for the kernel to store the sha1s. I guess
that's not too much when you are talking about the kernel repo. I took
the coward's way out and just mentioned the limitation in the
documentation, but I'm happy to be persuaded.
[1/7]: for_each_packed_object: automatically open pack index
[2/7]: cat-file: minor style fix in options list
[3/7]: cat-file: move batch_options definition to top of file
[4/7]: cat-file: add --buffer option
[5/7]: cat-file: stop returning value from batch_one_object
[6/7]: cat-file: split batch_one_object into two stages
[7/7]: cat-file: add --batch-all-objects option
-Peff
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
When for_each_packed_object is called, we call
prepare_packed_git() to make sure we have the actual list of
packs. But the latter does not actually open the pack
indices, meaning that pack->nr_objects may simply be 0 if
the pack has not otherwise been used since the program
started.
In practice, this didn't come up for the current callers,
because they iterate the packed objects only after iterating
all reachable objects (so for it to matter you would have to
have a pack consisting only of unreachable objects). But it
is a dangerous and confusing interface that should be fixed
for future callers.
Note that we do not end the iteration when a pack cannot be
opened, but we do return an error. That lets you complete
the iteration even in actively-repacked repository where an
.idx file may racily go away, but it also lets callers know
that they may not have gotten the complete list (which the
current reachability-check caller does care about).
We have to tweak one of the prune tests due to the changed
return value; an earlier test creates bogus .idx files and
does not clean them up. Having to make this tweak is a good
thing; it means we will not prune in a broken repository,
and the test confirms that we do not negatively impact a
more lenient caller, count-objects.
Signed-off-by: Jeff King <redacted>
---
sha1_file.c | 7 ++++++-
t/t5304-prune.sh | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
@@ -218,6 +218,7 @@ test_expect_success 'gc: prune old objects after local clone' '' test_expect_success'garbage report in count-objects -v''+test_when_finished"rm -f .git/objects/pack/fake*"&&:>.git/objects/pack/foo&&:>.git/objects/pack/foo.bar&&:>.git/objects/pack/foo.keep&&
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
We do not put extra whitespace before the first macro
argument.
Signed-off-by: Jeff King <redacted>
---
builtin/cat-file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -412,7 +412,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)OPT_CMDMODE('p',NULL,&opt,N_("pretty-print object's content"),'p'),OPT_CMDMODE(0,"textconv",&opt,N_("for blob objects, run textconv on object's content"),'c'),-OPT_BOOL(0,"allow-unknown-type",&unknown_type,+OPT_BOOL(0,"allow-unknown-type",&unknown_type,N_("allow -s and -t to work with broken/corrupt objects")),{OPTION_CALLBACK,0,"batch",&batch,"format",N_("show info and content of objects fed from the standard input"),
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
That way all of the functions can make use of it.
Signed-off-by: Jeff King <redacted>
---
builtin/cat-file.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
We use a direct write() to output the results of --batch and
--batch-check. This is good for processes feeding the input
and reading the output interactively, but it introduces
measurable overhead if you do not want this feature. For
example, on linux.git:
$ git rev-list --objects --all | cut -d' ' -f1 >objects
$ time git cat-file --batch-check='%(objectsize)' \
<objects >/dev/null
real 0m5.440s
user 0m5.060s
sys 0m0.384s
This patch adds an option to use regular stdio buffering:
$ time git cat-file --batch-check='%(objectsize)' \
--buffer <objects >/dev/null
real 0m4.975s
user 0m4.888s
sys 0m0.092s
Signed-off-by: Jeff King <redacted>
---
This selectively uses fwrite or write_or_die, depending on the buffer
setting. Another option would be to just always use fwrite(), and then
selectively fflush(). It feels kind of wasteful in the non-buffered
case, as it's just another layer to write through. OTOH, the cost of
writing a line into the buffer only to flush is probably dwarfed by the
system call of actually flushing.
If we went that direction, we could probably simplify the code a bit
(both getting rid of the batch_write function I call here, and dropping
a bunch of existing fflush() calls, where we must flush any time we use
printf for its formatting capabilities).
I also considered that the "--buffer" case is likely to be the common
one. We cannot flip the default, though, as it would break any existing
callers (who would need to specify "--no-buffer"). We can do the usual
deprecation dance, but I don't know if it is worth it for a plumbing
command like this.
Documentation/git-cat-file.txt | 7 +++++++
builtin/cat-file.c | 26 +++++++++++++++++++-------
2 files changed, 26 insertions(+), 7 deletions(-)
@@ -69,6 +69,13 @@ OPTIONS not be combined with any other options or arguments. See the section `BATCH OUTPUT` below for details.+--buffer::+ Normally batch output is flushed after each object is output, so+ that a process can interactively read and write from+ `cat-file`. With this option, the output uses normal stdio+ buffering; this is much more efficient when invoking+ `--batch-check` on a large number of objects.+ --allow-unknown-type:: Allow -s or -t to query broken/corrupt objects of unknown type.
@@ -415,6 +426,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)N_("for blob objects, run textconv on object's content"),'c'),OPT_BOOL(0,"allow-unknown-type",&unknown_type,N_("allow -s and -t to work with broken/corrupt objects")),+OPT_BOOL(0,"buffer",&batch.buffer_output,N_("buffer --batch output")),{OPTION_CALLBACK,0,"batch",&batch,"format",N_("show info and content of objects fed from the standard input"),PARSE_OPT_OPTARG,batch_option_callback},
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
If batch_one_object returns an error code, we stop reading
input. However, it will only do so if we feed it NULL,
which cannot happen; we give it the "buf" member of a
strbuf, which is always non-NULL.
We did originally stop on other errors (like a missing
object), but this was changed in 3c076db (cat-file --batch /
--batch-check: do not exit if hashes are missing,
2008-06-09). These days we keep going for any per-object
error (and print "missing" when necessary).
Signed-off-by: Jeff King <redacted>
---
builtin/cat-file.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
There are really two things going on in this function:
1. We convert the name we got on stdin to a sha1.
2. We look up and print information on the sha1.
Let's split out the second half so that we can call it
separately.
Signed-off-by: Jeff King <redacted>
---
builtin/cat-file.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
It can sometimes be useful to examine all objects in the
repository. Normally this is done with "git rev-list --all
--objects", but:
1. That shows only reachable objects. You may want to look
at all available objects.
2. It's slow. We actually open each object to walk the
graph. If your operation is OK with seeing unreachable
objects, it's an order of magnitude faster to just
enumerate the loose directories and pack indices.
You can do this yourself using "ls" and "git show-index",
but it's non-obvious. This patch adds an option to
"cat-file --batch-check" to operate on all available
objects (rather than reading names from stdin).
This is based on a proposal by Charles Bailey to provide a
separate "git list-all-objects" command. That is more
orthogonal, as it splits enumerating the objects from
getting information about them. However, in practice you
will either:
a. Feed the list of objects directly into cat-file anyway,
so you can find out information about them. Keeping it
in a single process is more efficient.
b. Ask the listing process to start telling you more
information about the objects, in which case you will
reinvent cat-file's batch-check formatter.
Adding a cat-file option is simple and efficient. And if you
really do want just the object names, you can always do:
git cat-file --batch-check='%(objectname)' --batch-all-objects
Signed-off-by: Jeff King <redacted>
---
Documentation/git-cat-file.txt | 8 ++++++++
builtin/cat-file.c | 44 ++++++++++++++++++++++++++++++++++++++++--
t/t1006-cat-file.sh | 27 ++++++++++++++++++++++++++
3 files changed, 77 insertions(+), 2 deletions(-)
@@ -69,6 +69,14 @@ OPTIONS not be combined with any other options or arguments. See the section `BATCH OUTPUT` below for details.+--batch-all-objects::+ Instead of reading a list of objects on stdin, perform the+ requested batch operation on all objects in the repository and+ any alternate object stores (not just reachable objects).+ Requires `--batch` or `--batch-check` be specified. Note that+ the order of the objects is unspecified, and there may be+ duplicate entries.+ --buffer:: Normally batch output is flushed after each object is output, so that a process can interactively read and write from
@@ -345,6 +374,15 @@ static int batch_objects(struct batch_options *opt)if(opt->print_contents)data.info.typep=&data.type;+if(opt->all_objects){+structobject_cb_datacb;+cb.opt=opt;+cb.expand=&data;+for_each_loose_object(batch_loose_object,&cb,0);+for_each_packed_object(batch_packed_object,&cb,0);+return0;+}+/**Wearegoingtocallget_sha1onapotentiallyverylargenumberof*objects.Inmostlargecases,thesewillbeactualobjectsha1s.The
@@ -436,6 +474,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)PARSE_OPT_OPTARG,batch_option_callback},OPT_BOOL(0,"follow-symlinks",&batch.follow_symlinks,N_("follow in-tree symlinks (used with --batch or --batch-check)")),+OPT_BOOL(0,"batch-all-objects",&batch.all_objects,+N_("show all objects with --batch or --batch-check")),OPT_END()};
@@ -547,4 +547,31 @@ test_expect_success 'git cat-file --batch --follow-symlink returns correct sha atest_cmpexpectactual'+test_expect_success'cat-file --batch-all-objects shows all objects''+# make new repos so we now the full set of objects; we will+# also make sure that there are some packed and some loose+# objects, some referenced and some not, and that there are+# some available only via alternates.+gitinitall-one&&+(+cdall-one&&+echocontent>file&&+gitaddfile&&+gitcommit-qmbase&&+gitrev-parseHEADHEAD^{tree}HEAD:file&&+gitrepack-ad&&+echonot-cloned|githash-object-w--stdin+)>expect.unsorted&&+gitclone-sall-oneall-two&&+(+cdall-two&&+echolocal-unref|githash-object-w--stdin+)>>expect.unsorted&&+sort<expect.unsorted>expect&&+git-Call-twocat-file--batch-all-objects\+--batch-check="%(objectname)">actual.unsorted&&+sort<actual.unsorted>actual&&+test_cmpexpectactual+'+ test_done
From: Jeff King <hidden> Date: 2016-06-15 23:05:26
On Mon, Jun 22, 2015 at 06:33:21AM -0400, Jeff King wrote:
By the way, in addition to not showing objects in order,
list-all-objects (and my cat-file option) may show duplicates. Do we
want to "sort -u" for the user? It might be nice for them to always get
a de-duped and sorted list. Aside from the CPU cost of sorting, it does
mean we'll allocate ~80MB for the kernel to store the sha1s. I guess
that's not too much when you are talking about the kernel repo. I took
the coward's way out and just mentioned the limitation in the
documentation, but I'm happy to be persuaded.
The patch below does the sort/de-dup. I'd probably just squash it into
patch 7, though.
I did have one additional thought, though. We are treating this as two
separate operations: "what are the sha1s in the repo" and "show me
information about this sha1". But by integrating with cat-file, we could
actually show information not just about a particular sha1, but about a
particular on-disk object.
E.g., if there are duplicates of a particular object, some formatters
like "%(objectsize:disk)" and "%(deltabase)" pick one arbitrarily to
show. I don't know if anybody actually cares about that in practice, but
if we show duplicates, we could give the accurate information for each
instance (and in fact we could give other information like loose vs
packed, which file contains the object, etc).
I tend to think that the lack of de-duping is sufficiently confusing
that it should be the default, and we can always add a "no really, show
me the duplicates" option later. It is not as simple as skipping the
de-dup step. We'd have to actually avoid calling sha1_object_info, and
use the information found in the loose/pack traversal (which would in
turn require exposing the low-level bits of sha1_object_info).
-- >8 --
Subject: cat-file: sort and de-dup output of --batch-all-objects
The sorting we could probably live without, but printing
duplicates is just a hassle for the user, who must then
de-dup themselves (or risk a wrong answer if they are doing
something like counting objects with a particular property).
Signed-off-by: Jeff King <redacted>
---
Documentation/git-cat-file.txt | 3 +--
builtin/cat-file.c | 22 +++++++++++++++-------
t/t1006-cat-file.sh | 3 +--
3 files changed, 17 insertions(+), 11 deletions(-)
@@ -74,8 +74,7 @@ OPTIONS requested batch operation on all objects in the repository and any alternate object stores (not just reachable objects). Requires `--batch` or `--batch-check` be specified. Note that- the order of the objects is unspecified, and there may be- duplicate entries.+ the objects are visited in order sorted by their hashes. --buffer:: Normally batch output is flushed after each object is output, so
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:26
On Mon, Jun 22, 2015 at 04:38:22AM -0400, Jeff King wrote:
On Sun, Jun 21, 2015 at 08:20:31PM +0100, Charles Bailey wrote:
quoted
+ prepare_packed_git();
+ for (p = packed_git; p; p = p->next) {
+ open_pack_index(p);
+ }
Yikes. The fact that you need to do this means that
for_each_packed_object is buggy, IMHO. I'll send a patch.
I'm glad you said that; the interface did seem a bit warty at the time
but as I "fixed" this early in my hacking I didn't remeber to revisit
this and ask if it was actually intentional.
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:27
On Mon, Jun 22, 2015 at 06:33:21AM -0400, Jeff King wrote:
On Mon, Jun 22, 2015 at 04:38:22AM -0400, Jeff King wrote:
quoted
quoted
+ prepare_packed_git();
+ for (p = packed_git; p; p = p->next) {
+ open_pack_index(p);
+ }
Yikes. The fact that you need to do this means that
for_each_packed_object is buggy, IMHO. I'll send a patch.
Here's that patch. And since I did not want to pile work on Charles, I
went ahead and just implemented the patches I suggested in the other
email.
I have to say that I think that adding this functionality to cat-file
makes a lot of sense. If it only catted files it might be a stretch but
as it's already grown --batch-check functionality, it now seems a
reasonable extension. I'm not particularly attached to having a
standalone "list-all-objects" command per se.
From: Charles Bailey <hidden> Date: 2016-06-15 23:05:27
On Mon, Jun 22, 2015 at 07:06:32AM -0400, Jeff King wrote:
On Mon, Jun 22, 2015 at 06:33:21AM -0400, Jeff King wrote:
quoted
By the way, in addition to not showing objects in order,
list-all-objects (and my cat-file option) may show duplicates. Do we
want to "sort -u" for the user? It might be nice for them to always get
a de-duped and sorted list. Aside from the CPU cost of sorting, it does
mean we'll allocate ~80MB for the kernel to store the sha1s. I guess
that's not too much when you are talking about the kernel repo. I took
the coward's way out and just mentioned the limitation in the
documentation, but I'm happy to be persuaded.
The patch below does the sort/de-dup. I'd probably just squash it into
patch 7, though.
Woah, 8 out of 7! Did you get a chance to measure the performance hit of
the sort? If not, I may test it out when I next get the chance.
From: Jeff King <hidden> Date: 2016-06-15 23:05:27
On Mon, Jun 22, 2015 at 11:03:50PM +0100, Charles Bailey wrote:
quoted
The patch below does the sort/de-dup. I'd probably just squash it into
patch 7, though.
Woah, 8 out of 7! Did you get a chance to measure the performance hit of
the sort? If not, I may test it out when I next get the chance.
No, that last patch was my "eh, one more thing before bed" patch. ;)
It's easy enough to time, though. Running:
git cat-file --batch-all-objects \
--batch-check='%(objectsize) %(objectname)' \
--buffer >/dev/null
on linux.git, my best-of-five goes from (no sorting):
real 0m3.604s
user 0m3.556s
sys 0m0.048s
to (with sorting):
real 0m4.053s
user 0m4.004s
sys 0m0.052s
So it does matter, but not too much. We could de-dup with a hash table,
which might be a little faster, but I doubt it would make much
difference. It's also mostly in sorted order already; it's possible
that a merge sort would behave a little better. I'm not sure how deep
it's worth going into that rabbit hole.
-Peff
From: Eric Sunshine <hidden> Date: 2016-06-15 23:05:32
On Mon, Jun 22, 2015 at 6:45 AM, Jeff King [off-list ref] wrote:
quoted hunk
[...] This patch adds an option to
"cat-file --batch-check" to operate on all available
objects (rather than reading names from stdin).
Signed-off-by: Jeff King <redacted>
---
@@ -547,4 +547,31 @@ test_expect_success 'git cat-file --batch --follow-symlink returns correct sha atest_cmpexpectactual'+test_expect_success'cat-file --batch-all-objects shows all objects''+# make new repos so we now the full set of objects; we will
s/now/know/
+ # also make sure that there are some packed and some loose
+ # objects, some referenced and some not, and that there are
+ # some available only via alternates.
+ git init all-one &&
+ (
+ cd all-one &&
+ echo content >file &&
+ git add file &&
+ git commit -qm base &&
+ git rev-parse HEAD HEAD^{tree} HEAD:file &&
+ git repack -ad &&
+ echo not-cloned | git hash-object -w --stdin
+ ) >expect.unsorted &&
+ git clone -s all-one all-two &&
+ (
+ cd all-two &&
+ echo local-unref | git hash-object -w --stdin
+ ) >>expect.unsorted &&
+ sort <expect.unsorted >expect &&
+ git -C all-two cat-file --batch-all-objects \
+ --batch-check="%(objectname)" >actual.unsorted &&
+ sort <actual.unsorted >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.4.4.719.g3984bc6
From: Jeff King <hidden> Date: 2016-06-15 23:05:32
On Fri, Jun 26, 2015 at 02:56:58AM -0400, Eric Sunshine wrote:
quoted
+test_expect_success 'cat-file --batch-all-objects shows all objects' '
+ # make new repos so we now the full set of objects; we will
s/now/know/
Yeah. I don't think this series otherwise needs re-rolled. Here it is in
an autosquash-able form:
-- >8 --
Subject: [PATCH] fixup! cat-file: add --batch-all-objects option
---
t/t1006-cat-file.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -548,7 +548,7 @@ test_expect_success 'git cat-file --batch --follow-symlink returns correct sha a' test_expect_success'cat-file --batch-all-objects shows all objects''-# make new repos so we now the full set of objects; we will+# make new repos so we know the full set of objects; we will# also make sure that there are some packed and some loose# objects, some referenced and some not, and that there are# some available only via alternates.