{"thread":{"id":"28742","subject":"a bug when execute \"git status\" in git version 1.7.7.431.g89633","startedAt":"2011-10-22T00:20:43Z","lastAt":"2011-10-25T18:00:04Z","messageCount":17,"participants":["John Hsing","Matthieu Moy","René Scharfe","Jeff King","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"178155","messageId":"4EA20C5B.3090808@gmail.com","threadId":"28742","inReplyTo":null,"subject":"a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"John Hsing","fromEmail":"tsyj2007@gmail.com","sentAt":"2011-10-22T00:20:43Z","receivedAt":"2011-10-22T00:20:43Z","isPatch":false,"sender":{"key":"tsyj2007@gmail.com","avatar":null},"body":"the error:\ngit: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr)\n(((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct\nmalloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >=\n(unsigned long)((((__builtin_offsetof (struct malloc_chunk,\nfd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) -\n1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask)\n== 0)' failed.\n"},{"id":"178189","messageId":"vpqfwiknmh3.fsf@bauges.imag.fr","threadId":"28742","inReplyTo":"4EA20C5B.3090808@gmail.com","subject":"Re: a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2011-10-23T08:25:28Z","receivedAt":"2011-10-23T08:25:28Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"John Hsing <tsyj2007@gmail.com> writes:\n\n> the error:\n\nWhen does this error occur? is it reproducible on any repository? Which\nOS?\n\n> git: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr)\n> (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct\n> malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >=\n> (unsigned long)((((__builtin_offsetof (struct malloc_chunk,\n> fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) -\n> 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask)\n> == 0)' failed.\n\nThe assertion is violated outside the source code of Git itself, so\neither Git calls malloc incorrectly, or this is a bug in your libc.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"178190","messageId":"4EA3D1BB.2010802@gmail.com","threadId":"28742","inReplyTo":"vpqfwiknmh3.fsf@bauges.imag.fr","subject":"Re: a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"John Hsing","fromEmail":"tsyj2007@gmail.com","sentAt":"2011-10-23T08:35:07Z","receivedAt":"2011-10-23T08:35:07Z","isPatch":false,"sender":{"key":"tsyj2007@gmail.com","avatar":null},"body":"ok,when i finish compiling git 1.7.7.431.g89633，I use it to check \nCyanogenod（an Android mod source） by “git status”，it happend this \nerror！but when i reuse git v1.7.7，it is OK！so i think it is a bug in \ngit 1.7.7.431.g89633！My OS is Ubuntu Linux 10.10，sorry for my bad \nenglish！ If you want to reproduce this error,please excute \"git status\" \nin https://github.com/CyanogenMod/android_packages_apps_DSPManager.git \nrepo!\n\n于2011年10月23日 星期日 16时25分28秒,Matthieu Moy写到:\n> John Hsing <tsyj2007@gmail.com> writes:\n>\n>> the error:\n>\n> When does this error occur? is it reproducible on any repository? Which\n> OS?\n>\n>> git: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr)\n>> (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct\n>> malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >=\n>> (unsigned long)((((__builtin_offsetof (struct malloc_chunk,\n>> fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) -\n>> 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask)\n>> == 0)' failed.\n>\n> The assertion is violated outside the source code of Git itself, so\n> either Git calls malloc incorrectly, or this is a bug in your libc.\n>\n"},{"id":"178200","messageId":"4EA415BD.1040109@lsrfire.ath.cx","threadId":"28742","inReplyTo":"4EA3D1BB.2010802@gmail.com","subject":"Re: a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-23T13:25:17Z","receivedAt":"2011-10-23T13:25:17Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.10.2011 10:35, schrieb John Hsing:\n> ok,when i finish compiling git 1.7.7.431.g89633，I use it to check\n> Cyanogenod（an Android mod source） by “git status”，it happend this\n> error！but when i reuse git v1.7.7，it is OK！so i think it is a bug in\n> git 1.7.7.431.g89633！My OS is Ubuntu Linux 10.10，sorry for my bad\n> english！ If you want to reproduce this error,please excute \"git status\"\n> in https://github.com/CyanogenMod/android_packages_apps_DSPManager.git\n> repo!\n\nI can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:\n\n\t$ a=android_packages_apps_DSPManager\n\t$ git-v1.7.7 clone https://github.com/CyanogenMod/$a.git\n\tCloning into android_packages_apps_DSPManager...\n\tremote: Counting objects: 902, done.\n\tremote: Compressing objects: 100% (412/412), done.\n\tremote: Total 902 (delta 367), reused 838 (delta 324)\n\tReceiving objects: 100% (902/902), 136.78 KiB | 264 KiB/s, done.\n\tResolving deltas: 100% (367/367), done.\n\t$ cd $a\n\n\t$ git-v1.7.7 status\n\t# On branch gingerbread\n\tnothing to commit (working directory clean)\n\n\t$ git-master status\n\tgit: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.\n\tAborted\n\nBisect points to 2548183ba, \"fix phantom untracked files when\ncore.ignorecase is set\" from Jeff (cc:d).  If I revert that patch from\nmaster (8963314c), git status works fine.\n\nThe following experimental patch fixes it for me as well, but I can't\nclaim to know exactly why.  In any case, estimate_cache_size() seems\nto guess too low.\n\nRené\n\n\n---\n read-cache.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 01a0e25..b143bd3 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1257,7 +1257,7 @@ static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entrie\n \t * Alignment can cause differences. This should be \"alignof\", but\n \t * since that's a gcc'ism, just use the size of a pointer.\n \t */\n-\tper_entry += sizeof(void *);\n+\tper_entry += 2 * sizeof(void *);\n \treturn ondisk_size + entries*per_entry;\n }\n \n"},{"id":"178201","messageId":"4EA42470.1070501@lsrfire.ath.cx","threadId":"28742","inReplyTo":"4EA415BD.1040109@lsrfire.ath.cx","subject":"Re: a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-23T14:28:00Z","receivedAt":"2011-10-23T14:28:00Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.10.2011 15:25, schrieb René Scharfe:\n> Am 23.10.2011 10:35, schrieb John Hsing:\n>> ok,when i finish compiling git 1.7.7.431.g89633，I use it to check\n>> Cyanogenod（an Android mod source） by “git status”，it happend this\n>> error！but when i reuse git v1.7.7，it is OK！so i think it is a bug in\n>> git 1.7.7.431.g89633！My OS is Ubuntu Linux 10.10，sorry for my bad\n>> english！ If you want to reproduce this error,please excute \"git status\"\n>> in https://github.com/CyanogenMod/android_packages_apps_DSPManager.git\n>> repo!\n> \n> I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:\n> \n> \t$ a=android_packages_apps_DSPManager\n> \t$ git-v1.7.7 clone https://github.com/CyanogenMod/$a.git\n> \tCloning into android_packages_apps_DSPManager...\n> \tremote: Counting objects: 902, done.\n> \tremote: Compressing objects: 100% (412/412), done.\n> \tremote: Total 902 (delta 367), reused 838 (delta 324)\n> \tReceiving objects: 100% (902/902), 136.78 KiB | 264 KiB/s, done.\n> \tResolving deltas: 100% (367/367), done.\n> \t$ cd $a\n> \n> \t$ git-v1.7.7 status\n> \t# On branch gingerbread\n> \tnothing to commit (working directory clean)\n> \n> \t$ git-master status\n> \tgit: malloc.c:3096: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.\n> \tAborted\n\nAnd valgrind reports the following errors (git was compiled with -O0 and\n-g) for master, but not for 1.7.7 nor master plus my ugly patch:\n\nInvalid write of size 1\n   at 0x4029DE5: memcpy (mc_replace_strmem.c:635)\n   by 0x81159A5: convert_from_disk (read-cache.c:1247)\n   by 0x8115BE4: read_index_from (read-cache.c:1326)\n   by 0x81157D7: read_index (read-cache.c:1202)\n   by 0x813A802: gitmodules_config (submodule.c:105)\n   by 0x806857E: cmd_status (commit.c:1209)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n Address 0x41f2d5e is 2 bytes after a block of size 6,356 alloc'd\n   at 0x4028876: malloc (vg_replace_malloc.c:236)\n   by 0x814C070: xmalloc (wrapper.c:35)\n   by 0x8115B8E: read_index_from (read-cache.c:1315)\n   by 0x81157D7: read_index (read-cache.c:1202)\n   by 0x813A802: gitmodules_config (submodule.c:105)\n   by 0x806857E: cmd_status (commit.c:1209)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n\nSyscall param lstat64(file_name) points to unaddressable byte(s)\n   at 0x4131D32: __lxstat64@@GLIBC_2.2 (lxstat64.c:48)\n   by 0x81154DE: refresh_index (read-cache.c:1133)\n   by 0x8068687: cmd_status (commit.c:1226)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd\n   at 0x4028876: malloc (vg_replace_malloc.c:236)\n   by 0x814C070: xmalloc (wrapper.c:35)\n   by 0x8115B8E: read_index_from (read-cache.c:1315)\n   by 0x81157D7: read_index (read-cache.c:1202)\n   by 0x813A802: gitmodules_config (submodule.c:105)\n   by 0x806857E: cmd_status (commit.c:1209)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n\nInvalid read of size 1\n   at 0x402A682: bcmp (mc_replace_strmem.c:679)\n   by 0x8113D12: df_name_compare (read-cache.c:387)\n   by 0x81478F9: do_compare_entry (unpack-trees.c:499)\n   by 0x814791B: compare_entry (unpack-trees.c:504)\n   by 0x8148086: unpack_callback (unpack-trees.c:747)\n   by 0x8145EA5: traverse_trees (tree-walk.c:407)\n   by 0x81477EB: traverse_trees_recursive (unpack-trees.c:460)\n   by 0x814823D: unpack_callback (unpack-trees.c:809)\n   by 0x8145EA5: traverse_trees (tree-walk.c:407)\n   by 0x81477EB: traverse_trees_recursive (unpack-trees.c:460)\n   by 0x814823D: unpack_callback (unpack-trees.c:809)\n   by 0x8145EA5: traverse_trees (tree-walk.c:407)\n Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd\n   at 0x4028876: malloc (vg_replace_malloc.c:236)\n   by 0x814C070: xmalloc (wrapper.c:35)\n   by 0x8115B8E: read_index_from (read-cache.c:1315)\n   by 0x81157D7: read_index (read-cache.c:1202)\n   by 0x813A802: gitmodules_config (submodule.c:105)\n   by 0x806857E: cmd_status (commit.c:1209)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n\nInvalid read of size 1\n   at 0x8100E0E: hash_name (name-hash.c:28)\n   by 0x8100F3D: hash_index_entry (name-hash.c:78)\n   by 0x8100FD3: lazy_init_name_hash (name-hash.c:96)\n   by 0x8101197: index_name_exists (name-hash.c:159)\n   by 0x80EADA9: dir_add_name (dir.c:596)\n   by 0x80EB8CD: read_directory_recursive (dir.c:994)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EBCED: read_directory (dir.c:1101)\n Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd\n   at 0x4028876: malloc (vg_replace_malloc.c:236)\n   by 0x814C070: xmalloc (wrapper.c:35)\n   by 0x8115B8E: read_index_from (read-cache.c:1315)\n   by 0x81157D7: read_index (read-cache.c:1202)\n   by 0x813A802: gitmodules_config (submodule.c:105)\n   by 0x806857E: cmd_status (commit.c:1209)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n\nInvalid read of size 1\n   at 0x402A687: bcmp (mc_replace_strmem.c:679)\n   by 0x8113DF4: cache_name_compare (read-cache.c:413)\n   by 0x81010F8: same_name (name-hash.c:134)\n   by 0x81011E0: index_name_exists (name-hash.c:164)\n   by 0x80EADA9: dir_add_name (dir.c:596)\n   by 0x80EB8CD: read_directory_recursive (dir.c:994)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n   by 0x80EB895: read_directory_recursive (dir.c:983)\n Address 0x41f2d5c is 0 bytes after a block of size 6,356 alloc'd\n   at 0x4028876: malloc (vg_replace_malloc.c:236)\n   by 0x814C070: xmalloc (wrapper.c:35)\n   by 0x8115B8E: read_index_from (read-cache.c:1315)\n   by 0x81157D7: read_index (read-cache.c:1202)\n   by 0x813A802: gitmodules_config (submodule.c:105)\n   by 0x806857E: cmd_status (commit.c:1209)\n   by 0x804B7F8: run_builtin (git.c:308)\n   by 0x804B956: handle_internal_command (git.c:466)\n   by 0x804BA4E: run_argv (git.c:512)\n   by 0x804BBC0: main (git.c:585)\n"},{"id":"178205","messageId":"20111023162944.GB28156@sigill.intra.peff.net","threadId":"28742","inReplyTo":"4EA415BD.1040109@lsrfire.ath.cx","subject":"Re: a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-23T16:29:44Z","receivedAt":"2011-10-23T16:29:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 23, 2011 at 03:25:17PM +0200, René Scharfe wrote:\n\n> I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:\n> [...]\n> Bisect points to 2548183ba, \"fix phantom untracked files when\n> core.ignorecase is set\" from Jeff (cc:d).  If I revert that patch from\n> master (8963314c), git status works fine.\n\nHmm. Interesting. I can't reproduce here. And I've been running with\nthis patch for over a year, and never seen that. Given your fix, I guess\nit's related to pointer size. Are you on a 32-bit machine, by any\nchance?\n\n-Peff\n"},{"id":"178206","messageId":"4EA453D3.7080002@lsrfire.ath.cx","threadId":"28742","inReplyTo":"20111023162944.GB28156@sigill.intra.peff.net","subject":"Re: a bug when execute \"git status\" in git version 1.7.7.431.g89633","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-23T17:50:11Z","receivedAt":"2011-10-23T17:50:11Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.10.2011 18:29, schrieb Jeff King:\n> On Sun, Oct 23, 2011 at 03:25:17PM +0200, René Scharfe wrote:\n> \n>> I can reproduce the malloc crash on Ubuntu 11.10 with these simple steps:\n>> [...]\n>> Bisect points to 2548183ba, \"fix phantom untracked files when\n>> core.ignorecase is set\" from Jeff (cc:d).  If I revert that patch from\n>> master (8963314c), git status works fine.\n> \n> Hmm. Interesting. I can't reproduce here. And I've been running with\n> this patch for over a year, and never seen that. Given your fix, I guess\n> it's related to pointer size. Are you on a 32-bit machine, by any\n> chance?\n\nYes, it's a 32-bit VM.  I think it's a case of unlucky filename lengths,\ncombined with the rounding up to the next multiple of 8.  The following\ntable lists the actual size needed for entries based on the length of\ntheir name entry.  Length calculation uses these offsets:\n\n\toffsetof(struct cache_entry, name) == 72\n\toffsetof(struct ondisk_cache_entry, name) == 62\n\n\tlen  ce_size                 ondisk_ce_size          delta\n\t  1  (72 + 1 + 8) & ~7 = 80  (62 + 1 + 8) & ~7 = 64     16\n\t  2  (72 + 2 + 8) & ~7 = 80  (62 + 2 + 8) & ~7 = 72      8\n\t  3  (72 + 3 + 8) & ~7 = 80  (62 + 3 + 8) & ~7 = 72      8\n\t  4  (72 + 4 + 8) & ~7 = 80  (62 + 4 + 8) & ~7 = 72      8\n\t  5  (72 + 5 + 8) & ~7 = 80  (62 + 5 + 8) & ~7 = 72      8\n\t  6  (72 + 6 + 8) & ~7 = 80  (62 + 6 + 8) & ~7 = 72      8\n\t  7  (72 + 7 + 8) & ~7 = 80  (62 + 7 + 8) & ~7 = 72      8\n\t  8  (72 + 8 + 8) & ~7 = 88  (62 + 8 + 8) & ~7 = 72     16\n\nSo in 25% of the cases an entry needs 16 bytes more in memory than on\ndisk and the rest needs 8 bytes more.\n\nestimate_cache_size() calculates the amount of memory needed for the\nindex, based on its on-disk representation.  It simply adds the\ndifference of the sizes of the two structs and the size of a pointer for\neach entry to its total size and returns that number.  I have:\n\n\tsizeof(void *) == 4\n\tsizeof(struct cache_entry) == 72\n\tsizeof(struct ondisk_cache_entry) == 64\n\nSo each entry gets 72 - 64 + 4 = 12 bytes extra.  If you happen to have\na lot of filenames with a delta of 16 then the resulting size won't be\nenough to hold the in-memory index.\n\nIs there a nice way to derive that we need 16 bytes per entry in the\nworst case, preferably without trying all eight possibilities as I did\nin the table above?  My modular math is rusty..\n\nRené\n"},{"id":"178209","messageId":"4EA4B8E7.5070106@lsrfire.ath.cx","threadId":"28742","inReplyTo":"4EA453D3.7080002@lsrfire.ath.cx","subject":"[PATCH] read-cache.c: fix index memory allocation","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-24T01:01:27Z","receivedAt":"2011-10-24T01:01:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"estimate_cache_size() tries to guess how much memory is needed for the\nin-memory representation of an index file.  It does that by using the\nfile size, the number of entries and the difference of the sizes of the\non-disk and in-memory structs -- without having to check the length of\nthe name of each entry, which varies for each entry, but their sums are\nthe same no matter the representation.\n\nExcept there can be a difference.  First of all, the size is really\ncalculated by ce_size and ondisk_ce_size based on offsetof(..., name),\nnot sizeof, which can be different.  And entries are padded with 1 to 8\nNULs at the end (after the variable name) to make their total length a\nmultiple of eight.\n\nSo in order to allocate enough memory to hold the index, change the\ndelta calculation to be based on offsetof(..., name) and round up to\nthe next multiple of eight.\n\nOn a 32-bit Linux, this delta was used before:\n\n\tsizeof(struct cache_entry)        == 72\n\tsizeof(struct ondisk_cache_entry) == 64\n\t                                    ---\n\t                                      8\n\nThe actual difference for an entry with a filename length of one was,\nhowever (find the definitions are in cache.h):\n\n\toffsetof(struct cache_entry, name)        == 72\n\toffsetof(struct ondisk_cache_entry, name) == 62\n\n\tce_size        == (72 + 1 + 8) & ~7 == 80\n\tondisk_ce_size == (62 + 1 + 8) & ~7 == 64\n\t                                      ---\n\t                                       16\n\nSo eight bytes less had been allocated for such entries.  The new\nformula yields the correct delta:\n\n\t(72 - 62 + 7) & ~7 == 16\n\nReported-by: John Hsing <tsyj2007@gmail.com>\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n read-cache.c            |    6 ++--\n t/t7510-status-index.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 53 insertions(+), 3 deletions(-)\n create mode 100755 t/t7510-status-index.sh\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 01a0e25..5790a91 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1249,9 +1249,9 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en\n \n static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n {\n-\tlong per_entry;\n-\n-\tper_entry = sizeof(struct cache_entry) - sizeof(struct ondisk_cache_entry);\n+\tsize_t fix_size_mem = offsetof(struct cache_entry, name);\n+\tsize_t fix_size_dsk = offsetof(struct ondisk_cache_entry, name);\n+\tlong per_entry = (fix_size_mem - fix_size_dsk + 7) & ~7;\n \n \t/*\n \t * Alignment can cause differences. This should be \"alignof\", but\ndiff --git a/t/t7510-status-index.sh b/t/t7510-status-index.sh\nnew file mode 100755\nindex 0000000..bca359d\n--- /dev/null\n+++ b/t/t7510-status-index.sh\n@@ -0,0 +1,50 @@\n+#!/bin/sh\n+\n+test_description='git status with certain file name lengths'\n+\n+. ./test-lib.sh\n+\n+files=\"0 1 2 3 4 5 6 7 8 9 a b c d e f g h i j k l m n o p q r s t u v w x y z\"\n+\n+check() {\n+\tlen=$1\n+\tprefix=$2\n+\n+\tfor i in $files\n+\tdo\n+\t\t: >$prefix$i\n+\tdone\n+\n+\ttest_expect_success \"status, filename length $len\" \"\n+\t\tgit add $prefix* &&\n+\t\tgit status\n+\t\"\n+\trm $prefix* .git/index\n+}\n+\n+check  1\n+check  2 p\n+check  3 pr\n+check  4 pre\n+check  5 pref\n+check  6 prefi\n+check  7 prefix\n+check  8 prefix-\n+check  9 prefix-p\n+check 10 prefix-pr\n+check 11 prefix-pre\n+check 12 prefix-pref\n+check 13 prefix-prefi\n+check 14 prefix-prefix\n+check 15 prefix-prefix-\n+check 16 prefix-prefix-p\n+check 17 prefix-prefix-pr\n+check 18 prefix-prefix-pre\n+check 19 prefix-prefix-pref\n+check 20 prefix-prefix-prefi\n+check 21 prefix-prefix-prefix\n+check 22 prefix-prefix-prefix-\n+check 23 prefix-prefix-prefix-p\n+check 24 prefix-prefix-prefix-pr\n+\n+test_done\n-- \n1.7.7\n"},{"id":"178227","messageId":"7vipne50lz.fsf@alter.siamese.dyndns.org","threadId":"28742","inReplyTo":"4EA4B8E7.5070106@lsrfire.ath.cx","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-24T07:07:20Z","receivedAt":"2011-10-24T07:07:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n\nThis approach may be the most appropriate for the maintenance track, but\nfor the purpose of going forward, I wonder if we really want to keep the\n\"estimate and allocate a large pool, and carve out individual pieces\".\n\nThis bulk-allocate dates back to the days when we didn't have ondisk vs\nincore representation differences, IIRC, and as the result we deliberately\nleak cache entries whenever an entry in the index is replaced with a new\none. Does the overhead to allocate individually really kill us that much\nfor say a tree with 30k files in it?\n"},{"id":"178228","messageId":"7vaa8q4zm9.fsf@alter.siamese.dyndns.org","threadId":"28742","inReplyTo":"4EA4B8E7.5070106@lsrfire.ath.cx","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-24T07:28:46Z","receivedAt":"2011-10-24T07:28:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n>  t/t7510-status-index.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 53 insertions(+), 3 deletions(-)\n>  create mode 100755 t/t7510-status-index.sh\n\n> diff --git a/t/t7510-status-index.sh b/t/t7510-status-index.sh\n> new file mode 100755\n> index 0000000..bca359d\n> --- /dev/null\n> +++ b/t/t7510-status-index.sh\n> @@ -0,0 +1,50 @@\n\nHmm, I cannot seem to make this fail this test without the fix on my\nFedora 14 i686 VM when applied to v1.7.6.4 (estimation code originates\ncf55870 back in v1.7.6.1 days), but it does break on 'master'.\n\nBy the way, I'll move this to 7511.\n\nAlso would a patch like this help?\n\n-- >8 --\nSubject: [PATCH] read_index(): die on estimation error\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n read-cache.c |    7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 0a64103..2926615 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1270,6 +1270,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \tint fd, i;\n \tstruct stat st;\n \tunsigned long src_offset, dst_offset;\n+\tsize_t bulk_alloc_size;\n \tstruct cache_header *hdr;\n \tvoid *mmap;\n \tsize_t mmap_size;\n@@ -1315,7 +1316,8 @@ int read_index_from(struct index_state *istate, const char *path)\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(estimate_cache_size(mmap_size, istate->cache_nr));\n+\tbulk_alloc_size = estimate_cache_size(mmap_size, istate->cache_nr);\n+\tistate->alloc = xmalloc(bulk_alloc_size);\n \tistate->initialized = 1;\n \n \tsrc_offset = sizeof(*hdr);\n@@ -1331,7 +1333,9 @@ int read_index_from(struct index_state *istate, const char *path)\n \n \t\tsrc_offset += ondisk_ce_size(ce);\n \t\tdst_offset += ce_size(ce);\n+\t\tif (bulk_alloc_size <= dst_offset)\n+\t\t\tdie(\"cache size estimation error\");\n \t}\n \tistate->timestamp.sec = st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n \n-- \n1.7.7.1.504.gcc718\n"},{"id":"178236","messageId":"4EA589D9.2000402@lsrfire.ath.cx","threadId":"28742","inReplyTo":"7vaa8q4zm9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-24T15:52:57Z","receivedAt":"2011-10-24T15:52:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.10.2011 09:28, schrieb Junio C Hamano:\n> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n> \n>>  t/t7510-status-index.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++\n>>  2 files changed, 53 insertions(+), 3 deletions(-)\n>>  create mode 100755 t/t7510-status-index.sh\n> \n>> diff --git a/t/t7510-status-index.sh b/t/t7510-status-index.sh\n>> new file mode 100755\n>> index 0000000..bca359d\n>> --- /dev/null\n>> +++ b/t/t7510-status-index.sh\n>> @@ -0,0 +1,50 @@\n> \n> Hmm, I cannot seem to make this fail this test without the fix on my\n> Fedora 14 i686 VM when applied to v1.7.6.4 (estimation code originates\n> cf55870 back in v1.7.6.1 days), but it does break on 'master'.\n\nErr, yes, I forgot to mention in the commit message that on my test\nsystem the breakage occurs only after 2548183ba, \"fix phantom untracked\nfiles when core.ignorecase is set\", which adds the pointer dir_next to\nstruct cache_entry.  This seems to have caused an unlucky constellation\nof offsets and struct sizes for the size estimator.\n\n> By the way, I'll move this to 7511.\n> \n> Also would a patch like this help?\n\nOnly a little, I suspect.  If we've moved past the end then it's too\nlate.  And if we catch the error before it happens, dying is only\nslightly better than crashing.\n\n> -- >8 --\n> Subject: [PATCH] read_index(): die on estimation error\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  read-cache.c |    7 ++++++-\n>  1 files changed, 6 insertions(+), 1 deletions(-)\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index 0a64103..2926615 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1270,6 +1270,7 @@ int read_index_from(struct index_state *istate, const char *path)\n>  \tint fd, i;\n>  \tstruct stat st;\n>  \tunsigned long src_offset, dst_offset;\n> +\tsize_t bulk_alloc_size;\n>  \tstruct cache_header *hdr;\n>  \tvoid *mmap;\n>  \tsize_t mmap_size;\n> @@ -1315,7 +1316,8 @@ int read_index_from(struct index_state *istate, const char *path)\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(estimate_cache_size(mmap_size, istate->cache_nr));\n> +\tbulk_alloc_size = estimate_cache_size(mmap_size, istate->cache_nr);\n> +\tistate->alloc = xmalloc(bulk_alloc_size);\n>  \tistate->initialized = 1;\n>  \n>  \tsrc_offset = sizeof(*hdr);\n> @@ -1331,7 +1333,9 @@ int read_index_from(struct index_state *istate, const char *path)\n>  \n>  \t\tsrc_offset += ondisk_ce_size(ce);\n>  \t\tdst_offset += ce_size(ce);\n> +\t\tif (bulk_alloc_size <= dst_offset)\n> +\t\t\tdie(\"cache size estimation error\");\n>  \t}\n>  \tistate->timestamp.sec = st.st_mtime;\n>  \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n>  \n"},{"id":"178237","messageId":"4EA58B63.8040800@lsrfire.ath.cx","threadId":"28742","inReplyTo":"7vipne50lz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-24T15:59:31Z","receivedAt":"2011-10-24T15:59:31Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.10.2011 09:07, schrieb Junio C Hamano:\n> Thanks.\n> \n> This approach may be the most appropriate for the maintenance track, but\n> for the purpose of going forward, I wonder if we really want to keep the\n> \"estimate and allocate a large pool, and carve out individual pieces\".\n> \n> This bulk-allocate dates back to the days when we didn't have ondisk vs\n> incore representation differences, IIRC, and as the result we deliberately\n> leak cache entries whenever an entry in the index is replaced with a new\n> one. Does the overhead to allocate individually really kill us that much\n> for say a tree with 30k files in it?\n\nProbably not; unpack_trees() does that already.  (It calls\ncreate_ce_entry() via unpack_nondirectories() via unpack_callback() via\ntraverse_trees()).\n\nRené\n"},{"id":"178248","messageId":"4EA5DFB2.3050406@lsrfire.ath.cx","threadId":"28742","inReplyTo":"7vipne50lz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-24T21:59:14Z","receivedAt":"2011-10-24T21:59:14Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.10.2011 09:07, schrieb Junio C Hamano:\n> Thanks.\n> \n> This approach may be the most appropriate for the maintenance track, but\n> for the purpose of going forward, I wonder if we really want to keep the\n> \"estimate and allocate a large pool, and carve out individual pieces\".\n> \n> This bulk-allocate dates back to the days when we didn't have ondisk vs\n> incore representation differences, IIRC, and as the result we deliberately\n> leak cache entries whenever an entry in the index is replaced with a new\n> one. Does the overhead to allocate individually really kill us that much\n> for say a tree with 30k files in it?\n\nSomething like this (applies to master)?  Very basic testing didn't show\nany slowdown of git status in the Linux repo.\n\n-- >8 --\nSubject: read-cache.c: allocate index entries individually\n\nThe code to estimate the in-memory size of the index based on its on-disk\nrepresentation is subtly wrong for certain architecture-dependent struct\nlayouts.  Instead of fixing it, replace the code to keep the index entries\nin a single large block of memory and allocate each entry separately\ninstead.  This is both simpler and more flexible, as individual entries\ncan now be freed.  Actually using that added flexibility is left for a\nlater patch.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\nNote: This patch was brought to you by the option --histogram, which\nproduced nicer output than the default diff method.\n\n cache.h      |    1 -\n read-cache.c |   86 +++++++++++++++++++++++----------------------------------\n 2 files changed, 35 insertions(+), 52 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex be07ec7..ec0e571 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -316,7 +316,6 @@ struct index_state {\n \tstruct string_list *resolve_undo;\n \tstruct cache_tree *cache_tree;\n \tstruct cache_time timestamp;\n-\tvoid *alloc;\n \tunsigned name_hash_initialized : 1,\n \t\t initialized : 1;\n \tstruct hash_table name_hash;\ndiff --git a/read-cache.c b/read-cache.c\nindex 01a0e25..7f75bfa 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1202,10 +1202,35 @@ 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+static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk)\n {\n+\tstruct cache_entry *ce;\n \tsize_t len;\n \tconst char *name;\n+\tunsigned int flags;\n+\n+\t/* On-disk flags are just 16 bits */\n+\tflags = ntohs(ondisk->flags);\n+\tlen = flags & CE_NAMEMASK;\n+\n+\tif (flags & CE_EXTENDED) {\n+\t\tstruct ondisk_cache_entry_extended *ondisk2;\n+\t\tint extended_flags;\n+\t\tondisk2 = (struct ondisk_cache_entry_extended *)ondisk;\n+\t\textended_flags = ntohs(ondisk2->flags2) << 16;\n+\t\t/* We do not yet understand any bit out of CE_EXTENDED_FLAGS */\n+\t\tif (extended_flags & ~CE_EXTENDED_FLAGS)\n+\t\t\tdie(\"Unknown index entry format %08x\", extended_flags);\n+\t\tflags |= extended_flags;\n+\t\tname = ondisk2->name;\n+\t}\n+\telse\n+\t\tname = ondisk->name;\n+\n+\tif (len == CE_NAMEMASK)\n+\t\tlen = strlen(name);\n+\n+\tce = xmalloc(cache_entry_size(len));\n \n \tce->ce_ctime.sec = ntohl(ondisk->ctime.sec);\n \tce->ce_mtime.sec = ntohl(ondisk->mtime.sec);\n@@ -1217,48 +1242,16 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en\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+\tce->ce_flags = flags;\n \n \thashcpy(ce->sha1, ondisk->sha1);\n \n-\tlen = ce->ce_flags & CE_NAMEMASK;\n-\n-\tif (ce->ce_flags & CE_EXTENDED) {\n-\t\tstruct ondisk_cache_entry_extended *ondisk2;\n-\t\tint extended_flags;\n-\t\tondisk2 = (struct ondisk_cache_entry_extended *)ondisk;\n-\t\textended_flags = ntohs(ondisk2->flags2) << 16;\n-\t\t/* We do not yet understand any bit out of CE_EXTENDED_FLAGS */\n-\t\tif (extended_flags & ~CE_EXTENDED_FLAGS)\n-\t\t\tdie(\"Unknown index entry format %08x\", extended_flags);\n-\t\tce->ce_flags |= extended_flags;\n-\t\tname = ondisk2->name;\n-\t}\n-\telse\n-\t\tname = ondisk->name;\n-\n-\tif (len == CE_NAMEMASK)\n-\t\tlen = strlen(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, name, len + 1);\n-}\n-\n-static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)\n-{\n-\tlong per_entry;\n-\n-\tper_entry = sizeof(struct cache_entry) - sizeof(struct ondisk_cache_entry);\n-\n-\t/*\n-\t * Alignment can cause differences. This should be \"alignof\", but\n-\t * since that's a gcc'ism, just use the size of a pointer.\n-\t */\n-\tper_entry += sizeof(void *);\n-\treturn ondisk_size + entries*per_entry;\n+\treturn ce;\n }\n \n /* remember to discard_cache() before reading a different cache! */\n@@ -1266,7 +1259,7 @@ int read_index_from(struct index_state *istate, const char *path)\n {\n \tint fd, i;\n \tstruct stat st;\n-\tunsigned long src_offset, dst_offset;\n+\tunsigned long src_offset;\n \tstruct cache_header *hdr;\n \tvoid *mmap;\n \tsize_t mmap_size;\n@@ -1305,29 +1298,18 @@ int read_index_from(struct index_state *istate, const char *path)\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-\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(estimate_cache_size(mmap_size, istate->cache_nr));\n \tistate->initialized = 1;\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\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\tce = create_from_disk(disk_ce);\n \t\tset_index_entry(istate, i, ce);\n \n \t\tsrc_offset += ondisk_ce_size(ce);\n-\t\tdst_offset += ce_size(ce);\n \t}\n \tistate->timestamp.sec = st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n@@ -1361,11 +1343,15 @@ unmap:\n \n int is_index_unborn(struct index_state *istate)\n {\n-\treturn (!istate->cache_nr && !istate->alloc && !istate->timestamp.sec);\n+\treturn (!istate->cache_nr && !istate->timestamp.sec);\n }\n \n int discard_index(struct index_state *istate)\n {\n+\tint i;\n+\n+\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\tfree(istate->cache[i]);\n \tresolve_undo_clear_index(istate);\n \tistate->cache_nr = 0;\n \tistate->cache_changed = 0;\n@@ -1374,8 +1360,6 @@ int discard_index(struct index_state *istate)\n \tistate->name_hash_initialized = 0;\n \tfree_hash(&istate->name_hash);\n \tcache_tree_free(&(istate->cache_tree));\n-\tfree(istate->alloc);\n-\tistate->alloc = NULL;\n \tistate->initialized = 0;\n \n \t/* no need to throw away allocated active_cache */\n-- \n1.7.7\n"},{"id":"178254","messageId":"20111024233427.GA24956@duynguyen-vnpc.dektech.internal","threadId":"28742","inReplyTo":"4EA5DFB2.3050406@lsrfire.ath.cx","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-24T23:34:27Z","receivedAt":"2011-10-24T23:34:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Oct 24, 2011 at 11:59:14PM +0200, René Scharfe wrote:\n> Am 24.10.2011 09:07, schrieb Junio C Hamano:\n> > Thanks.\n> > \n> > This approach may be the most appropriate for the maintenance track, but\n> > for the purpose of going forward, I wonder if we really want to keep the\n> > \"estimate and allocate a large pool, and carve out individual pieces\".\n> > \n> > This bulk-allocate dates back to the days when we didn't have ondisk vs\n> > incore representation differences, IIRC, and as the result we deliberately\n> > leak cache entries whenever an entry in the index is replaced with a new\n> > one. Does the overhead to allocate individually really kill us that much\n> > for say a tree with 30k files in it?\n> \n> Something like this (applies to master)?  Very basic testing didn't show\n> any slowdown of git status in the Linux repo.\n\n\"git status\" is slow. If your changes causes slowdown, it won't likely\nstand out while other fast commands may show (read_cache() is used in\nnearly all commands). So I tested using the following patch.\n\nThe result on linux-2.6 shows about 10-20 us slowdown per each\nread_cache() call (30-40 us on webkit, ~50k files) I think your patch\nis good enough :-)\n\n-- 8< --\ndiff --git a/Makefile b/Makefile\nindex 3139c19..c8b47bc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -444,6 +444,7 @@ TEST_PROGRAMS_NEED_X += test-string-pool\n TEST_PROGRAMS_NEED_X += test-subprocess\n TEST_PROGRAMS_NEED_X += test-svn-fe\n TEST_PROGRAMS_NEED_X += test-treap\n+TEST_PROGRAMS_NEED_X += test-read-cache\n \n TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))\n \ndiff --git a/test-read-cache.c b/test-read-cache.c\nindex e69de29..1ad80a0 100644\n--- a/test-read-cache.c\n+++ b/test-read-cache.c\n@@ -0,0 +1,14 @@\n+#include \"cache.h\"\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct timeval tv1, tv2;\n+\tint i;\n+\n+\tgettimeofday(&tv1, NULL);\n+\tfor (i = 0; i < 100; i++)\n+\t\tread_cache();\n+\tgettimeofday(&tv2, NULL);\n+\tprintf(\"%lu\\n\", (unsigned long)(tv2.tv_usec - tv1.tv_usec));\n+\treturn 0;\n+}\n-- 8< --\n"},{"id":"178256","messageId":"CACsJy8A7yVk15aAgqDkKTz31rChA7Oj-kS2VT2y2tWS6h01GyA@mail.gmail.com","threadId":"28742","inReplyTo":"20111024233427.GA24956@duynguyen-vnpc.dektech.internal","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-25T00:01:14Z","receivedAt":"2011-10-25T00:01:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 25, 2011 at 10:34 AM, Nguyen Thai Ngoc Duy\n<pclouds@gmail.com> wrote:\n> \"git status\" is slow. If your changes causes slowdown, it won't likely\n> stand out while other fast commands may show (read_cache() is used in\n> nearly all commands). So I tested using the following patch.\n>\n> The result on linux-2.6 shows about 10-20 us slowdown per each\n> read_cache() call (30-40 us on webkit, ~50k files) I think your patch\n> is good enough :-)\n\nThat was with -O0 by the way. valgrind/massif shows about 200kb memory\nmore with your patch on webkit repository (7.497 MB vs 7.285 MB),\nusing the same test, so memory overhead is ok too.\n-- \nDuy\n"},{"id":"178277","messageId":"7vmxcp3up7.fsf@alter.siamese.dyndns.org","threadId":"28742","inReplyTo":"4EA5DFB2.3050406@lsrfire.ath.cx","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-25T16:24:52Z","receivedAt":"2011-10-25T16:24:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> Something like this (applies to master)?  Very basic testing didn't show\n> any slowdown of git status in the Linux repo.\n>\n> -- >8 --\n> Subject: read-cache.c: allocate index entries individually\n\nYeah, and code reduction looks nice.\n"},{"id":"178287","messageId":"4EA6F924.6060208@lsrfire.ath.cx","threadId":"28742","inReplyTo":"CACsJy8A7yVk15aAgqDkKTz31rChA7Oj-kS2VT2y2tWS6h01GyA@mail.gmail.com","subject":"Re: [PATCH] read-cache.c: fix index memory allocation","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-25T18:00:04Z","receivedAt":"2011-10-25T18:00:04Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 25.10.2011 02:01, schrieb Nguyen Thai Ngoc Duy:\n> On Tue, Oct 25, 2011 at 10:34 AM, Nguyen Thai Ngoc Duy\n> <pclouds@gmail.com> wrote:\n>> \"git status\" is slow. If your changes causes slowdown, it won't likely\n>> stand out while other fast commands may show (read_cache() is used in\n>> nearly all commands). So I tested using the following patch.\n>>\n>> The result on linux-2.6 shows about 10-20 us slowdown per each\n>> read_cache() call (30-40 us on webkit, ~50k files) I think your patch\n>> is good enough :-)\n> \n> That was with -O0 by the way. valgrind/massif shows about 200kb memory\n> more with your patch on webkit repository (7.497 MB vs 7.285 MB),\n> using the same test, so memory overhead is ok too.\n\nWe can reduce that a bit -- unless block allocation of index entries\nis still done somewhere.\n\n-- >8 --\nSubject: [PATCH 2/1] cache.h: put single NUL at end of struct cache_entry\n\nSince in-memory index entries are allocated individually now, the\nvariable slack at the end meant to provide an eight byte alignment\nis not needed anymore.  Have a single NUL instead.  This saves zero\nto seven bytes for an entry, depending on its filename length.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n\n cache.h |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex ec0e571..bd106b5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -306,7 +306,7 @@ static inline unsigned int canon_mode(unsigned int mode)\n }\n \n #define flexible_size(STRUCT,len) ((offsetof(struct STRUCT,name) + (len) + 8) & ~7)\n-#define cache_entry_size(len) flexible_size(cache_entry,len)\n+#define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + 1)\n #define ondisk_cache_entry_size(len) flexible_size(ondisk_cache_entry,len)\n #define ondisk_cache_entry_extended_size(len) flexible_size(ondisk_cache_entry_extended,len)\n \n-- \n1.7.7\n"}]}