threads / patch / 8217

patch, 2 partspack-objects: pass fullname down to add_object_entry()

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

## tl;dr

6 messages between May 19, 2007 and May 22, 2007. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Junio C Hamano· May 19, 2007, 07:48 UTC · lore

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(-)
Show changes to builtin-pack-objects.c +14 −13
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
Junio C Hamano· May 19, 2007, 07:48 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to builtin-pack-objects.c +34 −1
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
Dana How· May 19, 2007, 16:10 UTC · re: Junio C Hamano · lore

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

On 5/19/07, Junio C Hamano <junkio@cox.net> wrote:
Show 12 quoted lines
> 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
Junio C Hamano· May 19, 2007, 20:03 UTC · re: Dana How · lore

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

"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.

Dana How· May 19, 2007, 23:56 UTC · re: Junio C Hamano · lore

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

On 5/19/07, Junio C Hamano <junkio@cox.net> wrote:
Show 12 quoted lines
> "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
Nicolas Pitre· May 22, 2007, 16:04 UTC · re: Junio C Hamano · lore

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

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.
[...]
Show 20 quoted lines
> @@ -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

← back to recent threads