{"thread":{"id":"11671","subject":"Updated in-memory index cleanup","startedAt":"2008-01-19T03:25:58Z","lastAt":"2008-01-21T00:19:38Z","messageCount":26,"participants":["Linus Torvalds","Junio C Hamano","Johannes Schindelin","Steffen Prohaska","Brian Downing"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"65935","messageId":"alpine.LFD.1.00.0801181911560.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":null,"subject":"Updated in-memory index cleanup","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-19T03:25:58Z","receivedAt":"2008-01-19T03:25:58Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis is a slightly updated version of the patch I sent out a few days ago, \nwhich make the index use separate in-memory and on-disk representations \n(with the in-memory format being host-endian and having an extended flags \nfield).\n\nNot only has the patch been updated for current 'master', it also now \nhandles the \"remove entry\" condition with a ce_flags value (CE_REMOVE) to \nmirror our old use of \"update entry\" (CE_UPDATE).\n\nIt also makes both CE_REMOVE and CE_UPDATE be in the \"in-memory only\" part \nof ce_flags, so that they don't use up precios bits from the on-disk \nformat.\n\nI have been using some version of this patch since I sent it out \noriginally, so it has actually gotten some testing. The CE_REMOVE parts \nare new, but are pretty obvious cleanups and not subtle. It passes the \ntest-suite and yadda-yadda.\n\nThe patch isn't trivial, but the bulk of it is (still) of the obvious \ncleanup kind, ie things like the removal of ntohl/htonl (since it's now \ndone by the disk-format conversion at read/write time):\n\n\t-\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n\t+\tif (S_ISGITLINK(ce->ce_mode)) {\n\nand using CE_REMOVE instead of setting mode and stage to 0:\n\n\t-\t\t\tce->ce_mode = 0;\n\t-\t\t\tce->ce_flags &= ~htons(CE_STAGEMASK);\n\t+\t\t\tce->ce_flags |= CE_REMOVE;\n\nwith the fact that there are about 45 more lines added than removed coming \nmainly from the new definition of the on-disk/in-memory structures and the \nconversion routines to convert from one to the other.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n builtin-apply.c        |   10 +-\n builtin-blame.c        |    2 +-\n builtin-commit.c       |    2 +-\n builtin-fsck.c         |    2 +-\n builtin-grep.c         |    4 +-\n builtin-ls-files.c     |   10 +-\n builtin-read-tree.c    |    3 +-\n builtin-rerere.c       |    4 +-\n builtin-update-index.c |   18 ++--\n cache-tree.c           |    4 +-\n cache.h                |   46 ++++++++----\n diff-lib.c             |   32 ++++-----\n dir.c                  |    2 +-\n entry.c                |    6 +-\n merge-index.c          |    2 +-\n merge-recursive.c      |    2 +-\n reachable.c            |    2 +-\n read-cache.c           |  195 ++++++++++++++++++++++++++++--------------------\n sha1_name.c            |    2 +-\n tree.c                 |    4 +-\n unpack-trees.c         |   29 ++++----\n 21 files changed, 214 insertions(+), 167 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 15432b6..30d86f2 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1946,7 +1946,7 @@ static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n \tif (!ce)\n \t\treturn 0;\n \n-\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\tif (S_ISGITLINK(ce->ce_mode)) {\n \t\tstrbuf_grow(buf, 100);\n \t\tstrbuf_addf(buf, \"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n \t} else {\n@@ -2023,7 +2023,7 @@ static int check_to_create_blob(const char *new_name, int ok_if_exists)\n \n static int verify_index_match(struct cache_entry *ce, struct stat *st)\n {\n-\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\tif (S_ISGITLINK(ce->ce_mode)) {\n \t\tif (!S_ISDIR(st->st_mode))\n \t\t\treturn -1;\n \t\treturn 0;\n@@ -2082,12 +2082,12 @@ static int check_patch(struct patch *patch, struct patch *prev_patch)\n \t\t\t\treturn error(\"%s: does not match index\",\n \t\t\t\t\t     old_name);\n \t\t\tif (cached)\n-\t\t\t\tst_mode = ntohl(ce->ce_mode);\n+\t\t\t\tst_mode = ce->ce_mode;\n \t\t} else if (stat_ret < 0)\n \t\t\treturn error(\"%s: %s\", old_name, strerror(errno));\n \n \t\tif (!cached)\n-\t\t\tst_mode = ntohl(ce_mode_from_stat(ce, st.st_mode));\n+\t\t\tst_mode = ce_mode_from_stat(ce, st.st_mode);\n \n \t\tif (patch->is_new < 0)\n \t\t\tpatch->is_new = 0;\n@@ -2388,7 +2388,7 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n \tce = xcalloc(1, ce_size);\n \tmemcpy(ce->name, path, namelen);\n \tce->ce_mode = create_ce_mode(mode);\n-\tce->ce_flags = htons(namelen);\n+\tce->ce_flags = namelen;\n \tif (S_ISGITLINK(mode)) {\n \t\tconst char *s = buf;\n \ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex 9b4c02e..c7e6887 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -2092,7 +2092,7 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con\n \tif (!mode) {\n \t\tint pos = cache_name_pos(path, len);\n \t\tif (0 <= pos)\n-\t\t\tmode = ntohl(active_cache[pos]->ce_mode);\n+\t\t\tmode = active_cache[pos]->ce_mode;\n \t\telse\n \t\t\t/* Let's not bother reading from HEAD tree */\n \t\t\tmode = S_IFREG | 0644;\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 0227936..c63ff82 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -156,7 +156,7 @@ static int list_paths(struct path_list *list, const char *with_tree,\n \n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n-\t\tif (ce->ce_flags & htons(CE_UPDATE))\n+\t\tif (ce->ce_flags & CE_UPDATE)\n \t\t\tcontinue;\n \t\tif (!pathspec_match(pattern, m, ce->name, 0))\n \t\t\tcontinue;\ndiff --git a/builtin-fsck.c b/builtin-fsck.c\nindex 2a6e94d..6fc9525 100644\n--- a/builtin-fsck.c\n+++ b/builtin-fsck.c\n@@ -765,7 +765,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\t\tstruct blob *blob;\n \t\t\tstruct object *obj;\n \n-\t\t\tmode = ntohl(active_cache[i]->ce_mode);\n+\t\t\tmode = active_cache[i]->ce_mode;\n \t\t\tif (S_ISGITLINK(mode))\n \t\t\t\tcontinue;\n \t\t\tblob = lookup_blob(active_cache[i]->sha1);\ndiff --git a/builtin-grep.c b/builtin-grep.c\nindex 0d6cc73..9180b39 100644\n--- a/builtin-grep.c\n+++ b/builtin-grep.c\n@@ -331,7 +331,7 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tchar *name;\n \t\tint kept;\n-\t\tif (!S_ISREG(ntohl(ce->ce_mode)))\n+\t\tif (!S_ISREG(ce->ce_mode))\n \t\t\tcontinue;\n \t\tif (!pathspec_matches(paths, ce->name))\n \t\t\tcontinue;\n@@ -387,7 +387,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n \n \tfor (nr = 0; nr < active_nr; nr++) {\n \t\tstruct cache_entry *ce = active_cache[nr];\n-\t\tif (!S_ISREG(ntohl(ce->ce_mode)))\n+\t\tif (!S_ISREG(ce->ce_mode))\n \t\t\tcontinue;\n \t\tif (!pathspec_matches(paths, ce->name))\n \t\t\tcontinue;\ndiff --git a/builtin-ls-files.c b/builtin-ls-files.c\nindex 0f0ab2d..d56e33e 100644\n--- a/builtin-ls-files.c\n+++ b/builtin-ls-files.c\n@@ -189,7 +189,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)\n \t\treturn;\n \n \tif (tag && *tag && show_valid_bit &&\n-\t    (ce->ce_flags & htons(CE_VALID))) {\n+\t    (ce->ce_flags & CE_VALID)) {\n \t\tstatic char alttag[4];\n \t\tmemcpy(alttag, tag, 3);\n \t\tif (isalpha(tag[0]))\n@@ -210,7 +210,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)\n \t} else {\n \t\tprintf(\"%s%06o %s %d\\t\",\n \t\t       tag,\n-\t\t       ntohl(ce->ce_mode),\n+\t\t       ce->ce_mode,\n \t\t       abbrev ? find_unique_abbrev(ce->sha1,abbrev)\n \t\t\t\t: sha1_to_hex(ce->sha1),\n \t\t       ce_stage(ce));\n@@ -242,7 +242,7 @@ static void show_files(struct dir_struct *dir, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\tif (show_unmerged && !ce_stage(ce))\n \t\t\t\tcontinue;\n-\t\t\tif (ce->ce_flags & htons(CE_UPDATE))\n+\t\t\tif (ce->ce_flags & CE_UPDATE)\n \t\t\t\tcontinue;\n \t\t\tshow_ce_entry(ce_stage(ce) ? tag_unmerged : tag_cached, ce);\n \t\t}\n@@ -350,7 +350,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tif (!ce_stage(ce))\n \t\t\tcontinue;\n-\t\tce->ce_flags |= htons(CE_STAGEMASK);\n+\t\tce->ce_flags |= CE_STAGEMASK;\n \t}\n \n \tif (prefix) {\n@@ -379,7 +379,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)\n \t\t\t */\n \t\t\tif (last_stage0 &&\n \t\t\t    !strcmp(last_stage0->name, ce->name))\n-\t\t\t\tce->ce_flags |= htons(CE_UPDATE);\n+\t\t\t\tce->ce_flags |= CE_UPDATE;\n \t\t}\n \t}\n }\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex c0ea034..5785401 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -45,8 +45,7 @@ static int read_cache_unmerged(void)\n \t\t\t\tcontinue;\n \t\t\tcache_tree_invalidate_path(active_cache_tree, ce->name);\n \t\t\tlast = ce;\n-\t\t\tce->ce_mode = 0;\n-\t\t\tce->ce_flags &= ~htons(CE_STAGEMASK);\n+\t\t\tce->ce_flags |= CE_REMOVE;\n \t\t}\n \t\t*dst++ = ce;\n \t}\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex a9e3ebc..b0c17bd 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -149,8 +149,8 @@ static int find_conflict(struct path_list *conflict)\n \t\tif (ce_stage(e2) == 2 &&\n \t\t    ce_stage(e3) == 3 &&\n \t\t    ce_same_name(e2, e3) &&\n-\t\t    S_ISREG(ntohl(e2->ce_mode)) &&\n-\t\t    S_ISREG(ntohl(e3->ce_mode))) {\n+\t\t    S_ISREG(e2->ce_mode) &&\n+\t\t    S_ISREG(e3->ce_mode)) {\n \t\t\tpath_list_insert((const char *)e2->name, conflict);\n \t\t\ti++; /* skip over both #2 and #3 */\n \t\t}\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex c3a14c7..a8795d3 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -47,10 +47,10 @@ static int mark_valid(const char *path)\n \tif (0 <= pos) {\n \t\tswitch (mark_valid_only) {\n \t\tcase MARK_VALID:\n-\t\t\tactive_cache[pos]->ce_flags |= htons(CE_VALID);\n+\t\t\tactive_cache[pos]->ce_flags |= CE_VALID;\n \t\t\tbreak;\n \t\tcase UNMARK_VALID:\n-\t\t\tactive_cache[pos]->ce_flags &= ~htons(CE_VALID);\n+\t\t\tactive_cache[pos]->ce_flags &= ~CE_VALID;\n \t\t\tbreak;\n \t\t}\n \t\tcache_tree_invalidate_path(active_cache_tree, path);\n@@ -95,7 +95,7 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru\n \tsize = cache_entry_size(len);\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, len);\n-\tce->ce_flags = htons(len);\n+\tce->ce_flags = len;\n \tfill_stat_cache_info(ce, st);\n \tce->ce_mode = ce_mode_from_stat(old, st->st_mode);\n \n@@ -139,7 +139,7 @@ static int process_directory(const char *path, int len, struct stat *st)\n \t/* Exact match: file or existing gitlink */\n \tif (pos >= 0) {\n \t\tstruct cache_entry *ce = active_cache[pos];\n-\t\tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n+\t\tif (S_ISGITLINK(ce->ce_mode)) {\n \n \t\t\t/* Do nothing to the index if there is no HEAD! */\n \t\t\tif (resolve_gitlink_ref(path, \"HEAD\", sha1) < 0)\n@@ -183,7 +183,7 @@ static int process_file(const char *path, int len, struct stat *st)\n \tint pos = cache_name_pos(path, len);\n \tstruct cache_entry *ce = pos < 0 ? NULL : active_cache[pos];\n \n-\tif (ce && S_ISGITLINK(ntohl(ce->ce_mode)))\n+\tif (ce && S_ISGITLINK(ce->ce_mode))\n \t\treturn error(\"%s is already a gitlink, not replacing\", path);\n \n \treturn add_one_path(ce, path, len, st);\n@@ -226,7 +226,7 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n \tce->ce_flags = create_ce_flags(len, stage);\n \tce->ce_mode = create_ce_mode(mode);\n \tif (assume_unchanged)\n-\t\tce->ce_flags |= htons(CE_VALID);\n+\t\tce->ce_flags |= CE_VALID;\n \toption = allow_add ? ADD_CACHE_OK_TO_ADD : 0;\n \toption |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;\n \tif (add_cache_entry(ce, option))\n@@ -246,14 +246,14 @@ static void chmod_path(int flip, const char *path)\n \tif (pos < 0)\n \t\tgoto fail;\n \tce = active_cache[pos];\n-\tmode = ntohl(ce->ce_mode);\n+\tmode = ce->ce_mode;\n \tif (!S_ISREG(mode))\n \t\tgoto fail;\n \tswitch (flip) {\n \tcase '+':\n-\t\tce->ce_mode |= htonl(0111); break;\n+\t\tce->ce_mode |= 0111; break;\n \tcase '-':\n-\t\tce->ce_mode &= htonl(~0111); break;\n+\t\tce->ce_mode &= ~0111; break;\n \tdefault:\n \t\tgoto fail;\n \t}\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 50b3526..bfc95d2 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -320,13 +320,13 @@ static int update_one(struct cache_tree *it,\n \t\t}\n \t\telse {\n \t\t\tsha1 = ce->sha1;\n-\t\t\tmode = ntohl(ce->ce_mode);\n+\t\t\tmode = ce->ce_mode;\n \t\t\tentlen = pathlen - baselen;\n \t\t}\n \t\tif (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1))\n \t\t\treturn error(\"invalid object %s\", sha1_to_hex(sha1));\n \n-\t\tif (!ce->ce_mode)\n+\t\tif (ce->ce_flags & CE_REMOVE)\n \t\t\tcontinue; /* entry being removed */\n \n \t\tstrbuf_grow(&buffer, entlen + 100);\ndiff --git a/cache.h b/cache.h\nindex 549f4bb..4a054c5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -94,48 +94,66 @@ struct cache_time {\n  * We save the fields in big-endian order to allow using the\n  * index file over NFS transparently.\n  */\n+struct ondisk_cache_entry {\n+\tstruct cache_time ctime;\n+\tstruct cache_time mtime;\n+\tunsigned int dev;\n+\tunsigned int ino;\n+\tunsigned int mode;\n+\tunsigned int uid;\n+\tunsigned int gid;\n+\tunsigned int size;\n+\tunsigned char sha1[20];\n+\tunsigned short flags;\n+\tchar name[FLEX_ARRAY]; /* more */\n+};\n+\n struct cache_entry {\n-\tstruct cache_time ce_ctime;\n-\tstruct cache_time ce_mtime;\n+\tunsigned int ce_ctime;\n+\tunsigned int ce_mtime;\n \tunsigned int ce_dev;\n \tunsigned int ce_ino;\n \tunsigned int ce_mode;\n \tunsigned int ce_uid;\n \tunsigned int ce_gid;\n \tunsigned int ce_size;\n+\tunsigned int ce_flags;\n \tunsigned char sha1[20];\n-\tunsigned short ce_flags;\n \tchar name[FLEX_ARRAY]; /* more */\n };\n \n #define CE_NAMEMASK  (0x0fff)\n #define CE_STAGEMASK (0x3000)\n-#define CE_UPDATE    (0x4000)\n #define CE_VALID     (0x8000)\n #define CE_STAGESHIFT 12\n \n-#define create_ce_flags(len, stage) htons((len) | ((stage) << CE_STAGESHIFT))\n-#define ce_namelen(ce) (CE_NAMEMASK & ntohs((ce)->ce_flags))\n+/* In-memory only */\n+#define CE_UPDATE    (0x10000)\n+#define CE_REMOVE    (0x20000)\n+\n+#define create_ce_flags(len, stage) ((len) | ((stage) << CE_STAGESHIFT))\n+#define ce_namelen(ce) (CE_NAMEMASK & (ce)->ce_flags)\n #define ce_size(ce) cache_entry_size(ce_namelen(ce))\n-#define ce_stage(ce) ((CE_STAGEMASK & ntohs((ce)->ce_flags)) >> CE_STAGESHIFT)\n+#define ondisk_ce_size(ce) ondisk_cache_entry_size(ce_namelen(ce))\n+#define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)\n \n #define ce_permissions(mode) (((mode) & 0100) ? 0755 : 0644)\n static inline unsigned int create_ce_mode(unsigned int mode)\n {\n \tif (S_ISLNK(mode))\n-\t\treturn htonl(S_IFLNK);\n+\t\treturn S_IFLNK;\n \tif (S_ISDIR(mode) || S_ISGITLINK(mode))\n-\t\treturn htonl(S_IFGITLINK);\n-\treturn htonl(S_IFREG | ce_permissions(mode));\n+\t\treturn S_IFGITLINK;\n+\treturn S_IFREG | ce_permissions(mode);\n }\n static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned int mode)\n {\n \textern int trust_executable_bit, has_symlinks;\n \tif (!has_symlinks && S_ISREG(mode) &&\n-\t    ce && S_ISLNK(ntohl(ce->ce_mode)))\n+\t    ce && S_ISLNK(ce->ce_mode))\n \t\treturn ce->ce_mode;\n \tif (!trust_executable_bit && S_ISREG(mode)) {\n-\t\tif (ce && S_ISREG(ntohl(ce->ce_mode)))\n+\t\tif (ce && S_ISREG(ce->ce_mode))\n \t\t\treturn ce->ce_mode;\n \t\treturn create_ce_mode(0666);\n \t}\n@@ -146,14 +164,14 @@ static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned in\n \tS_ISLNK(mode) ? S_IFLNK : S_ISDIR(mode) ? S_IFDIR : S_IFGITLINK)\n \n #define cache_entry_size(len) ((offsetof(struct cache_entry,name) + (len) + 8) & ~7)\n+#define ondisk_cache_entry_size(len) ((offsetof(struct ondisk_cache_entry,name) + (len) + 8) & ~7)\n \n struct index_state {\n \tstruct cache_entry **cache;\n \tunsigned int cache_nr, cache_alloc, cache_changed;\n \tstruct cache_tree *cache_tree;\n \ttime_t timestamp;\n-\tvoid *mmap;\n-\tsize_t mmap_size;\n+\tvoid *alloc;\n };\n \n extern struct index_state the_index;\ndiff --git a/diff-lib.c b/diff-lib.c\nindex d85d8f3..4a05b02 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -37,7 +37,7 @@ static int get_mode(const char *path, int *mode)\n \tif (!path || !strcmp(path, \"/dev/null\"))\n \t\t*mode = 0;\n \telse if (!strcmp(path, \"-\"))\n-\t\t*mode = ntohl(create_ce_mode(0666));\n+\t\t*mode = create_ce_mode(0666);\n \telse if (stat(path, &st))\n \t\treturn error(\"Could not access '%s'\", path);\n \telse\n@@ -384,7 +384,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\t\tcontinue;\n \t\t\t}\n \t\t\telse\n-\t\t\t\tdpath->mode = ntohl(ce_mode_from_stat(ce, st.st_mode));\n+\t\t\t\tdpath->mode = ce_mode_from_stat(ce, st.st_mode);\n \n \t\t\twhile (i < entries) {\n \t\t\t\tstruct cache_entry *nce = active_cache[i];\n@@ -398,10 +398,10 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\t */\n \t\t\t\tstage = ce_stage(nce);\n \t\t\t\tif (2 <= stage) {\n-\t\t\t\t\tint mode = ntohl(nce->ce_mode);\n+\t\t\t\t\tint mode = nce->ce_mode;\n \t\t\t\t\tnum_compare_stages++;\n \t\t\t\t\thashcpy(dpath->parent[stage-2].sha1, nce->sha1);\n-\t\t\t\t\tdpath->parent[stage-2].mode = ntohl(ce_mode_from_stat(nce, mode));\n+\t\t\t\t\tdpath->parent[stage-2].mode = ce_mode_from_stat(nce, mode);\n \t\t\t\t\tdpath->parent[stage-2].status =\n \t\t\t\t\t\tDIFF_STATUS_MODIFIED;\n \t\t\t\t}\n@@ -442,15 +442,15 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t}\n \t\t\tif (silent_on_removed)\n \t\t\t\tcontinue;\n-\t\t\tdiff_addremove(&revs->diffopt, '-', ntohl(ce->ce_mode),\n+\t\t\tdiff_addremove(&revs->diffopt, '-', ce->ce_mode,\n \t\t\t\t       ce->sha1, ce->name, NULL);\n \t\t\tcontinue;\n \t\t}\n \t\tchanged = ce_match_stat(ce, &st, ce_option);\n \t\tif (!changed && !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))\n \t\t\tcontinue;\n-\t\toldmode = ntohl(ce->ce_mode);\n-\t\tnewmode = ntohl(ce_mode_from_stat(ce, st.st_mode));\n+\t\toldmode = ce->ce_mode;\n+\t\tnewmode = ce_mode_from_stat(ce, st.st_mode);\n \t\tdiff_change(&revs->diffopt, oldmode, newmode,\n \t\t\t    ce->sha1, (changed ? null_sha1 : ce->sha1),\n \t\t\t    ce->name, NULL);\n@@ -471,7 +471,7 @@ static void diff_index_show_file(struct rev_info *revs,\n \t\t\t\t struct cache_entry *ce,\n \t\t\t\t unsigned char *sha1, unsigned int mode)\n {\n-\tdiff_addremove(&revs->diffopt, prefix[0], ntohl(mode),\n+\tdiff_addremove(&revs->diffopt, prefix[0], mode,\n \t\t       sha1, ce->name, NULL);\n }\n \n@@ -550,14 +550,14 @@ static int show_modified(struct rev_info *revs,\n \t\tp->len = pathlen;\n \t\tmemcpy(p->path, new->name, pathlen);\n \t\tp->path[pathlen] = 0;\n-\t\tp->mode = ntohl(mode);\n+\t\tp->mode = mode;\n \t\thashclr(p->sha1);\n \t\tmemset(p->parent, 0, 2 * sizeof(struct combine_diff_parent));\n \t\tp->parent[0].status = DIFF_STATUS_MODIFIED;\n-\t\tp->parent[0].mode = ntohl(new->ce_mode);\n+\t\tp->parent[0].mode = new->ce_mode;\n \t\thashcpy(p->parent[0].sha1, new->sha1);\n \t\tp->parent[1].status = DIFF_STATUS_MODIFIED;\n-\t\tp->parent[1].mode = ntohl(old->ce_mode);\n+\t\tp->parent[1].mode = old->ce_mode;\n \t\thashcpy(p->parent[1].sha1, old->sha1);\n \t\tshow_combined_diff(p, 2, revs->dense_combined_merges, revs);\n \t\tfree(p);\n@@ -569,9 +569,6 @@ static int show_modified(struct rev_info *revs,\n \t    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))\n \t\treturn 0;\n \n-\tmode = ntohl(mode);\n-\toldmode = ntohl(oldmode);\n-\n \tdiff_change(&revs->diffopt, oldmode, mode,\n \t\t    old->sha1, sha1, old->name, NULL);\n \treturn 0;\n@@ -628,7 +625,7 @@ static int diff_cache(struct rev_info *revs,\n \t\t\t\t\t   cached, match_missing))\n \t\t\t\tbreak;\n \t\t\tdiff_unmerge(&revs->diffopt, ce->name,\n-\t\t\t\t     ntohl(ce->ce_mode), ce->sha1);\n+\t\t\t\t     ce->ce_mode, ce->sha1);\n \t\t\tbreak;\n \t\tcase 3:\n \t\t\tdiff_unmerge(&revs->diffopt, ce->name,\n@@ -664,7 +661,7 @@ static void mark_merge_entries(void)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tif (!ce_stage(ce))\n \t\t\tcontinue;\n-\t\tce->ce_flags |= htons(CE_STAGEMASK);\n+\t\tce->ce_flags |= CE_STAGEMASK;\n \t}\n }\n \n@@ -722,8 +719,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \t\t\tcache_tree_invalidate_path(active_cache_tree,\n \t\t\t\t\t\t   ce->name);\n \t\t\tlast = ce;\n-\t\t\tce->ce_mode = 0;\n-\t\t\tce->ce_flags &= ~htons(CE_STAGEMASK);\n+\t\t\tce->ce_flags |= CE_REMOVE;\n \t\t}\n \t\t*dst++ = ce;\n \t}\ndiff --git a/dir.c b/dir.c\nindex 3e345c2..1b9cc7a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -391,7 +391,7 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)\n \t\t\tbreak;\n \t\tif (endchar == '/')\n \t\t\treturn index_directory;\n-\t\tif (!endchar && S_ISGITLINK(ntohl(ce->ce_mode)))\n+\t\tif (!endchar && S_ISGITLINK(ce->ce_mode))\n \t\t\treturn index_gitdir;\n \t}\n \treturn index_nonexistent;\ndiff --git a/entry.c b/entry.c\nindex 257ab46..44f4b89 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -103,7 +103,7 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout\n \tint fd;\n \tlong wrote;\n \n-\tswitch (ntohl(ce->ce_mode) & S_IFMT) {\n+\tswitch (ce->ce_mode & S_IFMT) {\n \t\tchar *new;\n \t\tstruct strbuf buf;\n \t\tunsigned long size;\n@@ -129,7 +129,7 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout\n \t\t\tstrcpy(path, \".merge_file_XXXXXX\");\n \t\t\tfd = mkstemp(path);\n \t\t} else\n-\t\t\tfd = create_file(path, ntohl(ce->ce_mode));\n+\t\t\tfd = create_file(path, ce->ce_mode);\n \t\tif (fd < 0) {\n \t\t\tfree(new);\n \t\t\treturn error(\"git-checkout-index: unable to create file %s (%s)\",\n@@ -221,7 +221,7 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t\n \t\tunlink(path);\n \t\tif (S_ISDIR(st.st_mode)) {\n \t\t\t/* If it is a gitlink, leave it alone! */\n-\t\t\tif (S_ISGITLINK(ntohl(ce->ce_mode)))\n+\t\t\tif (S_ISGITLINK(ce->ce_mode))\n \t\t\t\treturn 0;\n \t\t\tif (!state->force)\n \t\t\t\treturn error(\"%s is a directory\", path);\ndiff --git a/merge-index.c b/merge-index.c\nindex fa719cb..bbb700b 100644\n--- a/merge-index.c\n+++ b/merge-index.c\n@@ -48,7 +48,7 @@ static int merge_entry(int pos, const char *path)\n \t\t\tbreak;\n \t\tfound++;\n \t\tstrcpy(hexbuf[stage], sha1_to_hex(ce->sha1));\n-\t\tsprintf(ownbuf[stage], \"%o\", ntohl(ce->ce_mode));\n+\t\tsprintf(ownbuf[stage], \"%o\", ce->ce_mode);\n \t\targuments[stage] = hexbuf[stage];\n \t\targuments[stage + 4] = ownbuf[stage];\n \t} while (++pos < active_nr);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c292a77..bdf03b1 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -333,7 +333,7 @@ static struct path_list *get_unmerged(void)\n \t\t\titem->util = xcalloc(1, sizeof(struct stage_data));\n \t\t}\n \t\te = item->util;\n-\t\te->stages[ce_stage(ce)].mode = ntohl(ce->ce_mode);\n+\t\te->stages[ce_stage(ce)].mode = ce->ce_mode;\n \t\thashcpy(e->stages[ce_stage(ce)].sha, ce->sha1);\n \t}\n \ndiff --git a/reachable.c b/reachable.c\nindex 6383401..00f289f 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -176,7 +176,7 @@ static void add_cache_refs(struct rev_info *revs)\n \t\t * lookup_blob() on them, to avoid populating the hash table\n \t\t * with invalid information\n \t\t */\n-\t\tif (S_ISGITLINK(ntohl(active_cache[i]->ce_mode)))\n+\t\tif (S_ISGITLINK(active_cache[i]->ce_mode))\n \t\t\tcontinue;\n \n \t\tlookup_blob(active_cache[i]->sha1);\ndiff --git a/read-cache.c b/read-cache.c\nindex 7db5588..f5f9c3d 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -30,20 +30,16 @@ struct index_state the_index;\n  */\n void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)\n {\n-\tce->ce_ctime.sec = htonl(st->st_ctime);\n-\tce->ce_mtime.sec = htonl(st->st_mtime);\n-#ifdef USE_NSEC\n-\tce->ce_ctime.nsec = htonl(st->st_ctim.tv_nsec);\n-\tce->ce_mtime.nsec = htonl(st->st_mtim.tv_nsec);\n-#endif\n-\tce->ce_dev = htonl(st->st_dev);\n-\tce->ce_ino = htonl(st->st_ino);\n-\tce->ce_uid = htonl(st->st_uid);\n-\tce->ce_gid = htonl(st->st_gid);\n-\tce->ce_size = htonl(st->st_size);\n+\tce->ce_ctime = st->st_ctime;\n+\tce->ce_mtime = st->st_mtime;\n+\tce->ce_dev = st->st_dev;\n+\tce->ce_ino = st->st_ino;\n+\tce->ce_uid = st->st_uid;\n+\tce->ce_gid = st->st_gid;\n+\tce->ce_size = st->st_size;\n \n \tif (assume_unchanged)\n-\t\tce->ce_flags |= htons(CE_VALID);\n+\t\tce->ce_flags |= CE_VALID;\n }\n \n static int ce_compare_data(struct cache_entry *ce, struct stat *st)\n@@ -116,7 +112,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)\n \t\t\treturn DATA_CHANGED;\n \t\tbreak;\n \tcase S_IFDIR:\n-\t\tif (S_ISGITLINK(ntohl(ce->ce_mode)))\n+\t\tif (S_ISGITLINK(ce->ce_mode))\n \t\t\treturn 0;\n \tdefault:\n \t\treturn TYPE_CHANGED;\n@@ -128,14 +124,14 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n {\n \tunsigned int changed = 0;\n \n-\tswitch (ntohl(ce->ce_mode) & S_IFMT) {\n+\tswitch (ce->ce_mode & S_IFMT) {\n \tcase S_IFREG:\n \t\tchanged |= !S_ISREG(st->st_mode) ? TYPE_CHANGED : 0;\n \t\t/* We consider only the owner x bit to be relevant for\n \t\t * \"mode changes\"\n \t\t */\n \t\tif (trust_executable_bit &&\n-\t\t    (0100 & (ntohl(ce->ce_mode) ^ st->st_mode)))\n+\t\t    (0100 & (ce->ce_mode ^ st->st_mode)))\n \t\t\tchanged |= MODE_CHANGED;\n \t\tbreak;\n \tcase S_IFLNK:\n@@ -152,29 +148,17 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \tcase 0: /* Special case: unmerged file in index */\n \t\treturn MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;\n \tdefault:\n-\t\tdie(\"internal error: ce_mode is %o\", ntohl(ce->ce_mode));\n+\t\tdie(\"internal error: ce_mode is %o\", ce->ce_mode);\n \t}\n-\tif (ce->ce_mtime.sec != htonl(st->st_mtime))\n+\tif (ce->ce_mtime != (unsigned int) st->st_mtime)\n \t\tchanged |= MTIME_CHANGED;\n-\tif (ce->ce_ctime.sec != htonl(st->st_ctime))\n+\tif (ce->ce_ctime != (unsigned int) st->st_ctime)\n \t\tchanged |= CTIME_CHANGED;\n \n-#ifdef USE_NSEC\n-\t/*\n-\t * nsec seems unreliable - not all filesystems support it, so\n-\t * as long as it is in the inode cache you get right nsec\n-\t * but after it gets flushed, you get zero nsec.\n-\t */\n-\tif (ce->ce_mtime.nsec != htonl(st->st_mtim.tv_nsec))\n-\t\tchanged |= MTIME_CHANGED;\n-\tif (ce->ce_ctime.nsec != htonl(st->st_ctim.tv_nsec))\n-\t\tchanged |= CTIME_CHANGED;\n-#endif\n-\n-\tif (ce->ce_uid != htonl(st->st_uid) ||\n-\t    ce->ce_gid != htonl(st->st_gid))\n+\tif (ce->ce_uid != (unsigned int) st->st_uid ||\n+\t    ce->ce_gid != (unsigned int) st->st_gid)\n \t\tchanged |= OWNER_CHANGED;\n-\tif (ce->ce_ino != htonl(st->st_ino))\n+\tif (ce->ce_ino != (unsigned int) st->st_ino)\n \t\tchanged |= INODE_CHANGED;\n \n #ifdef USE_STDEV\n@@ -183,11 +167,11 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)\n \t * clients will have different views of what \"device\"\n \t * the filesystem is on\n \t */\n-\tif (ce->ce_dev != htonl(st->st_dev))\n+\tif (ce->ce_dev != (unsigned int) st->st_dev)\n \t\tchanged |= INODE_CHANGED;\n #endif\n \n-\tif (ce->ce_size != htonl(st->st_size))\n+\tif (ce->ce_size != (unsigned int) st->st_size)\n \t\tchanged |= DATA_CHANGED;\n \n \treturn changed;\n@@ -205,7 +189,7 @@ int ie_match_stat(struct index_state *istate,\n \t * If it's marked as always valid in the index, it's\n \t * valid whatever the checked-out copy says.\n \t */\n-\tif (!ignore_valid && (ce->ce_flags & htons(CE_VALID)))\n+\tif (!ignore_valid && (ce->ce_flags & CE_VALID))\n \t\treturn 0;\n \n \tchanged = ce_match_stat_basic(ce, st);\n@@ -228,7 +212,7 @@ int ie_match_stat(struct index_state *istate,\n \t */\n \tif (!changed &&\n \t    istate->timestamp &&\n-\t    istate->timestamp <= ntohl(ce->ce_mtime.sec)) {\n+\t    istate->timestamp <= ce->ce_mtime) {\n \t\tif (assume_racy_is_modified)\n \t\t\tchanged |= DATA_CHANGED;\n \t\telse\n@@ -257,7 +241,7 @@ int ie_modified(struct index_state *istate,\n \t * the length field is zero.  For other cases the ce_size\n \t * should match the SHA1 recorded in the index entry.\n \t */\n-\tif ((changed & DATA_CHANGED) && ce->ce_size != htonl(0))\n+\tif ((changed & DATA_CHANGED) && ce->ce_size != 0)\n \t\treturn changed;\n \n \tchanged_fs = ce_modified_check_fs(ce, st);\n@@ -320,7 +304,7 @@ int index_name_pos(struct index_state *istate, const char *name, int namelen)\n \twhile (last > first) {\n \t\tint next = (last + first) >> 1;\n \t\tstruct cache_entry *ce = istate->cache[next];\n-\t\tint cmp = cache_name_compare(name, namelen, ce->name, ntohs(ce->ce_flags));\n+\t\tint cmp = cache_name_compare(name, namelen, ce->name, ce->ce_flags);\n \t\tif (!cmp)\n \t\t\treturn next;\n \t\tif (cmp < 0) {\n@@ -405,7 +389,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)\n \tsize = cache_entry_size(namelen);\n \tce = xcalloc(1, size);\n \tmemcpy(ce->name, path, namelen);\n-\tce->ce_flags = htons(namelen);\n+\tce->ce_flags = namelen;\n \tfill_stat_cache_info(ce, &st);\n \n \tif (trust_executable_bit && has_symlinks)\n@@ -583,7 +567,7 @@ static int has_file_name(struct index_state *istate,\n \t\t\tcontinue;\n \t\tif (p->name[len] != '/')\n \t\t\tcontinue;\n-\t\tif (!ce_stage(p) && !p->ce_mode)\n+\t\tif (p->ce_flags & CE_REMOVE)\n \t\t\tcontinue;\n \t\tretval = -1;\n \t\tif (!ok_to_replace)\n@@ -616,7 +600,7 @@ static int has_dir_name(struct index_state *istate,\n \t\t}\n \t\tlen = slash - name;\n \n-\t\tpos = index_name_pos(istate, name, ntohs(create_ce_flags(len, stage)));\n+\t\tpos = index_name_pos(istate, name, create_ce_flags(len, stage));\n \t\tif (pos >= 0) {\n \t\t\t/*\n \t\t\t * Found one, but not so fast.  This could\n@@ -679,7 +663,7 @@ static int check_file_directory_conflict(struct index_state *istate,\n \t/*\n \t * When ce is an \"I am going away\" entry, we allow it to be added\n \t */\n-\tif (!ce_stage(ce) && !ce->ce_mode)\n+\tif (ce->ce_flags & CE_REMOVE)\n \t\treturn 0;\n \n \t/*\n@@ -704,7 +688,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \tint skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;\n \n \tcache_tree_invalidate_path(istate->cache_tree, ce->name);\n-\tpos = index_name_pos(istate, ce->name, ntohs(ce->ce_flags));\n+\tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n \n \t/* existing match? Just replace it. */\n \tif (pos >= 0) {\n@@ -736,7 +720,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e\n \t\tif (!ok_to_replace)\n \t\t\treturn error(\"'%s' appears as both a file and as a directory\",\n \t\t\t\t     ce->name);\n-\t\tpos = index_name_pos(istate, ce->name, ntohs(ce->ce_flags));\n+\t\tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n \t\tpos = -pos-1;\n \t}\n \treturn pos + 1;\n@@ -810,7 +794,7 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \t\t * valid again, under \"assume unchanged\" mode.\n \t\t */\n \t\tif (ignore_valid && assume_unchanged &&\n-\t\t    !(ce->ce_flags & htons(CE_VALID)))\n+\t\t    !(ce->ce_flags & CE_VALID))\n \t\t\t; /* mark this one VALID again */\n \t\telse\n \t\t\treturn ce;\n@@ -826,7 +810,6 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \tupdated = xmalloc(size);\n \tmemcpy(updated, ce, size);\n \tfill_stat_cache_info(updated, &st);\n-\n \t/*\n \t * If ignore_valid is not set, we should leave CE_VALID bit\n \t * alone.  Otherwise, paths marked with --no-assume-unchanged\n@@ -834,8 +817,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \t * automatically, which is not really what we want.\n \t */\n \tif (!ignore_valid && assume_unchanged &&\n-\t    !(ce->ce_flags & htons(CE_VALID)))\n-\t\tupdated->ce_flags &= ~htons(CE_VALID);\n+\t    !(ce->ce_flags & CE_VALID))\n+\t\tupdated->ce_flags &= ~CE_VALID;\n \n \treturn updated;\n }\n@@ -880,7 +863,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p\n \t\t\t\t/* If we are doing --really-refresh that\n \t\t\t\t * means the index is not valid anymore.\n \t\t\t\t */\n-\t\t\t\tce->ce_flags &= ~htons(CE_VALID);\n+\t\t\t\tce->ce_flags &= ~CE_VALID;\n \t\t\t\tistate->cache_changed = 1;\n \t\t\t}\n \t\t\tif (quiet)\n@@ -942,16 +925,34 @@ int read_index(struct index_state *istate)\n \treturn read_index_from(istate, get_index_file());\n }\n \n+static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_entry *ce)\n+{\n+\tce->ce_ctime = ntohl(ondisk->ctime.sec);\n+\tce->ce_mtime = ntohl(ondisk->mtime.sec);\n+\tce->ce_dev   = ntohl(ondisk->dev);\n+\tce->ce_ino   = ntohl(ondisk->ino);\n+\tce->ce_mode  = ntohl(ondisk->mode);\n+\tce->ce_uid   = ntohl(ondisk->uid);\n+\tce->ce_gid   = ntohl(ondisk->gid);\n+\tce->ce_size  = ntohl(ondisk->size);\n+\t/* On-disk flags are just 16 bits */\n+\tce->ce_flags = ntohs(ondisk->flags);\n+\thashcpy(ce->sha1, ondisk->sha1);\n+\tmemcpy(ce->name, ondisk->name, ce_namelen(ce)+1);\n+}\n+\n /* remember to discard_cache() before reading a different cache! */\n int read_index_from(struct index_state *istate, const char *path)\n {\n \tint fd, i;\n \tstruct stat st;\n-\tunsigned long offset;\n+\tunsigned long src_offset, dst_offset;\n \tstruct cache_header *hdr;\n+\tvoid *mmap;\n+\tsize_t mmap_size;\n \n \terrno = EBUSY;\n-\tif (istate->mmap)\n+\tif (istate->alloc)\n \t\treturn istate->cache_nr;\n \n \terrno = ENOENT;\n@@ -967,31 +968,47 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\tdie(\"cannot stat the open index (%s)\", strerror(errno));\n \n \terrno = EINVAL;\n-\tistate->mmap_size = xsize_t(st.st_size);\n-\tif (istate->mmap_size < sizeof(struct cache_header) + 20)\n+\tmmap_size = xsize_t(st.st_size);\n+\tif (mmap_size < sizeof(struct cache_header) + 20)\n \t\tdie(\"index file smaller than expected\");\n \n-\tistate->mmap = xmmap(NULL, istate->mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);\n+\tmmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n+\tif (mmap == MAP_FAILED)\n+\t\tdie(\"unable to map index file\");\n \n-\thdr = istate->mmap;\n-\tif (verify_hdr(hdr, istate->mmap_size) < 0)\n+\thdr = mmap;\n+\tif (verify_hdr(hdr, mmap_size) < 0)\n \t\tgoto unmap;\n \n \tistate->cache_nr = ntohl(hdr->hdr_entries);\n \tistate->cache_alloc = alloc_nr(istate->cache_nr);\n \tistate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *));\n \n-\toffset = sizeof(*hdr);\n+\t/*\n+\t * The disk format is actually larger than the in-memory format,\n+\t * due to space for nsec etc, so even though the in-memory one\n+\t * has room for a few  more flags, we can allocate using the same\n+\t * index size\n+\t */\n+\tistate->alloc = xmalloc(mmap_size);\n+\n+\tsrc_offset = sizeof(*hdr);\n+\tdst_offset = 0;\n \tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct ondisk_cache_entry *disk_ce;\n \t\tstruct cache_entry *ce;\n \n-\t\tce = (struct cache_entry *)((char *)(istate->mmap) + offset);\n-\t\toffset = offset + ce_size(ce);\n+\t\tdisk_ce = (struct ondisk_cache_entry *)((char *)mmap + src_offset);\n+\t\tce = (struct cache_entry *)((char *)istate->alloc + dst_offset);\n+\t\tconvert_from_disk(disk_ce, ce);\n \t\tistate->cache[i] = ce;\n+\n+\t\tsrc_offset += ondisk_ce_size(ce);\n+\t\tdst_offset += ce_size(ce);\n \t}\n \tistate->timestamp = st.st_mtime;\n-\twhile (offset <= istate->mmap_size - 20 - 8) {\n+\twhile (src_offset <= mmap_size - 20 - 8) {\n \t\t/* After an array of active_nr index entries,\n \t\t * there can be arbitrary number of extended\n \t\t * sections, each of which is prefixed with\n@@ -999,40 +1016,36 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\t * in 4-byte network byte order.\n \t\t */\n \t\tunsigned long extsize;\n-\t\tmemcpy(&extsize, (char *)(istate->mmap) + offset + 4, 4);\n+\t\tmemcpy(&extsize, (char *)mmap + src_offset + 4, 4);\n \t\textsize = ntohl(extsize);\n \t\tif (read_index_extension(istate,\n-\t\t\t\t\t ((const char *) (istate->mmap)) + offset,\n-\t\t\t\t\t (char *) (istate->mmap) + offset + 8,\n+\t\t\t\t\t (const char *) mmap + src_offset,\n+\t\t\t\t\t (char *) mmap + src_offset + 8,\n \t\t\t\t\t extsize) < 0)\n \t\t\tgoto unmap;\n-\t\toffset += 8;\n-\t\toffset += extsize;\n+\t\tsrc_offset += 8;\n+\t\tsrc_offset += extsize;\n \t}\n+\tmunmap(mmap, mmap_size);\n \treturn istate->cache_nr;\n \n unmap:\n-\tmunmap(istate->mmap, istate->mmap_size);\n+\tmunmap(mmap, mmap_size);\n \terrno = EINVAL;\n \tdie(\"index file corrupt\");\n }\n \n int discard_index(struct index_state *istate)\n {\n-\tint ret;\n-\n \tistate->cache_nr = 0;\n \tistate->cache_changed = 0;\n \tistate->timestamp = 0;\n \tcache_tree_free(&(istate->cache_tree));\n-\tif (istate->mmap == NULL)\n-\t\treturn 0;\n-\tret = munmap(istate->mmap, istate->mmap_size);\n-\tistate->mmap = NULL;\n-\tistate->mmap_size = 0;\n+\tfree(istate->alloc);\n+\tistate->alloc = NULL;\n \n \t/* no need to throw away allocated active_cache */\n-\treturn ret;\n+\treturn 0;\n }\n \n #define WRITE_BUFFER_SIZE 8192\n@@ -1144,10 +1157,32 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)\n \t\t * file, and never calls us, so the cached size information\n \t\t * for \"frotz\" stays 6 which does not match the filesystem.\n \t\t */\n-\t\tce->ce_size = htonl(0);\n+\t\tce->ce_size = 0;\n \t}\n }\n \n+static int ce_write_entry(SHA_CTX *c, int fd, struct cache_entry *ce)\n+{\n+\tint size = ondisk_ce_size(ce);\n+\tstruct ondisk_cache_entry *ondisk = xcalloc(1, size);\n+\n+\tondisk->ctime.sec = htonl(ce->ce_ctime);\n+\tondisk->ctime.nsec = 0;\n+\tondisk->mtime.sec = htonl(ce->ce_mtime);\n+\tondisk->mtime.nsec = 0;\n+\tondisk->dev  = htonl(ce->ce_dev);\n+\tondisk->ino  = htonl(ce->ce_ino);\n+\tondisk->mode = htonl(ce->ce_mode);\n+\tondisk->uid  = htonl(ce->ce_uid);\n+\tondisk->gid  = htonl(ce->ce_gid);\n+\tondisk->size = htonl(ce->ce_size);\n+\thashcpy(ondisk->sha1, ce->sha1);\n+\tondisk->flags = htons(ce->ce_flags);\n+\tmemcpy(ondisk->name, ce->name, ce_namelen(ce));\n+\n+\treturn ce_write(c, fd, ondisk, size);\n+}\n+\n int write_index(struct index_state *istate, int newfd)\n {\n \tSHA_CTX c;\n@@ -1157,7 +1192,7 @@ int write_index(struct index_state *istate, int newfd)\n \tint entries = istate->cache_nr;\n \n \tfor (i = removed = 0; i < entries; i++)\n-\t\tif (!cache[i]->ce_mode)\n+\t\tif (cache[i]->ce_flags & CE_REMOVE)\n \t\t\tremoved++;\n \n \thdr.hdr_signature = htonl(CACHE_SIGNATURE);\n@@ -1170,12 +1205,12 @@ int write_index(struct index_state *istate, int newfd)\n \n \tfor (i = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n-\t\tif (!ce->ce_mode)\n+\t\tif (ce->ce_flags & CE_REMOVE)\n \t\t\tcontinue;\n \t\tif (istate->timestamp &&\n-\t\t    istate->timestamp <= ntohl(ce->ce_mtime.sec))\n+\t\t    istate->timestamp <= ce->ce_mtime)\n \t\t\tce_smudge_racily_clean_entry(ce);\n-\t\tif (ce_write(&c, newfd, ce, ce_size(ce)) < 0)\n+\t\tif (ce_write_entry(&c, newfd, ce) < 0)\n \t\t\treturn -1;\n \t}\n \ndiff --git a/sha1_name.c b/sha1_name.c\nindex 13e1164..be8489e 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -695,7 +695,7 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)\n \t\t\t\tbreak;\n \t\t\tif (ce_stage(ce) == stage) {\n \t\t\t\thashcpy(sha1, ce->sha1);\n-\t\t\t\t*mode = ntohl(ce->ce_mode);\n+\t\t\t\t*mode = ce->ce_mode;\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\tpos++;\ndiff --git a/tree.c b/tree.c\nindex 8c0819f..87708ef 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -142,8 +142,8 @@ static int cmp_cache_name_compare(const void *a_, const void *b_)\n \n \tce1 = *((const struct cache_entry **)a_);\n \tce2 = *((const struct cache_entry **)b_);\n-\treturn cache_name_compare(ce1->name, ntohs(ce1->ce_flags),\n-\t\t\t\t  ce2->name, ntohs(ce2->ce_flags));\n+\treturn cache_name_compare(ce1->name, ce1->ce_flags,\n+\t\t\t\t  ce2->name, ce2->ce_flags);\n }\n \n int read_tree(struct tree *tree, int stage, const char **match)\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex aa2513e..ff46fd6 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -289,7 +289,6 @@ static struct checkout state;\n static void check_updates(struct cache_entry **src, int nr,\n \t\t\tstruct unpack_trees_options *o)\n {\n-\tunsigned short mask = htons(CE_UPDATE);\n \tunsigned cnt = 0, total = 0;\n \tstruct progress *progress = NULL;\n \tchar last_symlink[PATH_MAX];\n@@ -297,7 +296,7 @@ static void check_updates(struct cache_entry **src, int nr,\n \tif (o->update && o->verbose_update) {\n \t\tfor (total = cnt = 0; cnt < nr; cnt++) {\n \t\t\tstruct cache_entry *ce = src[cnt];\n-\t\t\tif (!ce->ce_mode || ce->ce_flags & mask)\n+\t\t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\t\ttotal++;\n \t\t}\n \n@@ -310,15 +309,15 @@ static void check_updates(struct cache_entry **src, int nr,\n \twhile (nr--) {\n \t\tstruct cache_entry *ce = *src++;\n \n-\t\tif (!ce->ce_mode || ce->ce_flags & mask)\n+\t\tif (ce->ce_flags & (CE_UPDATE | CE_REMOVE))\n \t\t\tdisplay_progress(progress, ++cnt);\n-\t\tif (!ce->ce_mode) {\n+\t\tif (ce->ce_flags & CE_REMOVE) {\n \t\t\tif (o->update)\n \t\t\t\tunlink_entry(ce->name, last_symlink);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (ce->ce_flags & mask) {\n-\t\t\tce->ce_flags &= ~mask;\n+\t\tif (ce->ce_flags & CE_UPDATE) {\n+\t\t\tce->ce_flags &= ~CE_UPDATE;\n \t\t\tif (o->update) {\n \t\t\t\tcheckout_entry(ce, &state, NULL);\n \t\t\t\t*last_symlink = '\\0';\n@@ -408,7 +407,7 @@ static void verify_uptodate(struct cache_entry *ce,\n \t\t * submodules that are marked to be automatically\n \t\t * checked out.\n \t\t */\n-\t\tif (S_ISGITLINK(ntohl(ce->ce_mode)))\n+\t\tif (S_ISGITLINK(ce->ce_mode))\n \t\t\treturn;\n \t\terrno = 0;\n \t}\n@@ -450,7 +449,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \tint cnt = 0;\n \tunsigned char sha1[20];\n \n-\tif (S_ISGITLINK(ntohl(ce->ce_mode)) &&\n+\tif (S_ISGITLINK(ce->ce_mode) &&\n \t    resolve_gitlink_ref(ce->name, \"HEAD\", sha1) == 0) {\n \t\t/* If we are not going to update the submodule, then\n \t\t * we don't care.\n@@ -481,7 +480,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,\n \t\t */\n \t\tif (!ce_stage(ce)) {\n \t\t\tverify_uptodate(ce, o);\n-\t\t\tce->ce_mode = 0;\n+\t\t\tce->ce_flags |= CE_REMOVE;\n \t\t}\n \t\tcnt++;\n \t}\n@@ -568,7 +567,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,\n \t\tcnt = cache_name_pos(ce->name, strlen(ce->name));\n \t\tif (0 <= cnt) {\n \t\t\tstruct cache_entry *ce = active_cache[cnt];\n-\t\t\tif (!ce_stage(ce) && !ce->ce_mode)\n+\t\t\tif (ce->ce_flags & CE_REMOVE)\n \t\t\t\treturn;\n \t\t}\n \n@@ -580,7 +579,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,\n static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tstruct unpack_trees_options *o)\n {\n-\tmerge->ce_flags |= htons(CE_UPDATE);\n+\tmerge->ce_flags |= CE_UPDATE;\n \tif (old) {\n \t\t/*\n \t\t * See if we can re-use the old CE directly?\n@@ -601,7 +600,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,\n \t\tinvalidate_ce_path(merge);\n \t}\n \n-\tmerge->ce_flags &= ~htons(CE_STAGEMASK);\n+\tmerge->ce_flags &= ~CE_STAGEMASK;\n \tadd_cache_entry(merge, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \treturn 1;\n }\n@@ -613,7 +612,7 @@ static int deleted_entry(struct cache_entry *ce, struct cache_entry *old,\n \t\tverify_uptodate(old, o);\n \telse\n \t\tverify_absent(ce, \"removed\", o);\n-\tce->ce_mode = 0;\n+\tce->ce_flags |= CE_REMOVE;\n \tadd_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);\n \tinvalidate_ce_path(ce);\n \treturn 1;\n@@ -634,7 +633,7 @@ static void show_stage_entry(FILE *o,\n \telse\n \t\tfprintf(o, \"%s%06o %s %d\\t%s\\n\",\n \t\t\tlabel,\n-\t\t\tntohl(ce->ce_mode),\n+\t\t\tce->ce_mode,\n \t\t\tsha1_to_hex(ce->sha1),\n \t\t\tce_stage(ce),\n \t\t\tce->name);\n@@ -920,7 +919,7 @@ int oneway_merge(struct cache_entry **src,\n \t\t\tstruct stat st;\n \t\t\tif (lstat(old->name, &st) ||\n \t\t\t    ce_match_stat(old, &st, CE_MATCH_IGNORE_VALID))\n-\t\t\t\told->ce_flags |= htons(CE_UPDATE);\n+\t\t\t\told->ce_flags |= CE_UPDATE;\n \t\t}\n \t\treturn keep_entry(old, o);\n \t}\n"},{"id":"65941","messageId":"7vk5m69s8n.fsf@gitster.siamese.dyndns.org","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801181911560.2957@woody.linux-foundation.org","subject":"[PATCH] index: be careful when handling long names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-19T07:42:00Z","receivedAt":"2008-01-19T07:42:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We currently use lower 12-bit (masked with CE_NAMEMASK) in the\nce_flags field to store the length of the name in cache_entry,\nwithout checking the length parameter given to\ncreate_ce_flags().  This can make us store incorrect length.\n\nCurrently we are mostly protected by the fact that many\ncodepaths first copy the path in a variable of size PATH_MAX,\nwhich typically is 4096 that happens to match the limit, but\nthat feels like a bug waiting to happen.  Besides, that would\nnot allow us to shorten the width of CE_NAMEMASK to use the bits\nfor new flags.\n\nThis redefines the meaning of the name length stored in the\ncache_entry.  A name that does not fit is represented by storing\nCE_NAMEMASK in the field, and the actual length needs to be\ncomputed by actually counting the bytes in the name[] field.\nThis way, only the unusually long paths need to suffer.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is rebased on your \"Updated in-memory index\" patch.\n\n   When we read on-disk index, I fear that we trust the name\n   length a bit too much.  With or without this patch, you could\n   craft a malicious index file that records a namelen that is\n   longer than the real string name length for the last entry to\n   cause us to go beyond the mmaped area.  Maybe we would want\n   to make sure that (1) the name lengths are sane; (2) names\n   have NUL at the place we expect them to be; and (3) names are\n   sorted in the proper cache order, while we iterate over the\n   ondisk structure and convert it to incore structure.\n\n cache.h          |   17 +++++++++++++++--\n read-cache.c     |   12 +++++++++++-\n t/t0000-basic.sh |   20 ++++++++++++++++++++\n 3 files changed, 46 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4a054c5..9eaffde 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -131,8 +131,21 @@ struct cache_entry {\n #define CE_UPDATE    (0x10000)\n #define CE_REMOVE    (0x20000)\n \n-#define create_ce_flags(len, stage) ((len) | ((stage) << CE_STAGESHIFT))\n-#define ce_namelen(ce) (CE_NAMEMASK & (ce)->ce_flags)\n+static inline unsigned create_ce_flags(size_t len, unsigned stage)\n+{\n+\tif (len >= CE_NAMEMASK)\n+\t\tlen = CE_NAMEMASK;\n+\treturn (len | (stage << CE_STAGESHIFT));\n+}\n+\n+static inline size_t ce_namelen(const struct cache_entry *ce)\n+{\n+\tsize_t len = ce->ce_flags & CE_NAMEMASK;\n+\tif (len < CE_NAMEMASK)\n+\t\treturn len;\n+\treturn strlen(ce->name + CE_NAMEMASK) + CE_NAMEMASK;\n+}\n+\n #define ce_size(ce) cache_entry_size(ce_namelen(ce))\n #define ondisk_ce_size(ce) ondisk_cache_entry_size(ce_namelen(ce))\n #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)\ndiff --git a/read-cache.c b/read-cache.c\nindex f5f9c3d..f98f57c 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -927,6 +927,8 @@ int read_index(struct index_state *istate)\n \n static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_entry *ce)\n {\n+\tsize_t len;\n+\n \tce->ce_ctime = ntohl(ondisk->ctime.sec);\n \tce->ce_mtime = ntohl(ondisk->mtime.sec);\n \tce->ce_dev   = ntohl(ondisk->dev);\n@@ -938,7 +940,15 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en\n \t/* On-disk flags are just 16 bits */\n \tce->ce_flags = ntohs(ondisk->flags);\n \thashcpy(ce->sha1, ondisk->sha1);\n-\tmemcpy(ce->name, ondisk->name, ce_namelen(ce)+1);\n+\n+\tlen = ce->ce_flags & CE_NAMEMASK;\n+\tif (len == CE_NAMEMASK)\n+\t\tlen = strlen(ondisk->name);\n+\t/*\n+\t * NEEDSWORK: If the original index is crafted, this copy could\n+\t * go unchecked.\n+\t */\n+\tmemcpy(ce->name, ondisk->name, len + 1);\n }\n \n /* remember to discard_cache() before reading a different cache! */\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 4e49d59..9f84b8d 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -297,4 +297,24 @@ test_expect_success 'absolute path works as expected' '\n \ttest \"$sym\" = \"$(test-absolute-path $dir2/syml)\"\n '\n \n+test_expect_success 'very long name in the index handled sanely' '\n+\n+\ta=a && # 1\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 16\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 256\n+\ta=$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a$a && # 4096\n+\ta=${a}q &&\n+\n+\t>path4 &&\n+\tgit update-index --add path4 &&\n+\t(\n+\t\tgit ls-files -s path4 |\n+\t\tsed -e \"s/\t.*/\t/\" |\n+\t\ttr -d \"\\012\"\n+\t\techo \"$a\"\n+\t) | git update-index --index-info &&\n+\tlen=$(git ls-files \"a*\" | wc -c) &&\n+\ttest $len = 4098\n+'\n+\n test_done\n-- \n1.5.4.rc3.37.gfdcf3\n"},{"id":"65942","messageId":"7vfxwu9s2z.fsf@gitster.siamese.dyndns.org","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801181911560.2957@woody.linux-foundation.org","subject":"[PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-19T07:45:24Z","receivedAt":"2008-01-19T07:45:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aside from the lstat(2) done for work tree files, there are\nquite many lstat(2) calls in refname dwimming codepath.  This\npatch is not about reducing them.\n\n * It adds a new ce_flag, CE_UPTODATE, that is meant to mark the\n   cache entries that record a regular file blob that is up to\n   date in the work tree.  If somebody later walks the index and\n   wants to see if the work tree has changes, they do not have\n   to be checked with lstat(2) again.\n\n * fill_stat_cache_info() marks the cache entry it just added\n   with CE_UPTODATE.  This has the effect of marking the paths\n   we write out of the index and lstat(2) immediately as \"no\n   need to lstat -- we know it is up-to-date\", from quite a lot\n   fo callers:\n\n    - git-apply --index\n    - git-update-index\n    - git-checkout-index\n    - git-add (uses add_file_to_index())\n    - git-commit (ditto)\n    - git-mv (ditto)\n\n * refresh_cache_ent() also marks the cache entry that are clean\n   with CE_UPTODATE.\n\n * write_index is changed not to write CE_UPTODATE out to the\n   index file, because CE_UPTODATE is meant to be transient only\n   in core.  For the same reason, CE_UPDATE is not written to\n   prevent an accident from happening.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Again, this is a rebase on top of your \"Updated in-memory\n   index\".  \"echo >>Makefile && git commit -m test Makefile\" in\n   the kernel repository does 23k lstat(2) with this patch, 46k\n   without.\n\n cache.h      |    3 +++\n diff.c       |   23 ++++++++++++++---------\n read-cache.c |   16 +++++++++++++++-\n 3 files changed, 32 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 9eaffde..3a47cdc 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -130,6 +130,7 @@ struct cache_entry {\n /* In-memory only */\n #define CE_UPDATE    (0x10000)\n #define CE_REMOVE    (0x20000)\n+#define CE_UPTODATE  (0x40000)\n \n static inline unsigned create_ce_flags(size_t len, unsigned stage)\n {\n@@ -149,6 +150,8 @@ static inline size_t ce_namelen(const struct cache_entry *ce)\n #define ce_size(ce) cache_entry_size(ce_namelen(ce))\n #define ondisk_ce_size(ce) ondisk_cache_entry_size(ce_namelen(ce))\n #define ce_stage(ce) ((CE_STAGEMASK & (ce)->ce_flags) >> CE_STAGESHIFT)\n+#define ce_uptodate(ce) ((ce)->ce_flags & CE_UPTODATE)\n+#define ce_mark_uptodate(ce) ((ce)->ce_flags |= CE_UPTODATE)\n \n #define ce_permissions(mode) (((mode) & 0100) ? 0755 : 0644)\n static inline unsigned int create_ce_mode(unsigned int mode)\ndiff --git a/diff.c b/diff.c\nindex 5b8afdc..d464fe3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1510,17 +1510,22 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int\n \tif (pos < 0)\n \t\treturn 0;\n \tce = active_cache[pos];\n-\tif ((lstat(name, &st) < 0) ||\n-\t    !S_ISREG(st.st_mode) || /* careful! */\n-\t    ce_match_stat(ce, &st, 0) ||\n-\t    hashcmp(sha1, ce->sha1))\n+\n+\t/*\n+\t * This is not the sha1 we are looking for, or\n+\t * unreusable because it is not a regular file.\n+\t */\n+\tif (hashcmp(sha1, ce->sha1) || !S_ISREG(ce->ce_mode))\n \t\treturn 0;\n-\t/* we return 1 only when we can stat, it is a regular file,\n-\t * stat information matches, and sha1 recorded in the cache\n-\t * matches.  I.e. we know the file in the work tree really is\n-\t * the same as the <name, sha1> pair.\n+\n+\t/*\n+\t * If ce matches the file in the work tree, we can reuse it.\n \t */\n-\treturn 1;\n+\tif (ce_uptodate(ce) ||\n+\t    (!lstat(name, &st) && !ce_match_stat(ce, &st, 0)))\n+\t\treturn 1;\n+\n+\treturn 0;\n }\n \n static int populate_from_stdin(struct diff_filespec *s)\ndiff --git a/read-cache.c b/read-cache.c\nindex f98f57c..6165885 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -40,6 +40,9 @@ void fill_stat_cache_info(struct cache_entry *ce, struct stat *st)\n \n \tif (assume_unchanged)\n \t\tce->ce_flags |= CE_VALID;\n+\n+\tif (S_ISREG(st->st_mode))\n+\t\tce_mark_uptodate(ce);\n }\n \n static int ce_compare_data(struct cache_entry *ce, struct stat *st)\n@@ -411,6 +414,7 @@ int add_file_to_index(struct index_state *istate, const char *path, int verbose)\n \t    !ie_match_stat(istate, istate->cache[pos], &st, ce_option)) {\n \t\t/* Nothing changed, really */\n \t\tfree(ce);\n+\t\tce_mark_uptodate(istate->cache[pos]);\n \t\treturn 0;\n \t}\n \n@@ -778,6 +782,9 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \tint changed, size;\n \tint ignore_valid = options & CE_MATCH_IGNORE_VALID;\n \n+\tif (ce_uptodate(ce))\n+\t\treturn ce;\n+\n \tif (lstat(ce->name, &st) < 0) {\n \t\tif (err)\n \t\t\t*err = errno;\n@@ -796,8 +803,15 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,\n \t\tif (ignore_valid && assume_unchanged &&\n \t\t    !(ce->ce_flags & CE_VALID))\n \t\t\t; /* mark this one VALID again */\n-\t\telse\n+\t\telse {\n+\t\t\t/*\n+\t\t\t * We do not mark the index itself \"modified\"\n+\t\t\t * because CE_UPTODATE flag is in-core only;\n+\t\t\t * we are not going to write this change out.\n+\t\t\t */\n+\t\t\tce_mark_uptodate(ce);\n \t\t\treturn ce;\n+\t\t}\n \t}\n \n \tif (ie_modified(istate, ce, &st, options)) {\n-- \n1.5.4.rc3.37.gfdcf3\n"},{"id":"65985","messageId":"alpine.LFD.1.00.0801191133330.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"7vfxwu9s2z.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-19T19:47:56Z","receivedAt":"2008-01-19T19:47:56Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 18 Jan 2008, Junio C Hamano wrote:\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n\n>  * Again, this is a rebase on top of your \"Updated in-memory\n>    index\".  \"echo >>Makefile && git commit -m test Makefile\" in\n>    the kernel repository does 23k lstat(2) with this patch, 46k\n>    without.\n\nSadly, doing a simple \"git commit\" without anything else, will still \napparently get us 69230 lstat() calls, with or without this patch. So we \nget 3 lstat() calls per path.\n\nThe call trace for the first one is\n\n - builtin-commit.c: line 786\n\tprepare_index\n\t  refresh_index\n\t    refresh_cache_ent\n\t      lstat\n\nwhile the second one is\n\n - builtin-commit.c: line 793\n\tprepare_log_message\n\t  run_status\n\t    wt_status_print\n\t      wt_status_print_changed\n\t        run_diff_files\n\t          lstat\n\nand the third one is\n\n - builtin-commit.c: line 795\n\trun_status\n\t  wt_status_print\n\t    wt_status_print_changed\n\t      run_diff_files\n\t        lstat\n\nand the reason is that \n - \"run_diff_files()\" doesn't honor ce_uptodate\n - ..but even if it did, wt_status_print_changed() does a wt_read_cache() \n   which will invalidate and re-read the index!\n\nThe first problem is trivially fixed (appended), the second one is notas \ntrivial.\n\n\t\tLinus\n---\n diff-lib.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 4a05b02..5d5a950 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -435,6 +435,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\tcontinue;\n \t\t}\n \n+\t\tif (ce_uptodate(ce))\n+\t\t\tcontinue;\n \t\tif (lstat(ce->name, &st) < 0) {\n \t\t\tif (errno != ENOENT && errno != ENOTDIR) {\n \t\t\t\tperror(ce->name);\n"},{"id":"65988","messageId":"alpine.LFD.1.00.0801191152310.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801191133330.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-19T19:58:47Z","receivedAt":"2008-01-19T19:58:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 19 Jan 2008, Linus Torvalds wrote:\n> \n> The call trace for the first one is\n\nBtw, the easiest way to do this on Linux isn't totally obvious, because \nyou cannot do a simple breakpoint on \"lstat()\", since GNU libc internally \nuses other names.\n\nSo if anybody wants to work on this and runs Linux, the way to do this \ntrivially (once you know how) is to do\n\n\tgdb git\n\t..\n\t.. run the startup to wait for the libraries to be loaded \n\t..\t\n\t(gdb) b main\n\t(gdb) run commit\n\t..\n\t.. modern glibc low-level lstat() call is __lxstat64\n\t.. at least on x86-64\n\t..\n\t(gdb) b __lxstat64\n\t..\n\t.. Ignore the first few ones (we have 23000+ files in the index, \n\t.. so don't worry about getting the *first* one)\n\t..\n\t(gdb) ignore 2 1000\n\t(gdb) c\n\t..\n\t.. Ok, look at that backtrace, then go to the next one by\n\t.. just knowing that we'll be doing 23,000+ for each iteration\n\t.. over the index.\n\t..\n\t.. Rinse and repeat this as required:\n\t..\n\t(gdb) where\n\t(gdb) ignore 2 24000\n\t(gdb) c\n\nwhich gets you the backtraces I showed you, without having to think too \nmuch about all the odd lstat() calls we do for resolving refs etc.\n\n\t\t\tLinus\n"},{"id":"66005","messageId":"alpine.LFD.1.00.0801191709380.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801191133330.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T01:22:29Z","receivedAt":"2008-01-20T01:22:29Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 19 Jan 2008, Linus Torvalds wrote:\n> \n> and the reason is that \n>  - \"run_diff_files()\" doesn't honor ce_uptodate\n>  - ..but even if it did, wt_status_print_changed() does a wt_read_cache() \n>    which will invalidate and re-read the index!\n> \n> The first problem is trivially fixed (appended), the second one is not as \n> trivial.\n\nOk, I have worked on that not-as-trivial thing here, and indeed, it was \nquite painful and needed major cleanups elsewhere.\n\nThe reason why wt-status.c ended up invalidating and re-reading the cache \nmultiple times was that it uses \"run_diff_index()\", which in turn uses \n\"read_tree()\" to populate the index with *both* the old index and the tree \nwe want to compare against.\n\nSo this patch re-writes run_diff_index() to not use read_tree(), but \ninstead use \"unpack_trees()\" to diff the index to a tree. That, in turn, \nmeans that we don't need to modify the index itself, which then means that \nwe don't need to invalidate it and re-read it!\n\nThis, together with the lstat() optimizations, means that \"git commit\" on \nthe kernel tree really only needs to lstat() the index entries once. That \nnoticeably cuts down on the cached timings.\n\nBest time before:\n\n\t[torvalds@woody linux]$ time git commit > /dev/null\n\treal    0m0.399s\n\tuser    0m0.232s\n\tsys     0m0.164s\n\nBest time after:\n\n\t[torvalds@woody linux]$ time git commit > /dev/null\n\treal    0m0.254s\n\tuser    0m0.140s\n\tsys     0m0.112s\n\nso it's a noticeable improvement in addition to being a nice conceptual \ncleanup (it's really not that pretty that \"run_diff_index()\" dirties the \nindex!)\n\nDoing an \"strace -c\" on it also shows that as it cuts the number of \nlstat() calls by two thirds, it goes from being lstat()-limited to being \nlimited by getdents() (which is the readdir system call):\n\nBefore:\n\t% time     seconds  usecs/call     calls    errors syscall\n\t------ ----------- ----------- --------- --------- ----------------\n\t 60.69    0.000704           0     69230        31 lstat\n\t 23.62    0.000274           0      5522           getdents\n\t  8.36    0.000097           0      5508      2638 open\n\t  2.59    0.000030           0      2869           close\n\t  2.50    0.000029           0       274           write\n\t  1.47    0.000017           0      2844           fstat\n\nAfter:\n\t% time     seconds  usecs/call     calls    errors syscall\n\t------ ----------- ----------- --------- --------- ----------------\n\t 45.17    0.000276           0      5522           getdents\n\t 26.51    0.000162           0     23112        31 lstat\n\t 19.80    0.000121           0      5503      2638 open\n\t  4.91    0.000030           0      2864           close\n\t  1.48    0.000020           0       274           write\n\t  1.34    0.000018           0      2844           fstat\n\t...\n\nIt passes the test-suite for me, but this is another of one of those \nreally core functions, and certainly pretty subtle, so..\n\nNOTE! The Linux lstat() system call is really quite cheap when everything \nis cached, so the fact that this is quite noticeable on Linux is likely to \nmean that it is *much* more noticeable on other operating systems. I bet \nyou'll see a much bigger performance improvement from this on Windows in \nparticular.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n\nThis patch also includes the already sent-out trivial fix to \nrun_diff_files to take advantage of ce_uptodate().\n\n\n diff-lib.c     |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++-----\n unpack-trees.h |    1 +\n wt-status.c    |    4 ---\n 3 files changed, 59 insertions(+), 10 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 4a05b02..23d0fa6 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -9,6 +9,7 @@\n #include \"revision.h\"\n #include \"cache-tree.h\"\n #include \"path-list.h\"\n+#include \"unpack-trees.h\"\n \n /*\n  * diff-files\n@@ -435,6 +436,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\tcontinue;\n \t\t}\n \n+\t\tif (ce_uptodate(ce))\n+\t\t\tcontinue;\n \t\tif (lstat(ce->name, &st) < 0) {\n \t\t\tif (errno != ENOENT && errno != ENOTDIR) {\n \t\t\t\tperror(ce->name);\n@@ -665,12 +668,54 @@ static void mark_merge_entries(void)\n \t}\n }\n \n+static int oneway_diff(struct cache_entry **src,\n+\tstruct unpack_trees_options *o,\n+\tint remove)\n+{\n+\tstruct cache_entry *idx = src[0];\n+\tstruct cache_entry *tree = src[1];\n+\tstruct rev_info *revs = o->unpack_data;\n+\n+\t/*\n+\t * Unpack-trees generates a DF/conflict entry if\n+\t * there was a directory in the index and a tree\n+\t * in the tree. From a diff standpoint, that's a\n+\t * delete of the tree and a create of the file.\n+\t */\n+\tif (tree == o->df_conflict_entry)\n+\t\ttree = NULL;\n+\n+\t/*\n+\t * Something added to the tree?\n+\t */\n+\tif (!tree) {\n+\t\tif (ce_path_match(idx, revs->prune_data))\n+\t\t\tshow_new_file(revs, idx, o->index_only, 0);\n+\t\treturn 1;\n+\t}\n+\n+\t/*\n+\t * Something removed from the tree?\n+\t */\n+\tif (!idx) {\n+\t\tif (ce_path_match(tree, revs->prune_data))\n+\t\t\tdiff_index_show_file(revs, \"-\", tree, tree->sha1, tree->ce_mode);\n+\t\treturn 0;\n+\t}\n+\n+\t/* Show difference between old and new */\n+\tif (ce_path_match(idx, revs->prune_data))\n+\t\tshow_modified(revs, tree, idx, 1, o->index_only, 0);\n+\treturn 1;\n+}\n+\n int run_diff_index(struct rev_info *revs, int cached)\n {\n-\tint ret;\n \tstruct object *ent;\n \tstruct tree *tree;\n \tconst char *tree_name;\n+\tstruct unpack_trees_options opts;\n+\tstruct tree_desc t;\n \tint match_missing = 0;\n \n \t/*\n@@ -687,13 +732,20 @@ int run_diff_index(struct rev_info *revs, int cached)\n \ttree = parse_tree_indirect(ent->sha1);\n \tif (!tree)\n \t\treturn error(\"bad tree object %s\", tree_name);\n-\tif (read_tree(tree, 1, revs->prune_data))\n-\t\treturn error(\"unable to read tree object %s\", tree_name);\n-\tret = diff_cache(revs, active_cache, active_nr, revs->prune_data,\n-\t\t\t cached, match_missing);\n+\n+\tmemset(&opts, 0, sizeof(opts));\n+\topts.head_idx = 1;\n+\topts.index_only = cached;\n+\topts.merge = 1;\n+\topts.fn = oneway_diff;\n+\topts.unpack_data = revs;\n+\n+\tinit_tree_desc(&t, tree->buffer, tree->size);\n+\tunpack_trees(1, &t, &opts);\n+\t\n \tdiffcore_std(&revs->diffopt);\n \tdiff_flush(&revs->diffopt);\n-\treturn ret;\n+\treturn 0;\n }\n \n int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 5517faa..197a004 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -25,6 +25,7 @@ struct unpack_trees_options {\n \tint merge_size;\n \n \tstruct cache_entry *df_conflict_entry;\n+\tvoid *unpack_data;\n };\n \n extern int unpack_trees(unsigned n, struct tree_desc *t,\ndiff --git a/wt-status.c b/wt-status.c\nindex c0c2472..a18ed11 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -229,7 +229,6 @@ static void wt_status_print_initial(struct wt_status *s)\n \tstruct strbuf buf;\n \n \tstrbuf_init(&buf, 0);\n-\twt_read_cache(s);\n \tif (active_nr) {\n \t\ts->commitable = 1;\n \t\twt_status_print_cached_header(s);\n@@ -256,7 +255,6 @@ static void wt_status_print_updated(struct wt_status *s)\n \trev.diffopt.detect_rename = 1;\n \trev.diffopt.rename_limit = 100;\n \trev.diffopt.break_opt = 0;\n-\twt_read_cache(s);\n \trun_diff_index(&rev, 1);\n }\n \n@@ -268,7 +266,6 @@ static void wt_status_print_changed(struct wt_status *s)\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = wt_status_print_changed_cb;\n \trev.diffopt.format_callback_data = s;\n-\twt_read_cache(s);\n \trun_diff_files(&rev, 0);\n }\n \n@@ -335,7 +332,6 @@ static void wt_status_print_verbose(struct wt_status *s)\n \tsetup_revisions(0, NULL, &rev, s->reference);\n \trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \trev.diffopt.detect_rename = 1;\n-\twt_read_cache(s);\n \trun_diff_index(&rev, 1);\n \n \tfflush(stdout);\n"},{"id":"66006","messageId":"alpine.LFD.1.00.0801191729490.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801191709380.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T01:34:10Z","receivedAt":"2008-01-20T01:34:10Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 19 Jan 2008, Linus Torvalds wrote:\n> \n> It passes the test-suite for me, but this is another of one of those \n> really core functions, and certainly pretty subtle, so..\n\nBy this I don't mean that I don't think this patch is good, but it does \nmean that while I think it's a great patch (along with all the other index \ncleanups and the lstat-avoidance this one depends on), it's probably not \nappropriate for master without some more people looking at it, and \nprobably cooking in a next for a while.\n\n\t\tLinus\n"},{"id":"66008","messageId":"alpine.LSU.1.00.0801200142170.5731@racer.site","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801191709380.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T01:48:34Z","receivedAt":"2008-01-20T01:48:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 19 Jan 2008, Linus Torvalds wrote:\n\n\n> Best time before:\n> \n> \t[torvalds@woody linux]$ time git commit > /dev/null\n> \treal    0m0.399s\n> \tuser    0m0.232s\n> \tsys     0m0.164s\n> \n> Best time after:\n> \n> \t[torvalds@woody linux]$ time git commit > /dev/null\n> \treal    0m0.254s\n> \tuser    0m0.140s\n> \tsys     0m0.112s\n\nWow.\n\n> I bet you'll see a much bigger performance improvement from this on \n> Windows in particular.\n\nI bet so, too.  Traditionally, filesystem calls are painfully slow on \nWindows.\n\nBut I cannot test before Monday, so I would not be mad if somebody else \ncould perform some tests on Windows.\n\nNow, the changes you made are quite a ways off of my current knowledge of \ngit codepaths, so I cannot really comment.  Although they look sane \nenough.\n\n> @@ -687,13 +732,20 @@ int run_diff_index(struct rev_info *revs, int cached)\n>  \ttree = parse_tree_indirect(ent->sha1);\n>  \tif (!tree)\n>  \t\treturn error(\"bad tree object %s\", tree_name);\n> -\tif (read_tree(tree, 1, revs->prune_data))\n> -\t\treturn error(\"unable to read tree object %s\", tree_name);\n> -\tret = diff_cache(revs, active_cache, active_nr, revs->prune_data,\n> -\t\t\t cached, match_missing);\n> +\n> +\tmemset(&opts, 0, sizeof(opts));\n> +\topts.head_idx = 1;\n> +\topts.index_only = cached;\n> +\topts.merge = 1;\n> +\topts.fn = oneway_diff;\n> +\topts.unpack_data = revs;\n> +\n> +\tinit_tree_desc(&t, tree->buffer, tree->size);\n> +\tunpack_trees(1, &t, &opts);\n> +\t\n>  \tdiffcore_std(&revs->diffopt);\n>  \tdiff_flush(&revs->diffopt);\n> -\treturn ret;\n> +\treturn 0;\n>  }\n>  \n>  int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n\nWhat about do_diff_cache()?  Shouldn't it get exactly the same treatment \nas run_diff_index()?\n\nIn which case, do_diff_cache() would not be needed anymore, too.\n\n> diff --git a/unpack-trees.h b/unpack-trees.h\n> index 5517faa..197a004 100644\n> --- a/unpack-trees.h\n> +++ b/unpack-trees.h\n> @@ -25,6 +25,7 @@ struct unpack_trees_options {\n>  \tint merge_size;\n>  \n>  \tstruct cache_entry *df_conflict_entry;\n> +\tvoid *unpack_data;\n\nMaybe this should be called \"cb_data\", in line with similar places in \ngit.git?\n\n> diff --git a/wt-status.c b/wt-status.c\n> index c0c2472..a18ed11 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -229,7 +229,6 @@ static void wt_status_print_initial(struct wt_status *s)\n>  \tstruct strbuf buf;\n>  \n>  \tstrbuf_init(&buf, 0);\n> -\twt_read_cache(s);\n\nIf I'm not mistaken, you removed all four call sites of wt_read_cache(), \nand it is static anyway, so it should be removed, no?\n\nCiao,\nDscho \"who will try to wrap his head around what read_tree() && \ndiff_cache() are supposed to do\"\n"},{"id":"66010","messageId":"alpine.LFD.1.00.0801191801170.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801200142170.5731@racer.site","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T02:02:21Z","receivedAt":"2008-01-20T02:02:21Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 20 Jan 2008, Johannes Schindelin wrote:\n> \n> If I'm not mistaken, you removed all four call sites of wt_read_cache(), \n> and it is static anyway, so it should be removed, no?\n\nYes.\n\nThe wt_read_cache() changes are really independent, and while removing \nthem was my original goal, that didn't end up being the interesting part \nof the patch, which is why I then left that part half-way..\n\n\t\tLinus\n"},{"id":"66011","messageId":"alpine.LSU.1.00.0801200231250.5731@racer.site","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801191709380.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T02:42:06Z","receivedAt":"2008-01-20T02:42:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 19 Jan 2008, Linus Torvalds wrote:\n\n> @@ -687,13 +732,20 @@ int run_diff_index(struct rev_info *revs, int cached)\n>  \ttree = parse_tree_indirect(ent->sha1);\n>  \tif (!tree)\n>  \t\treturn error(\"bad tree object %s\", tree_name);\n> -\tif (read_tree(tree, 1, revs->prune_data))\n> -\t\treturn error(\"unable to read tree object %s\", tree_name);\n> -\tret = diff_cache(revs, active_cache, active_nr, revs->prune_data,\n> -\t\t\t cached, match_missing);\n> +\n> +\tmemset(&opts, 0, sizeof(opts));\n> +\topts.head_idx = 1;\n> +\topts.index_only = cached;\n> +\topts.merge = 1;\n> +\topts.fn = oneway_diff;\n> +\topts.unpack_data = revs;\n> +\n> +\tinit_tree_desc(&t, tree->buffer, tree->size);\n> +\tunpack_trees(1, &t, &opts);\n> +\t\n>  \tdiffcore_std(&revs->diffopt);\n>  \tdiff_flush(&revs->diffopt);\n> -\treturn ret;\n> +\treturn 0;\n>  }\n>\n\nTwo problems I see (before I go to bed): match_missing is now ignored, and \nthe return value is set to 0, whereas it was the return value of \ndiff_cache() before.\n\nThe first is easily fixed, by replacing the two \"0\"s in the calls to \nshow_new_file() and show_modified() with \"!revs->ignore_merges\".  Unless I \nam missing something, of course.\n\nAnd the second is not _that_ bad, as it seems that diff_cache() has only \none return statement, and it returns 0, AFAICS.  But it had to be said \nsomewhere. \n\nCiao,\nDscho\n"},{"id":"66029","messageId":"07FDB6ED-EBE0-4F66-9167-6A0F8AD58E2B@zib.de","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801200142170.5731@racer.site","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2008-01-20T10:33:06Z","receivedAt":"2008-01-20T10:33:06Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Jan 20, 2008, at 2:48 AM, Johannes Schindelin wrote:\n\n>> I bet you'll see a much bigger performance improvement from this on\n>> Windows in particular.\n>\n> I bet so, too.  Traditionally, filesystem calls are painfully slow on\n> Windows.\n>\n> But I cannot test before Monday, so I would not be mad if somebody  \n> else\n> could perform some tests on Windows.\n\nHas someone collected the whole series on a topic branch?  I did\nnot follow the discussion closely and apparently more than Linus'\npatch is needed.  I couldn't immediately figure out which of the\npatches from the thread should be applied in what order.\n\n\tSteffen\n"},{"id":"66034","messageId":"alpine.LSU.1.00.0801201352340.5731@racer.site","threadId":"11671","inReplyTo":"07FDB6ED-EBE0-4F66-9167-6A0F8AD58E2B@zib.de","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T14:15:26Z","receivedAt":"2008-01-20T14:15:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jan 2008, Steffen Prohaska wrote:\n\n> On Jan 20, 2008, at 2:48 AM, Johannes Schindelin wrote:\n> \n> > > I bet you'll see a much bigger performance improvement from this on \n> > > Windows in particular.\n> > \n> > I bet so, too.  Traditionally, filesystem calls are painfully slow on \n> > Windows.\n> > \n> > But I cannot test before Monday, so I would not be mad if somebody \n> > else could perform some tests on Windows.\n> \n> Has someone collected the whole series on a topic branch?  I did not \n> follow the discussion closely and apparently more than Linus' patch is \n> needed.  I couldn't immediately figure out which of the patches from the \n> thread should be applied in what order.\n\nAFAICT Linus' patch relies on Junio's lstat() patch that was the OP in \nthis thread.\n\nFWIW I applied both patches to current \"master\" and pushed to the \"lstat\" \nbranch of http://repo.or.cz/w/git/dscho.git\n\nHth,\nDscho\n"},{"id":"66035","messageId":"15ECE22B-FCBB-4F12-919B-694E48D48E0D@zib.de","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801200142170.5731@racer.site","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2008-01-20T15:10:52Z","receivedAt":"2008-01-20T15:10:52Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Jan 20, 2008, at 2:48 AM, Johannes Schindelin wrote:\n\n> On Sat, 19 Jan 2008, Linus Torvalds wrote:\n>\n>\n>> Best time before:\n>>\n>> \t[torvalds@woody linux]$ time git commit > /dev/null\n>> \treal    0m0.399s\n>> \tuser    0m0.232s\n>> \tsys     0m0.164s\n>>\n>> Best time after:\n>>\n>> \t[torvalds@woody linux]$ time git commit > /dev/null\n>> \treal    0m0.254s\n>> \tuser    0m0.140s\n>> \tsys     0m0.112s\n>\n> Wow.\n>\n>> I bet you'll see a much bigger performance improvement from this on\n>> Windows in particular.\n>\n> I bet so, too.  Traditionally, filesystem calls are painfully slow on\n> Windows.\n>\n> But I cannot test before Monday, so I would not be mad if somebody  \n> else\n> could perform some tests on Windows.\n\nHere are timings for Windows XP running in a virtual machine on a\nLaptop.  The work tree contains 7k files.  I stripped user and\nsys times because they are apparently meaningless for MINGW.\n\nBest time before:\n\n     $ time git commit >/dev/null\n     real    0m1.662s\n\nBest time after:\n\n     $ time git commit >/dev/null\n     real    0m1.196s\n\nThe absolute time improvement is obviously larger, although the\nrelative improvement is slightly smaller than in Linus' example.\n\n\nAnd here are the timings for the host system, which is Mac OS X,\non the same work tree.\n\nBest time before:\n\n     $ time git commit >/dev/null\n     real    0m0.571s\n     user    0m0.332s\n     sys     0m0.237s\n\nBest time after:\n\n     $ time git commit >/dev/null\n     real    0m0.463s\n     user    0m0.273s\n     sys     0m0.186s\n\nInterestingly, the relative improvements are even smaller on Mac\nOS X.\n\n\tSteffen\n"},{"id":"66036","messageId":"alpine.LSU.1.00.0801201515060.5731@racer.site","threadId":"11671","inReplyTo":"15ECE22B-FCBB-4F12-919B-694E48D48E0D@zib.de","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T15:18:48Z","receivedAt":"2008-01-20T15:18:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jan 2008, Steffen Prohaska wrote:\n\n> On Jan 20, 2008, at 2:48 AM, Johannes Schindelin wrote:\n> \n> > On Sat, 19 Jan 2008, Linus Torvalds wrote:\n> > \n> > > Best time before:\n> > > \n> > > \t[torvalds@woody linux]$ time git commit > /dev/null\n> > > \treal    0m0.399s\n> > > \tuser    0m0.232s\n> > > \tsys     0m0.164s\n> > > \n> > > Best time after:\n> > > \n> > > \t[torvalds@woody linux]$ time git commit > /dev/null\n> > > \treal    0m0.254s\n> > > \tuser    0m0.140s\n> > > \tsys     0m0.112s\n> > \n> > Wow.\n> > \n> > > I bet you'll see a much bigger performance improvement from this on\n> > > Windows in particular.\n> > \n> > I bet so, too.  Traditionally, filesystem calls are painfully slow on\n> > Windows.\n> > \n> > But I cannot test before Monday, so I would not be mad if somebody else\n> > could perform some tests on Windows.\n> \n> Here are timings for Windows XP running in a virtual machine on a\n> Laptop.  The work tree contains 7k files.  I stripped user and\n> sys times because they are apparently meaningless for MINGW.\n> \n> Best time before:\n> \n>    $ time git commit >/dev/null\n>    real    0m1.662s\n> \n> Best time after:\n> \n>    $ time git commit >/dev/null\n>    real    0m1.196s\n> \n> The absolute time improvement is obviously larger, although the\n> relative improvement is slightly smaller than in Linus' example.\n> \n> \n> And here are the timings for the host system, which is Mac OS X,\n> on the same work tree.\n> \n> Best time before:\n> \n>    $ time git commit >/dev/null\n>    real    0m0.571s\n>    user    0m0.332s\n>    sys     0m0.237s\n> \n> Best time after:\n> \n>    $ time git commit >/dev/null\n>    real    0m0.463s\n>    user    0m0.273s\n>    sys     0m0.186s\n> \n> Interestingly, the relative improvements are even smaller on Mac\n> OS X.\n\nStill, nothing to spit at.\n\nFWIW I have made two patches which should be probably folded into Linus' \npatch; I will post them as a reply.\n\nCiao,\nDscho\n"},{"id":"66037","messageId":"alpine.LSU.1.00.0801201519320.5731@racer.site","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801201515060.5731@racer.site","subject":"[PATCH] Also use unpack_trees() in do_diff_cache()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T15:19:56Z","receivedAt":"2008-01-20T15:19:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nAs in run_diff_index(), we call unpack_trees() with the oneway_diff()\nfunction in do_diff_cache() now.  This makes the function diff_cache()\nobsolete.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\nObviously on top of Junio's + Linus' patch.\n\n diff-lib.c |   92 ++++++++---------------------------------------------------\n 1 files changed, 13 insertions(+), 79 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex dc7b514..7941486 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -580,81 +580,6 @@ static int show_modified(struct rev_info *revs,\n \treturn 0;\n }\n \n-static int diff_cache(struct rev_info *revs,\n-\t\t      struct cache_entry **ac, int entries,\n-\t\t      const char **pathspec,\n-\t\t      int cached, int match_missing)\n-{\n-\twhile (entries) {\n-\t\tstruct cache_entry *ce = *ac;\n-\t\tint same = (entries > 1) && ce_same_name(ce, ac[1]);\n-\n-\t\tif (DIFF_OPT_TST(&revs->diffopt, QUIET) &&\n-\t\t\tDIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))\n-\t\t\tbreak;\n-\n-\t\tif (!ce_path_match(ce, pathspec))\n-\t\t\tgoto skip_entry;\n-\n-\t\tswitch (ce_stage(ce)) {\n-\t\tcase 0:\n-\t\t\t/* No stage 1 entry? That means it's a new file */\n-\t\t\tif (!same) {\n-\t\t\t\tshow_new_file(revs, ce, cached, match_missing);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\t/* Show difference between old and new */\n-\t\t\tshow_modified(revs, ac[1], ce, 1,\n-\t\t\t\t      cached, match_missing);\n-\t\t\tbreak;\n-\t\tcase 1:\n-\t\t\t/* No stage 3 (merge) entry?\n-\t\t\t * That means it's been deleted.\n-\t\t\t */\n-\t\t\tif (!same) {\n-\t\t\t\tdiff_index_show_file(revs, \"-\", ce,\n-\t\t\t\t\t\t     ce->sha1, ce->ce_mode);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\t/* We come here with ce pointing at stage 1\n-\t\t\t * (original tree) and ac[1] pointing at stage\n-\t\t\t * 3 (unmerged).  show-modified with\n-\t\t\t * report-missing set to false does not say the\n-\t\t\t * file is deleted but reports true if work\n-\t\t\t * tree does not have it, in which case we\n-\t\t\t * fall through to report the unmerged state.\n-\t\t\t * Otherwise, we show the differences between\n-\t\t\t * the original tree and the work tree.\n-\t\t\t */\n-\t\t\tif (!cached &&\n-\t\t\t    !show_modified(revs, ce, ac[1], 0,\n-\t\t\t\t\t   cached, match_missing))\n-\t\t\t\tbreak;\n-\t\t\tdiff_unmerge(&revs->diffopt, ce->name,\n-\t\t\t\t     ntohl(ce->ce_mode), ce->sha1);\n-\t\t\tbreak;\n-\t\tcase 3:\n-\t\t\tdiff_unmerge(&revs->diffopt, ce->name,\n-\t\t\t\t     0, null_sha1);\n-\t\t\tbreak;\n-\n-\t\tdefault:\n-\t\t\tdie(\"impossible cache entry stage\");\n-\t\t}\n-\n-skip_entry:\n-\t\t/*\n-\t\t * Ignore all the different stages for this file,\n-\t\t * we've handled the relevant cases now.\n-\t\t */\n-\t\tdo {\n-\t\t\tac++;\n-\t\t\tentries--;\n-\t\t} while (entries && ce_same_name(ce, ac[0]));\n-\t}\n-\treturn 0;\n-}\n-\n /*\n  * This turns all merge entries into \"stage 3\". That guarantees that\n  * when we read in the new tree (into \"stage 1\"), we won't lose sight\n@@ -758,6 +683,8 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \tint i;\n \tstruct cache_entry **dst;\n \tstruct cache_entry *last = NULL;\n+\tstruct unpack_trees_options opts;\n+\tstruct tree_desc t;\n \n \t/*\n \t * This is used by git-blame to run diff-cache internally;\n@@ -786,8 +713,15 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \ttree = parse_tree_indirect(tree_sha1);\n \tif (!tree)\n \t\tdie(\"bad tree object %s\", sha1_to_hex(tree_sha1));\n-\tif (read_tree(tree, 1, opt->paths))\n-\t\treturn error(\"unable to read tree %s\", sha1_to_hex(tree_sha1));\n-\treturn diff_cache(&revs, active_cache, active_nr, revs.prune_data,\n-\t\t\t  1, 0);\n+\n+\tmemset(&opts, 0, sizeof(opts));\n+\topts.head_idx = 1;\n+\topts.index_only = 1;\n+\topts.merge = 1;\n+\topts.fn = oneway_diff;\n+\topts.unpack_data = &revs;\n+\n+\tinit_tree_desc(&t, tree->buffer, tree->size);\n+\tunpack_trees(1, &t, &opts);\n+\treturn 0;\n }\n-- \n1.5.4.rc3.44.g6cd4\n"},{"id":"66038","messageId":"alpine.LSU.1.00.0801201521230.5731@racer.site","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801201515060.5731@racer.site","subject":"[PATCH] Try to resurrect the handling for 'diff-index -m'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T15:21:50Z","receivedAt":"2008-01-20T15:21:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nrun_diff_index() had special handling for 'diff-index -m', which\nwas lost in the conversion to use unpack_trees() instead of\ndiff_cache().\n\n---\n\nAlso on top of the patches of Junio and Linus.\n\nI am not too certain about this; I just tried to reproduce what the old\ncode did.\n\n diff-lib.c |   24 +++++++++++++-----------\n 1 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 7941486..27032a9 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -614,11 +614,19 @@ static int oneway_diff(struct cache_entry **src,\n \t\ttree = NULL;\n \n \t/*\n+\t * Backward compatibility wart - \"diff-index -m\" does\n+\t * not mean \"do not ignore merges\", but totally different.\n+\t * Therefore, the \"match_missing\" argument is set to\n+\t * \"!revs->ignore_merges\".\n+\t */\n+\n+\t/*\n \t * Something added to the tree?\n \t */\n \tif (!tree) {\n \t\tif (ce_path_match(idx, revs->prune_data))\n-\t\t\tshow_new_file(revs, idx, o->index_only, 0);\n+\t\t\tshow_new_file(revs, idx, o->index_only,\n+\t\t\t\t\t!revs->ignore_merges);\n \t\treturn 1;\n \t}\n \n@@ -627,13 +635,15 @@ static int oneway_diff(struct cache_entry **src,\n \t */\n \tif (!idx) {\n \t\tif (ce_path_match(tree, revs->prune_data))\n-\t\t\tdiff_index_show_file(revs, \"-\", tree, tree->sha1, tree->ce_mode);\n+\t\t\tdiff_index_show_file(revs, \"-\",\n+\t\t\t\t\ttree, tree->sha1, tree->ce_mode);\n \t\treturn 0;\n \t}\n \n \t/* Show difference between old and new */\n \tif (ce_path_match(idx, revs->prune_data))\n-\t\tshow_modified(revs, tree, idx, 1, o->index_only, 0);\n+\t\tshow_modified(revs, tree, idx, 1, o->index_only,\n+\t\t\t\t!revs->ignore_merges);\n \treturn 1;\n }\n \n@@ -644,14 +654,6 @@ int run_diff_index(struct rev_info *revs, int cached)\n \tconst char *tree_name;\n \tstruct unpack_trees_options opts;\n \tstruct tree_desc t;\n-\tint match_missing = 0;\n-\n-\t/*\n-\t * Backward compatibility wart - \"diff-index -m\" does\n-\t * not mean \"do not ignore merges\", but totally different.\n-\t */\n-\tif (!revs->ignore_merges)\n-\t\tmatch_missing = 1;\n \n \tmark_merge_entries();\n \n-- \n1.5.4.rc3.44.g6cd4\n"},{"id":"66040","messageId":"alpine.LFD.1.00.0801200942191.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"15ECE22B-FCBB-4F12-919B-694E48D48E0D@zib.de","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T18:20:46Z","receivedAt":"2008-01-20T18:20:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOn Sun, 20 Jan 2008, Steffen Prohaska wrote:\n> \n> Here are timings for Windows XP running in a virtual machine on a\n> Laptop.  The work tree contains 7k files.  I stripped user and\n> sys times because they are apparently meaningless for MINGW.\n> \n> Best time before:\n> \n>    $ time git commit >/dev/null\n>    real    0m1.662s\n> \n> Best time after:\n> \n>    $ time git commit >/dev/null\n>    real    0m1.196s\n\nOk, very interesting.\n\nThat's a 28% reduction in running time (or, alternatively, if you want to \nmake it sound better, a 39% performance improvement ;), but quite frankly, \nI was hoping for even more. On my tests (with the kernel tree), the patch \nseries literally removed half of the system calls, and I was really hoping \nthat performance would be almost linear in system calls, especially on \nWindows where they are more expensive.\n\nBut I guess that was a bit optimistic. On Linux, we still had about 50% \nuser time, and I guess the same is _largely_ true on Windows. Cutting the \nsystem time in half only gives you about a quarter performance \nimprovement, and I was just incorrect in expecting system calls to be the \nlimiting factor.\n\nThat's still a nice performance improvement, of course, but I was really \nhoping for more, since people have claimed that git is slow on windows. \nMaybe that's simply not true any more now that (a) people use the thinner \nmingw layers and (b) many things - like git-commit - are built-in, so that \nthere simply isn't the horrible process creation overhead.\n\nI have absolutely no idea how to do performance analysis or even something \nsimple as getting a list of system calls from Windows (even if I had a \nWindows machine, which I obviously don't ;), so I'm afraid I have no clue \nwhy git might be considered slow there. I was hoping this was it\n\n> And here are the timings for the host system, which is Mac OS X,\n> on the same work tree.\n> \n> Best time before:\n> \n>    $ time git commit >/dev/null\n>    real    0m0.571s\n>    user    0m0.332s\n>    sys     0m0.237s\n> \n> Best time after:\n> \n>    $ time git commit >/dev/null\n>    real    0m0.463s\n>    user    0m0.273s\n>    sys     0m0.186s\n> \n> Interestingly, the relative improvements are even smaller on Mac\n> OS X.\n\nWell, you're testing something with 7k files, and I was testing something \nwith 23k files. The changes in question only affect the number of lstat's \non those files, they don't affect things like the basic setup overhead we \nhave for doign things like checking that refs are unique.\n\nSo your test-case does have relatively more overhead (compared to what got \noptimized) than the numbers I quoted. \n\nWe also do know that while Linux has a very low-overhead lstat(), the real \nproblem on OS X has been the memory management, not the filesystem. We had \nsome case where the page fault overhead was something like two orders of \nmagnitude bigger, didn't we?\n\n(Yeah - just checked. Commit 6d2fa7f1b489c65e677c18eda5c144dbc5d614ab: \n\"index-pack usage of mmap() is unacceptably slower..\"). That took a minute \non Linux, and an hour on OS X)\n\nIt would be good to have a system-level profile of something like this. On \nLinux, it's now mostly in user space with \"git commit\", and oprofile \nshows:\n\n\tsamples  %        image name               app name                 symbol name\n\t23318    11.5388  git                      git                      cache_name_compare\n\t11910     5.8936  git                      git                      excluded\n\t9775      4.8371  libcrypto.so.0.9.8b      libcrypto.so.0.9.8b      (no symbols)\n\t7990      3.9538  libc-2.7.so              libc-2.7.so              strlen\n\t7468      3.6955  vmlinux                  vmlinux                  __d_lookup\n\t7137      3.5317  libc-2.7.so              libc-2.7.so              internal_fnmatch\n\t7047      3.4872  libz.so.1.2.3            libz.so.1.2.3            (no symbols)\n\t6317      3.1259  git                      git                      index_name_pos\n\t5537      2.7400  git                      git                      unpack_trees_rec\n\t5079      2.5133  vmlinux                  vmlinux                  ext3fs_dirhash\n\t4774      2.3624  libc-2.7.so              libc-2.7.so              strcmp\n\t4005      1.9819  vmlinux                  vmlinux                  __link_path_walk\n\t3436      1.7003  vmlinux                  vmlinux                  ext3_htree_store_dirent\n\t3421      1.6929  vmlinux                  vmlinux                  __kmalloc\n\t3188      1.5776  vmlinux                  vmlinux                  _atomic_dec_and_lock\n\t3041      1.5048  libc-2.7.so              libc-2.7.so              _int_malloc\n\t2904      1.4370  libc-2.7.so              libc-2.7.so              fnmatch@@GLIBC_2.2.5\n\t2727      1.3494  vmlinux                  vmlinux                  str2hashbuf\n\t...\n\nand one thing to look out for would be that some of these things might be \nrelatively much more costly on other systems.\n\nfnmatch? It's certainly visible on Linux, I wonder if the Windows/OSX \nversion is more expensive due to trying to be case-insensitive or \nsomething?\n\n\t\t\t\tLinus\n"},{"id":"66044","messageId":"20080120200310.GK22503@lavos.net","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801200942191.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2008-01-20T20:03:10Z","receivedAt":"2008-01-20T20:03:10Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Sun, Jan 20, 2008 at 10:20:46AM -0800, Linus Torvalds wrote:\n> But I guess that was a bit optimistic. On Linux, we still had about 50% \n> user time, and I guess the same is _largely_ true on Windows. Cutting the \n> system time in half only gives you about a quarter performance \n> improvement, and I was just incorrect in expecting system calls to be the \n> limiting factor.\n> \n> [...]\n> \n> fnmatch? It's certainly visible on Linux, I wonder if the Windows/OSX \n> version is more expensive due to trying to be case-insensitive or \n> something?\n\nWindows (msys) Git, with the lstat patch and on my 15433-file test case,\nis at least 90-95% system time.  I don't have any good tools for\nmeasuring this, but just using the silly performance monitor you get\nwith ctrl-alt-del and enabling the \"View -> Show Kernel Times\" curve on\nthe graph shows this fact pretty clearly.\n\nI think this shows that:\n\n1. There's not much overhead in msysGit's stat wrapper at all.  It gets\n   converted quite efficiently to native Windows calls.\n\n2. Windows really sucks at filesystem ops.  Film at 11.\n\nOne thing I dimly recall is that the nightmare interface of\nFindFirstFile/FindNextFile (which I assume is what is actually getting\ncalled for opendir/readdir) actually returns some stat information with\neach file.  Would it be possible to use that directly in some cases\nrather than taking an extra stat call to get it?\n\n-bcd\n"},{"id":"66048","messageId":"alpine.LFD.1.00.0801201144300.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801201519320.5731@racer.site","subject":"Re: [PATCH] Also use unpack_trees() in do_diff_cache()","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T20:32:56Z","receivedAt":"2008-01-20T20:32:56Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 20 Jan 2008, Johannes Schindelin wrote:\n> \n> As in run_diff_index(), we call unpack_trees() with the oneway_diff()\n> function in do_diff_cache() now.  This makes the function diff_cache()\n> obsolete.\n\nSadly, this doesn't work yet.\n\ndiiff_cache() does magic things with different stages. \n\nThe new unpack_trees() model *should* make it possible to do an even \nbetter job at this (we should really approximate the diff_files() \nbehaviour), but oneway_diff() currently doesn't do it right.\n\nTry\n\n\tgit diff-index [-p] [--cached] -m HEAD\n\nin a tree with unmerged entries (or just \"git diff HEAD\", which does the \nsame thing).\n\nThe following patch gets us closer, but really only for the \"--cached\" \ncase (for the non-cached case, it should use the working tree entry ratehr \nthan generate a unmerged entry), and I also suspect there's a case it gets \nwrong for the case of the unmerged path not existign in the tree at all \n(the \"if (tree)\" entry basically ends up being a stand-in for the \"is this \nthe first index entry for this path we see\" case).\n\nThe really nice thing would be to do what \"diff_files()\" does for this \nall, something that the old model couldn't do at all (that's why it \ncurrently prints out that \"* Unmerged path xyzzy\" thing - the *correct* \nthing to do would be to generate the same combined diff that diff_files() \ndoes).\n\nSo some more work is needed - either to get back the old (arguably \nquite broken) behaviour, or to go the whole way and do it right.\n\nAnyway, my point with this all is that even without any of this set of \nfixes, git doesn't do this optimally. Try this with the current stable git \nmaster:\n\n\tgit diff --cached HEAD\n\nwith a unmerged file, and compare it to the much more useful \"git diff\" \noutput. But right now, this patch series makes this case behave even \nworse, I think.\n\n\t\tLinus\n\n---\n diff-lib.c |    6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 23d0fa6..3ac4d1f 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -685,6 +685,12 @@ static int oneway_diff(struct cache_entry **src,\n \tif (tree == o->df_conflict_entry)\n \t\ttree = NULL;\n \n+\tif (ce_stage(idx)) {\n+\t\tif (tree)\n+\t\t\tdiff_unmerge(&revs->diffopt, idx->name, idx->ce_mode, idx->sha1);\n+\t\treturn 1;\n+\t}\n+\n \t/*\n \t * Something added to the tree?\n \t */\n"},{"id":"66050","messageId":"65FCA357-7F7E-4FE0-A423-2528A43B915D@zib.de","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801200942191.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2008-01-20T21:40:38Z","receivedAt":"2008-01-20T21:40:38Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Jan 20, 2008, at 7:20 PM, Linus Torvalds wrote:\n\n>\n> On Sun, 20 Jan 2008, Steffen Prohaska wrote:\n>\n> So your test-case does have relatively more overhead (compared to  \n> what got\n> optimized) than the numbers I quoted.\n\nI cloned the kernel tree and measured these numbers:\n\nBest time before:\n\n     $ time git commit >/dev/null\n     real    0m1.487s\n     user    0m0.786s\n     sys     0m0.696s\n\nBest time after:\n\n     $ time git commit >/dev/null\n     real    0m1.087s\n     user    0m0.578s\n     sys     0m0.508s\n\n\n> We also do know that while Linux has a very low-overhead lstat(),  \n> the real\n> problem on OS X has been the memory management, not the filesystem.  \n> We had\n> some case where the page fault overhead was something like two  \n> orders of\n> magnitude bigger, didn't we?\n>\n> (Yeah - just checked. Commit 6d2fa7f1b489c65e677c18eda5c144dbc5d614ab:\n> \"index-pack usage of mmap() is unacceptably slower..\"). That took a  \n> minute\n> on Linux, and an hour on OS X)\n>\n> It would be good to have a system-level profile of something like  \n> this. On\n> Linux, it's now mostly in user space with \"git commit\", and oprofile\n> shows:\n\n[...]\n\nI used Mac OS X's Sampler to collect some statistics.\n\nIt took me some time to find out about and work around an\nannoying limitation.  Sampler is not capable of simply sampling a\nfull run of a process but needs to be manually stopped before the\nprocess terminates.  You literally have to push a button before\nthe process exits--what a crazy tool.  I worked around this by\nadding a sleep(100) instruction.  Sampler allows you to prune the\nsamples collected in mach_wait_until() and the relative numbers\nshould be fine.\n\nHere is what I have.\n\nBefore Junio's and your patch, lstat dominates with 25%:\n\n# Report - Timed Samples for git-commit\n# Samples < 20 have been filtered\n# Generated from the visible portion of the outline view\n+ 25% lstat\n| + 25% lstat\n| | - 17% wt_status_print\n| | - 8.4% refresh_index\n+ 16% getdirentries\n| + 16% getdirentries\n| | - 16% read_directory_recursive\n+ 10% fnmatch\n| + 10% fnmatch\n| | + 10% excluded\n| | | - 10% read_directory_recursive\n+ 9.7% sha1_block_data_order\n| + 9.7% sha1_block_data_order\n| | + 9.7% SHA1_Update\n| | | + 8.0% read_index_from\n| | | | - 6.8% wt_read_cache\n| | | - 1.6% ce_write\n+ 8.6% cache_name_compare\n| + 8.6% cache_name_compare\n| | - 6.9% cmp_cache_name_compare\n| | - 1.6% index_name_pos\n- 5.2% __mmap\n- 3.2% open\n+ 2.0% excluded\n| + 2.0% excluded\n| | - 2.0% read_directory_recursive\n- 1.6% mbrtowc_l\n- 1.5% find_pack_entry_one\n- 1.4% cmp_cache_name_compare\n- 1.3% qsort\n\n\n\nAfter your patches getdirentries and fnmatch dominate:\n\n# Report - Timed Samples for git-commit\n# Samples < 20 have been filtered\n# Generated from the visible portion of the outline view\n+ 29% getdirentries\n| + 29% getdirentries\n| | - 29% read_directory_recursive\n+ 19% fnmatch\n| + 19% fnmatch\n| | + 19% excluded\n| | | - 19% read_directory_recursive\n+ 9.6% lstat\n| + 9.6% lstat\n| | - 9.5% refresh_index\n+ 5.5% open\n| + 5.5% open\n| | + 3.9% excluded\n| | | - 3.9% read_directory_recursive\n| | - 1.6% read_directory_recursive\n+ 3.9% cache_name_compare\n| + 3.9% cache_name_compare\n| | - 3.8% index_name_pos\n+ 3.0% sha1_block_data_order\n| + 3.0% sha1_block_data_order\n| | + 3.0% SHA1_Update\n| | | - 1.6% read_index_from\n+ 2.8% excluded\n| + 2.8% excluded\n| | - 2.8% read_directory_recursive\n- 2.5% __memcpy\n- 1.9% strcmp\n- 1.6% close\n- 1.4% fnmatch1\n\n\nThe reports above were created with Sampler's \"Invert call tree\"\noption, while the next one is generated without this option.\n\n# Report - Timed Samples for git-commit\n# Samples < 20 have been filtered\n# Generated from the visible portion of the outline view\n+ 85% run_status\n| + 85% wt_status_print\n| | + 73% read_directory\n| | | + 73% read_directory_recursive\n| | | | + 73% read_directory_recursive\n| | | | | + 71% read_directory_recursive\n| | | | | | + 44% read_directory_recursive\n| | | | | | | + 15% excluded\n| | | | | | | | - 12% fnmatch\n| | | | | | | | - 1.6% open\n| | | | | | | - 12% getdirentries\n| | | | | | | + 10% read_directory_recursive\n| | | | | | | | - 4.0% getdirentries\n| | | | | | | | - 2.6% excluded\n| | | | | | | |   1.7% read_directory_recursive\n| | | | | | | - 2.6% dir_add_name\n| | | | | | + 14% excluded\n| | | | | | | - 11% fnmatch\n| | | | | | - 8.0% getdirentries\n| | | | | | - 1.6% dir_add_name\n| | - 11% run_diff_index\n+ 15% prepare_index\n| + 10% refresh_index\n| | - 9.7% lstat\n| - 2.5% write_index\n| - 1.9% read_index\n\nWith this kind of report you can see how the running time is\ndistributed over the different functions called by cmd_commit().\n\n\n> and one thing to look out for would be that some of these things  \n> might be\n> relatively much more costly on other systems.\n>\n> fnmatch? It's certainly visible on Linux, I wonder if the Windows/OSX\n> version is more expensive due to trying to be case-insensitive or\n> something?\n\nSo roughly 50% of the running time is spent in getdirentries\nand fnmatch on the MacBook Pro I used to run these tests.\n\n\tSteffen\n"},{"id":"66053","messageId":"alpine.LFD.1.00.0801201338140.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801201144300.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Also use unpack_trees() in do_diff_cache()","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T21:53:15Z","receivedAt":"2008-01-20T21:53:15Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 20 Jan 2008, Linus Torvalds wrote:\n> \n> The following patch gets us closer, but really only for the \"--cached\" \n> case (for the non-cached case, it should use the working tree entry ratehr \n> than generate a unmerged entry), and I also suspect there's a case it gets \n> wrong for the case of the unmerged path not existign in the tree at all \n> (the \"if (tree)\" entry basically ends up being a stand-in for the \"is this \n> the first index entry for this path we see\" case).\n\nThis cleanup patch really looks big and fairly ugly, but it splits up the \nmagic rules for \"unpack_trees()\" and adds a lot of comments about what is \ngoing on, and in the presense lays the groundwork for doing a much better \njob on unmerged entries - it now keeps track of how many index entries we \nhave that match the tree entry we found, so it should be pretty trivial \nto now add the logic to do a combined diff..\n\nIt replaces the previous patch, and should fix both of the issues I \nmention above. It adds a lot more lines than it removes, but much of that \nis due to some comments and a lot cleaner abstractions, so that the \nresulting code is, I think, quite a bit more obvious.\n\nThe unpack_trees() interface really isn't trivial (it can't be: the things \nwe need to do with the index for a merge - at the same time as traversing \nit! - are quite involved). So clarifying all that is definitely worth it.\n\nWith this patch in place, I think Dscho's two other patches are now good \nto go, and we now generate the same output we used to. In addition, we now \nhave the infrastructure to generate *better* output, so if we want to make \n\"git diff HEAD\" generate a nice combined diff, this sets the stage for \nthat.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n diff-lib.c |  112 ++++++++++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 90 insertions(+), 22 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 23d0fa6..2a3a9ff 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -668,45 +668,113 @@ static void mark_merge_entries(void)\n \t}\n }\n \n-static int oneway_diff(struct cache_entry **src,\n-\tstruct unpack_trees_options *o,\n-\tint remove)\n+/*\n+ * This gets a mix of an existing index and a tree, one pathname entry\n+ * at a time. The index entry may be a single stage-0 one, but it could\n+ * also be multiple unmerged entries (in which case you idx_pos/idx_nr\n+ * will give you the position and number of entries in the index).\n+ */\n+static void do_oneway_diff(struct unpack_trees_options *o,\n+\tstruct cache_entry *idx,\n+\tstruct cache_entry *tree,\n+\tint idx_pos, int idx_nr)\n {\n-\tstruct cache_entry *idx = src[0];\n-\tstruct cache_entry *tree = src[1];\n \tstruct rev_info *revs = o->unpack_data;\n \n-\t/*\n-\t * Unpack-trees generates a DF/conflict entry if\n-\t * there was a directory in the index and a tree\n-\t * in the tree. From a diff standpoint, that's a\n-\t * delete of the tree and a create of the file.\n-\t */\n-\tif (tree == o->df_conflict_entry)\n-\t\ttree = NULL;\n+\tif (o->index_only && idx && ce_stage(idx)) {\n+\t\tif (tree)\n+\t\t\tdiff_unmerge(&revs->diffopt, idx->name, idx->ce_mode, idx->sha1);\n+\t\treturn;\n+\t}\n \n \t/*\n \t * Something added to the tree?\n \t */\n \tif (!tree) {\n-\t\tif (ce_path_match(idx, revs->prune_data))\n-\t\t\tshow_new_file(revs, idx, o->index_only, 0);\n-\t\treturn 1;\n+\t\tshow_new_file(revs, idx, o->index_only, 0);\n+\t\treturn;\n \t}\n \n \t/*\n \t * Something removed from the tree?\n \t */\n \tif (!idx) {\n-\t\tif (ce_path_match(tree, revs->prune_data))\n-\t\t\tdiff_index_show_file(revs, \"-\", tree, tree->sha1, tree->ce_mode);\n-\t\treturn 0;\n+\t\tdiff_index_show_file(revs, \"-\", tree, tree->sha1, tree->ce_mode);\n+\t\treturn;\n \t}\n \n \t/* Show difference between old and new */\n-\tif (ce_path_match(idx, revs->prune_data))\n-\t\tshow_modified(revs, tree, idx, 1, o->index_only, 0);\n-\treturn 1;\n+\tshow_modified(revs, tree, idx, 1, o->index_only, 0);\n+}\n+\n+/*\n+ * Count how many index entries go with the first one\n+ */\n+static inline int count_skip(const struct cache_entry *src, int pos)\n+{\n+\tint skip = 1;\n+\n+\t/* We can only have multiple entries if the first one is not stage-0 */\n+\tif (ce_stage(src)) {\n+\t\tstruct cache_entry **p = active_cache + pos;\n+\t\tint namelen = ce_namelen(src);\n+\n+\t\tfor (;;) {\n+\t\t\tconst struct cache_entry *ce;\n+\t\t\tpos++;\n+\t\t\tif (pos >= active_nr)\n+\t\t\t\tbreak;\n+\t\t\tce = *++p;\n+\t\t\tif (ce_namelen(ce) != namelen)\n+\t\t\t\tbreak;\n+\t\t\tif (memcmp(ce->name, src->name, namelen))\n+\t\t\t\tbreak;\n+\t\t\tskip++;\n+\t\t}\n+\t}\n+\treturn skip;\n+}\n+\n+/*\n+ * The unpack_trees() interface is designed for merging, so\n+ * the different source entries are designed primarily for\n+ * the source trees, with the old index being really mainly\n+ * used for being replaced by the result.\n+ *\n+ * For diffing, the index is more important, and we only have a\n+ * single tree.  \n+ *\n+ * We're supposed to return how many index entries we want to skip.\n+ *\n+ * This wrapper makes it all more readable, and takes care of all\n+ * the fairly complex unpack_trees() semantic requirements, including\n+ * the skipping, the path matching, the type conflict cases etc.\n+ */\n+static int oneway_diff(struct cache_entry **src,\n+\tstruct unpack_trees_options *o,\n+\tint index_pos)\n+{\n+\tint skip = 0;\n+\tstruct cache_entry *idx = src[0];\n+\tstruct cache_entry *tree = src[1];\n+\tstruct rev_info *revs = o->unpack_data;\n+\n+\tif (index_pos >= 0)\n+\t\tskip = count_skip(idx, index_pos);\n+\n+\t/*\n+\t * Unpack-trees generates a DF/conflict entry if\n+\t * there was a directory in the index and a tree\n+\t * in the tree. From a diff standpoint, that's a\n+\t * delete of the tree and a create of the file.\n+\t */\n+\tif (tree == o->df_conflict_entry)\n+\t\ttree = NULL;\n+\n+\tif (ce_path_match(idx ? idx : tree, revs->prune_data))\n+\t\tdo_oneway_diff(o, idx, tree, index_pos, skip);\n+\n+\treturn skip;\n }\n \n int run_diff_index(struct rev_info *revs, int cached)\n"},{"id":"66055","messageId":"alpine.LFD.1.00.0801201358280.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"65FCA357-7F7E-4FE0-A423-2528A43B915D@zib.de","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T22:09:26Z","receivedAt":"2008-01-20T22:09:26Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 20 Jan 2008, Steffen Prohaska wrote:\n> > \n> > fnmatch? It's certainly visible on Linux, I wonder if the Windows/OSX\n> > version is more expensive due to trying to be case-insensitive or\n> > something?\n> \n> So roughly 50% of the running time is spent in getdirentries\n> and fnmatch on the MacBook Pro I used to run these tests.\n\nOk. That seems to explain it (I have some trouble reading/comparing your \nprofiles against mine, but yeah, fnmatch and getdirentries seem to be \nexcessive from that \"invert call tree\" thing.\n\nOn Linux, the fnmatch and readdir overhead is way down in the noise both \nbefore and after the lstat removal. So removing the lstat's - which were \nfairly cheap, but there were _lots_ of them, made more of a relative \ndifference on Linux.\n\nI don't know what can be done about that on OS X. We really can't avoid \nreading the directory tree and matching it against ignore files. That's \nvery integral for the status generation of \"git commit\", after all.\n\nMaybe there are better fnmatch libraries for OS X? But getdirentries() was \nthe bigger cost, and I don't see any alternative for that.\n\n\t\t\tLinus\n"},{"id":"66060","messageId":"alpine.LSU.1.00.0801202331380.5731@racer.site","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801201338140.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Also use unpack_trees() in do_diff_cache()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-20T23:34:23Z","receivedAt":"2008-01-20T23:34:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jan 2008, Linus Torvalds wrote:\n\n> In addition, we now have the infrastructure to generate *better* output, \n> so if we want to make \"git diff HEAD\" generate a nice combined diff, \n> this sets the stage for that.\n\nNote: \"git diff HEAD\" as it is now still holds value; it tells you about \n_all_ changes -- staged and unstaged -- and I have to admit that I used it \na lot (as \"git diff HEAD -- file | git apply -R\"; note the absence of \n\"--index\" to the call to apply, otherwise \"git checkout file\" would have \ndone the same).\n\nSo if we ever do combined diffs for \"git diff HEAD\", I want an option to \nturn them off, too.\n\nCiao,\nDscho\n"},{"id":"66061","messageId":"alpine.LFD.1.00.0801201550550.2957@woody.linux-foundation.org","threadId":"11671","inReplyTo":"alpine.LSU.1.00.0801202331380.5731@racer.site","subject":"Re: [PATCH] Also use unpack_trees() in do_diff_cache()","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-20T23:58:22Z","receivedAt":"2008-01-20T23:58:22Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 20 Jan 2008, Johannes Schindelin wrote:\n> \n> Note: \"git diff HEAD\" as it is now still holds value;\n\nOh, absolutely.\n\nIt's not \"git diff HEAD\" that is broken.\n\nIt's \"git diff --cached HEAD\" that doesn't work. The \"--cached\" means that \nit's supposed to diff the index against HEAD, but since it cannot handle \nunmerged entries, instead of getting a diff, you get just a line saying\n\n\t* Unmerged path xyzzy\n\nand no diff at all.\n\nThat's the thing we should think about improving on (although it's not \n100% clear that a combined diff is the rigth thing either).\n\nAnyway, to make it easier for people to follow along with this, I've put \nmy whole series now on kernel.org in a \"new-lstat\" branch at:\n\n\tgit://git.kernel.org/pub/scm/linux/kernel/git/torvalds/git.git new-lstat\n\n(That git repo is a bit odd, it doesn't have any \"master\" branch at all, \njust that new-lstat one. I didn't want anybody to think that it's \nanything but a temporary git tree for this one series of patches. It \nwill clone into an odd kind of repo that only has a \"origin/new-lstat\" \nremote branch, nothing else).\n\nI'll probably rebase that thing if I keep working on it, but I think it's \npretty good.\n\n\t\tLinus\n"},{"id":"66063","messageId":"7vwsq458vw.fsf@gitster.siamese.dyndns.org","threadId":"11671","inReplyTo":"07FDB6ED-EBE0-4F66-9167-6A0F8AD58E2B@zib.de","subject":"Re: [PATCH] Avoid running lstat(2) on the same cache entry.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-21T00:18:11Z","receivedAt":"2008-01-21T00:18:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Prohaska <prohaska@zib.de> writes:\n\n> On Jan 20, 2008, at 2:48 AM, Johannes Schindelin wrote:\n>\n>>> I bet you'll see a much bigger performance improvement from this on\n>>> Windows in particular.\n>>\n>> I bet so, too.  Traditionally, filesystem calls are painfully slow on\n>> Windows.\n>>\n>> But I cannot test before Monday, so I would not be mad if somebody\n>> else\n>> could perform some tests on Windows.\n>\n> Has someone collected the whole series on a topic branch?\n\nYes, I have.\n\nHaven't pushed it out, though.\n"},{"id":"66064","messageId":"alpine.LSU.1.00.0801210017580.5731@racer.site","threadId":"11671","inReplyTo":"alpine.LFD.1.00.0801201550550.2957@woody.linux-foundation.org","subject":"Re: [PATCH] Also use unpack_trees() in do_diff_cache()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-01-21T00:19:38Z","receivedAt":"2008-01-21T00:19:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jan 2008, Linus Torvalds wrote:\n\n> On Sun, 20 Jan 2008, Johannes Schindelin wrote:\n> > \n> > Note: \"git diff HEAD\" as it is now still holds value;\n> \n> Oh, absolutely.\n> \n> It's not \"git diff HEAD\" that is broken.\n> \n> It's \"git diff --cached HEAD\" that doesn't work. The \"--cached\" means \n> that it's supposed to diff the index against HEAD, but since it cannot \n> handle unmerged entries, instead of getting a diff, you get just a line \n> saying\n> \n> \t* Unmerged path xyzzy\n> \n> and no diff at all.\n\nSilly me.  Unmerged paths can only have problems when you look at the \nindex, of course.\n\nSorry for the noise,\nDscho\n"}]}