[PATCH 1/2] pack-objects: pass fullname down to add_object_entry()

[PATCH 1/2] pack-objects: pass fullname down to add_object_entry()

[Junio C Hamano]

Instead of giving a hash for grouping, pass fullname to add_object_entry().
I want to add "do not try deltifying this object" bit to object_entry based on
the settings in .gitattributes, and hashing the name down too early would
interfere with that plan.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-pack-objects.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 5fa9813..12d9685 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -721,6 +721,9 @@ static unsigned name_hash(const char *name)
 	unsigned char c;
 	unsigned hash = 0;
 
+	if (!name)
+		return 0;
+
 	/*
 	 * This effectively just creates a sortable number from the
 	 * last sixteen non-whitespace characters. Last characters
@@ -735,12 +738,13 @@ static unsigned name_hash(const char *name)
 }
 
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
-			    unsigned hash, int exclude)
+			    const char *name, int exclude)
 {
 	struct object_entry *entry;
 	struct packed_git *p, *found_pack = NULL;
 	off_t found_offset = 0;
 	int ix;
+	unsigned hash = name_hash(name);
 
 	ix = nr_objects ? locate_object_entry_hash(sha1) : -1;
 	if (ix >= 0) {
@@ -929,10 +933,9 @@ static void add_pbase_object(struct tree_desc *tree,
 		if (cmp < 0)
 			return;
 		if (name[cmplen] != '/') {
-			unsigned hash = name_hash(fullname);
 			add_object_entry(entry.sha1,
 					 S_ISDIR(entry.mode) ? OBJ_TREE : OBJ_BLOB,
-					 hash, 1);
+					 fullname, 1);
 			return;
 		}
 		if (S_ISDIR(entry.mode)) {
@@ -992,10 +995,11 @@ static int check_pbase_path(unsigned hash)
 	return 0;
 }
 
-static void add_preferred_base_object(const char *name, unsigned hash)
+static void add_preferred_base_object(const char *name)
 {
 	struct pbase_tree *it;
 	int cmplen;
+	unsigned hash = name_hash(name);
 
 	if (!num_preferred_base || check_pbase_path(hash))
 		return;
@@ -1003,7 +1007,7 @@ static void add_preferred_base_object(const char *name, unsigned hash)
 	cmplen = name_cmp_len(name);
 	for (it = pbase_tree; it; it = it->next) {
 		if (cmplen == 0) {
-			add_object_entry(it->pcache.sha1, OBJ_TREE, 0, 1);
+			add_object_entry(it->pcache.sha1, OBJ_TREE, NULL, 1);
 		}
 		else {
 			struct tree_desc tree;
@@ -1434,7 +1438,6 @@ static void read_object_list_from_stdin(void)
 {
 	char line[40 + 1 + PATH_MAX + 2];
 	unsigned char sha1[20];
-	unsigned hash;
 
 	for (;;) {
 		if (!fgets(line, sizeof(line), stdin)) {
@@ -1457,22 +1460,20 @@ static void read_object_list_from_stdin(void)
 		if (get_sha1_hex(line, sha1))
 			die("expected sha1, got garbage:\n %s", line);
 
-		hash = name_hash(line+41);
-		add_preferred_base_object(line+41, hash);
-		add_object_entry(sha1, 0, hash, 0);
+		add_preferred_base_object(line+41);
+		add_object_entry(sha1, 0, line+41, 0);
 	}
 }
 
 static void show_commit(struct commit *commit)
 {
-	add_object_entry(commit->object.sha1, OBJ_COMMIT, 0, 0);
+	add_object_entry(commit->object.sha1, OBJ_COMMIT, NULL, 0);
 }
 
 static void show_object(struct object_array_entry *p)
 {
-	unsigned hash = name_hash(p->name);
-	add_preferred_base_object(p->name, hash);
-	add_object_entry(p->item->sha1, p->item->type, hash, 0);
+	add_preferred_base_object(p->name);
+	add_object_entry(p->item->sha1, p->item->type, p->name, 0);
 }
 
 static void show_edge(struct commit *commit)
-- 
1.5.2.rc3.87.g404f

[PATCH 2/2] Teach "delta" attribute to pack-objects.

[Junio C Hamano]

This teaches pack-objects to use .gitattributes mechanism so
that the user can specify certain blobs are not worth spending
CPU cycles to attempt deltification.

The name of the attrbute is "delta", and when it is set to
false, like this:

	== .gitattributes ==
	*.jpg	-delta

they are always stored in the plain-compressed base object
representation.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-pack-objects.c |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 12d9685..651011c 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "attr.h"
 #include "object.h"
 #include "blob.h"
 #include "commit.h"
@@ -40,9 +41,10 @@ struct object_entry {
 	enum object_type in_pack_type;	/* could be delta */
 	unsigned char in_pack_header_size;
 	unsigned char preferred_base; /* we do not pack this, but is available
-				       * to be used as the base objectto delta
+				       * to be used as the base object to delta
 				       * objects against.
 				       */
+	unsigned char no_try_delta;
 };
 
 /*
@@ -737,6 +739,28 @@ static unsigned name_hash(const char *name)
 	return hash;
 }
 
+static void setup_delta_attr_check(struct git_attr_check *check)
+{
+	static struct git_attr *attr_delta;
+
+	if (!attr_delta)
+		attr_delta = git_attr("delta", 5);
+
+	check[0].attr = attr_delta;
+}
+
+static int no_try_delta(const char *path)
+{
+	struct git_attr_check check[1];
+
+	setup_delta_attr_check(check);
+	if (git_checkattr(path, ARRAY_SIZE(check), check))
+		return 0;
+	if (ATTR_FALSE(check->value))
+		return 1;
+	return 0;
+}
+
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
 			    const char *name, int exclude)
 {
@@ -801,6 +825,9 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
 	if (progress)
 		display_progress(&progress_state, nr_objects);
 
+	if (name && no_try_delta(name))
+		entry->no_try_delta = 1;
+
 	return 1;
 }
 
@@ -1349,6 +1376,10 @@ static void find_deltas(struct object_entry **list, int window, int depth)
 
 		if (entry->size < 50)
 			continue;
+
+		if (entry->no_try_delta)
+			continue;
+
 		free_delta_index(n->index);
 		n->index = NULL;
 		free(n->data);
@@ -1376,6 +1407,8 @@ static void find_deltas(struct object_entry **list, int window, int depth)
 			m = array + other_idx;
 			if (!m->entry)
 				break;
+			if (m->entry->no_try_delta)
+				continue;
 			if (try_delta(n, m, max_depth) < 0)
 				break;
 		}
-- 
1.5.2.rc3.87.g404f

Re: [PATCH 2/2] Teach "delta" attribute to pack-objects.

[Dana How]

On 5/19/07, Junio C Hamano <junkio@cox.net> wrote:
> This teaches pack-objects to use .gitattributes mechanism so
> that the user can specify certain blobs are not worth spending
> CPU cycles to attempt deltification.
>
> The name of the attrbute is "delta", and when it is set to
> false, like this:
>
>         == .gitattributes ==
>         *.jpg   -delta
>
> they are always stored in the plain-compressed base object
> representation.

And we could also have an attribute "repack" :
    == .gitattributes ==
    *.wmv    -repack
which would result in *.wmv files [enormous] not being packed.
Since add_object_entry() now gets the name and
can reject objects (e.g. currently on their current packing status),
a call to "no_pack" which mirrors your new "no_try_delta"
could be inserted there.  But such an attribute should be
ignored when --stdout is in effect -- it only affects on-disk
repacking, not packing for transfers, which is why it's named "repack".

-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH 2/2] Teach "delta" attribute to pack-objects.

[Junio C Hamano]

"Dana How" <danahow@gmail.com> writes:

> ...  But such an attribute should be
> ignored when --stdout is in effect -- it only affects on-disk
> repacking, not packing for transfers, which is why it's named "repack".

Yes -- if we want to have an option to keep objects selectively
left out of packs in loose format, you would need 'repack' which
acts differently between the server-feeding-client case vs
packing-repository case.

Which is a bigger change that I did not want to show in the
quick-and-clean patch, but I would agree we would want both.

Re: [PATCH 2/2] Teach "delta" attribute to pack-objects.

[Dana How]

On 5/19/07, Junio C Hamano <junkio@cox.net> wrote:
> "Dana How" <danahow@gmail.com> writes:
> > ...  But such an attribute should be
> > ignored when --stdout is in effect -- it only affects on-disk
> > repacking, not packing for transfers, which is why it's named "repack".
>
> Yes -- if we want to have an option to keep objects selectively
> left out of packs in loose format, you would need 'repack' which
> acts differently between the server-feeding-client case vs
> packing-repository case.
>
> Which is a bigger change that I did not want to show in the
> quick-and-clean patch, but I would agree we would want both.

OK -- I'll put this on the back-burner with the so-called
"ent:relative" patch and I'll revisit this once your "delta"
attribute shows up in next or master.

At the moment I'm experimenting on a git repository with
a 4.5GB checkout,  and 18 months of history in 4K commits
comprising 100GB (uncompressed) of blobs stored in
7 packfiles of 2GB or less.  Hopefully I'll be able to say
more about tweaking packing shortly.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell

Re: [PATCH 2/2] Teach "delta" attribute to pack-objects.

[Nicolas Pitre]

On Sat, 19 May 2007, Junio C Hamano wrote:

> This teaches pack-objects to use .gitattributes mechanism so
> that the user can specify certain blobs are not worth spending
> CPU cycles to attempt deltification.
[...]
> @@ -1349,6 +1376,10 @@ static void find_deltas(struct object_entry **list, int window, int depth)
>  
>  		if (entry->size < 50)
>  			continue;
> +
> +		if (entry->no_try_delta)
> +			continue;
> +
>  		free_delta_index(n->index);
>  		n->index = NULL;
>  		free(n->data);
> @@ -1376,6 +1407,8 @@ static void find_deltas(struct object_entry **list, int window, int depth)
>  			m = array + other_idx;
>  			if (!m->entry)
>  				break;
> +			if (m->entry->no_try_delta)
> +				continue;
>  			if (try_delta(n, m, max_depth) < 0)
>  				break;
>  		}

This last hunk is unnecessary.  Because of the other hunk above, the 
no_try_delta objects will never get into the m array.

Well done otherwise.  This attribute thing is really nice.


Nicolas