On Tue, Feb 04, 2014 at 10:37:24AM -0800, Junio C Hamano wrote: > Kirill Smelkov <kirr@xxxxxxxxxx> writes: > > >> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html