Re: [PATCH v4] fast-import: do not write bad delta for replaced subtrees

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 20, 2011, 17:48 UTC
Message-ID: <20110820174812.GD15864@elie.gateway.2wire.net>
In-Reply-To: <1313860946-1596-1-git-send-email-divanorama@gmail.com>

Dmitry Ivankov wrote:

> How about adding a new bit field "no_delta" instead?

Currently the layout of "struct tree_entry_ms" is:

	uint16_t mode;			two bytes
	unsigned char sha1[20]		20 bytes

which adds up to 22 bytes.  Here is "struct tree_entry":

	struct tree_entry *tree;		one machine word
	struct atom_str *name;			one machine word
	struct tree_entry_ms versions[2];	44 bytes

Although it only looks like it adds one byte per tree entry, in practice I suspect your patch adds four. Is that worth it? (The answer might be yes. I'm not sure.)

> The patch is
> smaller this way. Also could 04000 theoretically be S_IFDIR on some
> platform?

No, these modes are part of the format of objects as written on disk and over the wire, so when we meet a platform with S_IFDIR != 040000, there will have to be bigger changes (to distinguish between the platform's idea of file status and git's idea of modes).

> - switch to a separate no_delta bit in tree_entry

If it doesn't cost too much, this is a good idea.

> - when setting no_delta = 1 don't check for S_ISDIR(versions[0].mode),
>   this is a redundant check and logic duplication. Who knows, maybe some
>   day we'll want to delta a tree against blob. :)

Why? When versions[0] is not a tree, the hack is not needed, since versions[0].mode and versions[0].sha1 accurately describe the delta base and are not inconsistent with anything.

Thanks, that was helpful.