[PATCH/RFC v2 2/7] remove some memcpy() and strchr() calls inside create_directories()
- From
- Kjetil Barvik <barvik@broadpark.no>
- Date
- Feb 1, 2009, 20:23 UTC
- Message-ID
- <f395c63c938c6be861614df6989a696a71599641.1233499703.git.barvik@broadpark.no>
- In-Reply-To
- <cover.1233499703.git.barvik@broadpark.no>
Remove the call to memcpy() and strchr() for each path component tested, and instead add each path component as we go forward inside the while-loop.
Impact: small optimisation
Signed-off-by: Kjetil Barvik <barvik@broadpark.no> ---
* Kjetil Barvik <barvik@broadpark.no> writes: | OK, maybe I instead should have tried to merge the function | create_directories() with the safe_create_leading_directories() and | *_const() functions? What do pepople think? * Junio C Hamano <gitster@pobox.com> writes | Strictly speaking, the safe_create_leading_* functions are meant to | work on paths inside $GIT_DIR and are not meant for paths inside the | work tree, which is this function is about. Their semantics are | different with respect to adjust_shared_perm().
Ok, I think I keep the create_directories() function as is. Also because I think a shared "create_directories()" function could maybe make the calls to the lstat_cache() function less likely to be sorted alphabetically, and therefore be less cache effective.
| I see existing (mis)uses of the safe_create_leading_* in | builtin-apply.c, builtin-clone.c (the one that creates the | work_tree, the other one is Ok), merge-recursive.c (both call sites) | that are used to touch the work tree, but all places that create | regular files in the work tree do not run adjust_shared_perm() but | simply honor the user's umask.
Inside update_file_flags() in merge-recursive.c I see a call to make_room_for_path() (if update_wd is non-zero) which calls safe_create_leading_directories_const().
If we get lots of calls to make_room_for_path() in some cases, maybe we will also save some stat() calls if we fix the call to safe_create_*() at this place?
Remember that safe_create_*() calls stat() on each path component to make sure that the component exists, same as create_directories() used to do before.
entry.c | 23 ++++++++++++++--------- 1 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/entry.c b/entry.c index 05aa58d..1879b83 100644 --- a/entry.c +++ b/entry.c @@ -2,15 +2,19 @@ #include "blob.h" #include "dir.h" -static void create_directories(const char *path, const struct checkout *state) +static void create_directories(const char *path, int path_len, + const struct checkout *state) { - int len = strlen(path); - char *buf = xmalloc(len + 1); - const char *slash = path; - - while ((slash = strchr(slash+1, '/')) != NULL) { - len = slash - path; - memcpy(buf, path, len); + char *buf = xmalloc(path_len + 1); + int len = 0; + + while (len < path_len) { + do { + buf[len] = path[len]; + len++; + } while (len < path_len && path[len] != '/'); + if (len >= path_len) + break; buf[len] = 0; /* @@ -190,6 +194,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t memcpy(path, state->base_dir, len); strcpy(path + len, ce->name); + len += ce_namelen(ce); if (!lstat(path, &st)) { unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID); @@ -218,6 +223,6 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t return error("unable to unlink old '%s' (%s)", path, strerror(errno)); } else if (state->not_new) return 0; - create_directories(path, state); + create_directories(path, len, state); return write_entry(ce, path, state, 0); }
-- 1.6.1.349.g99fa5