Re: [PATCH/RFC] Replace ce_namelen() with a ce_namelen field
From: Thomas Rast <hidden>
Date: 2016-06-15 22:54:13
Thomas Gummerer [off-list ref] writes:
Replace the ce_namelen() function in cache.h with a ce_namelen field in struct cache_entry. This will both give us a tiny bit of a performance enhancement when working with long pathnames and is part of the refactoring for the index-v5 file format.
Expand this, at least proportionally to the damage incurred. For example, I know that you know that there's a good reason why we want to put it in a new field when reading from v5 :-)
Signed-off-by: Thomas Gummerer <redacted> --- builtin/add.c | 4 ++-- builtin/apply.c | 1 + builtin/blame.c | 1 + builtin/checkout-index.c | 8 ++++---- builtin/checkout.c | 7 ++++--- builtin/clean.c | 2 +- builtin/commit.c | 2 +- builtin/grep.c | 2 +- builtin/ls-files.c | 8 ++++---- builtin/rm.c | 2 +- builtin/update-index.c | 9 ++++++--- cache-tree.c | 4 ++-- cache.h | 11 ++--------- diff-lib.c | 6 +++--- entry.c | 2 +- merge-recursive.c | 2 +- name-hash.c | 4 ++-- preload-index.c | 2 +- read-cache.c | 43 ++++++++++++++++++++++++------------------- rerere.c | 2 +- resolve-undo.c | 2 +- sha1_name.c | 6 +++--- submodule.c | 2 +- tree.c | 1 + unpack-trees.c | 31 ++++++++++++++++--------------- 25 files changed, 85 insertions(+), 79 deletions(-)
[...]
quoted hunk
diff --git a/cache.h b/cache.h index cc5048c..5f93f22 100644 --- a/cache.h +++ b/cache.h@@ -128,6 +128,7 @@ struct cache_entry { unsigned int ce_gid; unsigned int ce_size; unsigned int ce_flags; + unsigned int ce_namelen; unsigned char sha1[20]; struct cache_entry *next; struct cache_entry *dir_next;@@ -205,15 +206,7 @@ static inline unsigned create_ce_flags(size_t len, unsigned stage) return (len | (stage << CE_STAGESHIFT)); } -static inline size_t ce_namelen(const struct cache_entry *ce) -{ - size_t len = ce->ce_flags & CE_NAMEMASK; - if (len < CE_NAMEMASK) - return len; - return strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK; -}
AFAICT almost all of the changes relate to the change from ce_namelen(ce)
to ce->ce_namelen. What's stopping you from defining
#define ce_namelen(ce) ((ce)->ce_namelen)
as a simple compatibility macro?
I'm not sure whether such internal backwards-compatibility is desirable,
but it would certainly make the patch much more reviewable. If Junio
would rather see the conversion, maybe do it in a separate patch that
simply substitutes the ce->ce_namelen for all the ce_namelen(ce)?
--
Thomas Rast
trast@{inf,student}.ethz.ch