Kirill Smelkov [off-list ref] writes:
quoted
if we did not want to reinvent the whole tree walking thing, which
would add risks for new bugs and burden to maintain this and the
usual two-tree diff tree walking in sync.
Junio, I understand it is not good to duplicate code and increase
maintenance risks....
Instead I propose we switch to the new tree walker completely.
I am not fundamentally opposed to the idea. We'll see what happens.
On Tue, Feb 04, 2014 at 10:37:24AM -0800, Junio C Hamano wrote:
Kirill Smelkov [off-list ref] writes:
quoted
quoted
if we did not want to reinvent the whole tree walking thing, which
would add risks for new bugs and burden to maintain this and the
usual two-tree diff tree walking in sync.
Junio, I understand it is not good to duplicate code and increase
maintenance risks....
Instead I propose we switch to the new tree walker completely.
I am not fundamentally opposed to the idea. We'll see what happens.
Thanks. Locally, with draft patches with generalized tree-walker, for
`git log --raw --no-abbrev --no-renames` I was able to get to as fast as
with old two-tree walker for navy.git and 1% faster for linux.git and
all tests pass.
Only, before I clean things up, I'd like to ask - would the following
patch be accepted
---- 8< ---
diff --git a/tree-walk.c b/tree-walk.c
index 79dba1d..4dc86c7 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -37,7 +37,7 @@ static void decode_tree_entry(struct tree_desc *desc, const char *buf, unsigned
/* Initialize the descriptor entry */
desc->entry.path = path;
- desc->entry.mode = mode;
+ desc->entry.mode = canon_mode(mode);
desc->entry.sha1 = (const unsigned char *)(path + len);
}
diff --git a/tree-walk.h b/tree-walk.h
index ae04b64..ae7fb3a 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -16,7 +16,7 @@ struct tree_desc {
static inline const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep)
{
*pathp = desc->entry.path;
- *modep = canon_mode(desc->entry.mode);
+ *modep = desc->entry.mode;
return desc->entry.sha1;
}
---- 8< ---
?
The reason I ask is because it is more handy to work with tree_desc
structures, compared to splitting it to sha1/path/mode/pathlen, and
if canon_mode() is done only once, clients don't have to pay the
performance price of canon_mode on each tree_desc "extract".
I understand there is fsck, which on invalid tree entry prints the mode,
but maybe somehow fsck should be special, and common interface be tuned
to usual clients?
Thanks,
Kirill