{"thread":{"id":"37231","subject":"[PATCH v3 0/9] Speed up cache loading time","startedAt":"2014-07-28T12:03:06Z","lastAt":"2014-07-31T02:41:36Z","messageCount":13,"participants":["Nguyễn Thái Ngọc Duy","Eric Sunshine","Duy Nguyen","David Turner"],"isPatch":true,"patchVersion":3,"patchTotal":9},"messages":[{"id":"246793","messageId":"1406548995-28549-1-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":null,"subject":"[PATCH v3 0/9] Speed up cache loading time","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:06Z","receivedAt":"2014-07-28T12:03:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is the polished version of v2 [1], rebased on master (now that\nsplit-index has graduated). I wanted to add some tests, but can't\nthink of any way to do it (at least it won't work on Windows).\nAlthough with --strict turned on by default, it's less likely that\nbugs in index-helper could propagate far.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/251186\n\nNguyễn Thái Ngọc Duy (9):\n  trace.c: add GIT_TRACE_PACK_STATS for pack usage statistics\n  read-cache.c: fix constness of verify_hdr()\n  read-cache: allow to keep mmap'd memory after reading\n  index-helper: new daemon for caching index and related stuff\n  trace.c: add GIT_TRACE_INDEX_STATS for index  statistics\n  index-helper: add --strict\n  daemonize(): set a flag before exiting the main process\n  index-helper: add --detach\n  index-helper: add Windows support\n\n .gitignore                               |   1 +\n Documentation/git-index-helper.txt (new) |  56 +++++++\n Documentation/git.txt                    |   4 +\n Makefile                                 |   9 ++\n builtin/gc.c                             |   2 +-\n cache.h                                  |  11 +-\n config.mak.uname                         |   3 +\n daemon.c                                 |   2 +-\n git-compat-util.h                        |   1 +\n git.c                                    |   1 +\n index-helper.c (new)                     | 258 +++++++++++++++++++++++++++++++\n read-cache.c                             | 119 +++++++++++++-\n setup.c                                  |   4 +-\n sha1_file.c                              |  24 +++\n shm.c (new)                              | 163 +++++++++++++++++++\n shm.h (new)                              |  23 +++\n trace.c                                  |  16 ++\n trace.h                                  |   1 +\n 18 files changed, 686 insertions(+), 12 deletions(-)\n create mode 100644 Documentation/git-index-helper.txt\n create mode 100644 index-helper.c\n create mode 100644 shm.c\n create mode 100644 shm.h\n\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246794","messageId":"1406548995-28549-2-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 1/9] trace.c: add GIT_TRACE_PACK_STATS for pack usage statistics","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:07Z","receivedAt":"2014-07-28T12:03:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"trace_stats() is intended for GIT_TRACE_*_STATS variable group and\nGIT_TRACE_PACK_STATS is more like an example how new vars can be added.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git.txt |  3 +++\n cache.h               |  2 ++\n git.c                 |  1 +\n sha1_file.c           | 24 ++++++++++++++++++++++++\n trace.c               | 13 +++++++++++++\n trace.h               |  1 +\n 6 files changed, 44 insertions(+)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex a0f1ede..a124612 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -946,6 +946,9 @@ Unsetting the variable, or setting it to empty, \"0\" or\n \ttime of each Git command.\n \tSee 'GIT_TRACE' for available trace output options.\n \n+'GIT_TRACE_PACK_STATS'::\n+\tPrint various statistics.\n+\n 'GIT_TRACE_SETUP'::\n \tEnables trace messages printing the .git, working tree and current\n \tworking directory after Git has completed its setup phase.\ndiff --git a/cache.h b/cache.h\nindex fcb511d..1c84227 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1502,4 +1502,6 @@ void stat_validity_update(struct stat_validity *sv, int fd);\n \n int versioncmp(const char *s1, const char *s2);\n \n+void report_pack_stats(struct trace_key *key);\n+\n #endif /* CACHE_H */\ndiff --git a/git.c b/git.c\nindex 9c49519..6a6a8a8 100644\n--- a/git.c\n+++ b/git.c\n@@ -614,6 +614,7 @@ int main(int argc, char **av)\n \tgit_setup_gettext();\n \n \ttrace_command_performance(argv);\n+\ttrace_stats();\n \n \t/*\n \t * \"git-xxxx\" is the same as \"git xxxx\", but we obviously:\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 3f70b1d..21a487b 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -467,6 +467,7 @@ static unsigned int peak_pack_open_windows;\n static unsigned int pack_open_windows;\n static unsigned int pack_open_fds;\n static unsigned int pack_max_fds;\n+static unsigned int pack_access_nr;\n static size_t peak_pack_mapped;\n static size_t pack_mapped;\n struct packed_git *packed_git;\n@@ -492,6 +493,28 @@ void pack_report(void)\n \t\tsz_fmt(pack_mapped), sz_fmt(peak_pack_mapped));\n }\n \n+void report_pack_stats(struct trace_key *key)\n+{\n+\ttrace_printf_key(key, \"\\n\"\n+\t\t\t \"pack_report: getpagesize()            = %10\" SZ_FMT \"\\n\"\n+\t\t\t \"pack_report: core.packedGitWindowSize = %10\" SZ_FMT \"\\n\"\n+\t\t\t \"pack_report: core.packedGitLimit      = %10\" SZ_FMT \"\\n\"\n+\t\t\t \"pack_report: pack_used_ctr            = %10u\\n\"\n+\t\t\t \"pack_report: pack_mmap_calls          = %10u\\n\"\n+\t\t\t \"pack_report: pack_open_windows        = %10u / %10u\\n\"\n+\t\t\t \"pack_report: pack_mapped              = \"\n+\t\t\t \"%10\" SZ_FMT \" / %10\" SZ_FMT \"\\n\"\n+\t\t\t \"pack_report: pack accesss             = %10u\\n\",\n+\t\t\t sz_fmt(getpagesize()),\n+\t\t\t sz_fmt(packed_git_window_size),\n+\t\t\t sz_fmt(packed_git_limit),\n+\t\t\t pack_used_ctr,\n+\t\t\t pack_mmap_calls,\n+\t\t\t pack_open_windows, peak_pack_open_windows,\n+\t\t\t sz_fmt(pack_mapped), sz_fmt(peak_pack_mapped),\n+\t\t\t pack_access_nr);\n+}\n+\n /*\n  * Open and mmap the index file at path, perform a couple of\n  * consistency checks, then record its information to p.  Return 0 on\n@@ -2086,6 +2109,7 @@ static void write_pack_access_log(struct packed_git *p, off_t obj_offset)\n \tstatic struct trace_key pack_access = TRACE_KEY_INIT(PACK_ACCESS);\n \ttrace_printf_key(&pack_access, \"%s %\"PRIuMAX\"\\n\",\n \t\t\t p->pack_name, (uintmax_t)obj_offset);\n+\tpack_access_nr++;\n }\n \n int do_check_packed_object_crc;\ndiff --git a/trace.c b/trace.c\nindex e583dc6..c52da0f 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -426,3 +426,16 @@ void trace_command_performance(const char **argv)\n \tsq_quote_argv(&command_line, argv, 0);\n \tcommand_start_time = getnanotime();\n }\n+\n+static struct trace_key trace_pack_stats = TRACE_KEY_INIT(PACK_STATS);\n+\n+static void print_stats_atexit(void)\n+{\n+\treport_pack_stats(&trace_pack_stats);\n+}\n+\n+void trace_stats(void)\n+{\n+\tif (trace_want(&trace_pack_stats))\n+\t\tatexit(print_stats_atexit);\n+}\ndiff --git a/trace.h b/trace.h\nindex ae6a332..9c06e82 100644\n--- a/trace.h\n+++ b/trace.h\n@@ -18,6 +18,7 @@ extern int trace_want(struct trace_key *key);\n extern void trace_disable(struct trace_key *key);\n extern uint64_t getnanotime(void);\n extern void trace_command_performance(const char **argv);\n+extern void trace_stats(void);\n \n #ifndef HAVE_VARIADIC_MACROS\n \n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246795","messageId":"1406548995-28549-3-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 2/9] read-cache.c: fix constness of verify_hdr()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:08Z","receivedAt":"2014-07-28T12:03:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n read-cache.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 5d3c8bd..949270b 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1324,7 +1324,7 @@ struct ondisk_cache_entry_extended {\n \t\t\t    ondisk_cache_entry_extended_size(ce_namelen(ce)) : \\\n \t\t\t    ondisk_cache_entry_size(ce_namelen(ce)))\n \n-static int verify_hdr(struct cache_header *hdr, unsigned long size)\n+static int verify_hdr(const struct cache_header *hdr, unsigned long size)\n {\n \tgit_SHA_CTX c;\n \tunsigned char sha1[20];\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246796","messageId":"1406548995-28549-4-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 3/9] read-cache: allow to keep mmap'd memory after reading","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:09Z","receivedAt":"2014-07-28T12:03:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n cache.h      |  3 +++\n read-cache.c | 13 ++++++++++++-\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex 1c84227..978e53d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -306,10 +306,13 @@ struct index_state {\n \tstruct split_index *split_index;\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n+\t\t keep_mmap : 1,\n \t\t initialized : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\n \tunsigned char sha1[20];\n+\tvoid *mmap;\n+\tsize_t mmap_size;\n };\n \n extern struct index_state the_index;\ndiff --git a/read-cache.c b/read-cache.c\nindex 949270b..b679781 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1498,6 +1498,10 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tmmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);\n \tif (mmap == MAP_FAILED)\n \t\tdie_errno(\"unable to map index file\");\n+\tif (istate->keep_mmap) {\n+\t\tistate->mmap = mmap;\n+\t\tistate->mmap_size = mmap_size;\n+\t}\n \tclose(fd);\n \n \thdr = mmap;\n@@ -1550,10 +1554,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t\tsrc_offset += 8;\n \t\tsrc_offset += extsize;\n \t}\n-\tmunmap(mmap, mmap_size);\n+\tif (!istate->keep_mmap)\n+\t\tmunmap(mmap, mmap_size);\n \treturn istate->cache_nr;\n \n unmap:\n+\tistate->mmap = NULL;\n \tmunmap(mmap, mmap_size);\n \tdie(\"index file corrupt\");\n }\n@@ -1579,6 +1585,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\tdiscard_index(split_index->base);\n \telse\n \t\tsplit_index->base = xcalloc(1, sizeof(*split_index->base));\n+\tsplit_index->base->keep_mmap = istate->keep_mmap;\n \tret = do_read_index(split_index->base,\n \t\t\t    git_path(\"sharedindex.%s\",\n \t\t\t\t     sha1_to_hex(split_index->base_sha1)), 1);\n@@ -1621,6 +1628,10 @@ int discard_index(struct index_state *istate)\n \tfree(istate->cache);\n \tistate->cache = NULL;\n \tistate->cache_alloc = 0;\n+\tif (istate->keep_mmap && istate->mmap) {\n+\t\tmunmap(istate->mmap, istate->mmap_size);\n+\t\tistate->mmap = NULL;\n+\t}\n \tdiscard_split_index(istate);\n \treturn 0;\n }\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246797","messageId":"1406548995-28549-5-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 4/9] index-helper: new daemon for caching index and related stuff","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:10Z","receivedAt":"2014-07-28T12:03:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Instead of reading the index from disk and worrying about disk\ncorruption, the index is cached in memory (memory bit-flips happen\ntoo, but hopefully less often). The result is faster read.\n\nThe biggest gain is not having to verify the trailing SHA-1, which\ntakes lots of time especially on large index files. But this also\nopens doors for further optimiztions:\n\n - we could create an in-memory format that's essentially the memory\n   dump of the index to eliminate most of parsing/allocation\n   overhead. The mmap'd memory can be used straight away.\n\n - we could cache non-index info such as name hash\n\nThe shared memory's name folows the template \"git-<object>-<SHA1>\"\nwhere <SHA1> is the trailing SHA-1 of the index file. <object> is\n\"index\" for cached index files (and may be \"name-hash\" for name-hash\ncache). If such shared memory exists, it contains the same index\ncontent as on disk. The content is already validated by the daemon and\ngit won't validate it again (except comparing the trailing SHA-1s).\n\nGit can poke the daemon to tell it to refresh the index cache, or to\nkeep it alive some more minutes via UNIX signals. It can't give any\nreal index data directly to the daemon. Real data goes to disk first,\nthen the daemon reads and verifies it from there.\n\n$GIT_DIR/index-helper.pid contains a reference to daemon process (and\nit's pid on *nix). The file's mtime is updated every time it's accessed\n(or should be updated often enough). Old index-helper.pid is considered\nstale and ignored.\n\nindex-helper requires POSIX realtime extension. POSIX shm interface\nhowever is abstracted away so that Windows support can be added later.\n\nOn webkit.git with index format v2, duplicating 8 times to 1.4m\nentries and 200MB in size:\n\n  (vanilla)      0.986986364 s: read_index_from .git/index\n  (index-helper) 0.267850279 s: read_index_from .git/index\n\nInterestingly with index v4, we get less out of index-helper. It makes\nsense as v4 requires more processing after loading the index:\n\n  (vanilla)      0.722496666 s: read_index_from .git/index\n  (index-helper) 0.302741500 s: read_index_from .git/index\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n .gitignore                               |   1 +\n Documentation/git-index-helper.txt (new) |  44 +++++++++\n Makefile                                 |   9 ++\n cache.h                                  |   2 +\n config.mak.uname                         |   1 +\n git-compat-util.h                        |   1 +\n index-helper.c (new)                     | 157 +++++++++++++++++++++++++++++++\n read-cache.c                             |  85 +++++++++++++++--\n shm.c (new)                              |  67 +++++++++++++\n shm.h (new)                              |  23 +++++\n 10 files changed, 381 insertions(+), 9 deletions(-)\n create mode 100644 Documentation/git-index-helper.txt\n create mode 100644 index-helper.c\n create mode 100644 shm.c\n create mode 100644 shm.h\n\ndiff --git a/.gitignore b/.gitignore\nindex 81e12c0..66148ef 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -71,6 +71,7 @@\n /git-http-fetch\n /git-http-push\n /git-imap-send\n+/git-index-helper\n /git-index-pack\n /git-init\n /git-init-db\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nnew file mode 100644\nindex 0000000..da41c4d\n--- /dev/null\n+++ b/Documentation/git-index-helper.txt\n@@ -0,0 +1,44 @@\n+git-index-helper(1)\n+===================\n+\n+NAME\n+----\n+git-index-helper - A simple cache daemon for speeding up index file access\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git index-helper' [options]\n+\n+DESCRIPTION\n+-----------\n+Keep the index file in memory for faster access. This daemon is per\n+repository.\n+\n+OPTIONS\n+-------\n+\n+--exit-after=<n>::\n+\tExit if the cached index is not accessed for `<n>`\n+\tminutes. Specify 0 to wait forever. Default is 10.\n+\n+NOTES\n+-----\n+On UNIX-like systems, $GIT_DIR/index-helper.pid contains the process\n+id of the daemon. At least on Linux, shared memory objects are\n+availble via /dev/shm with the name pattern \"git-<something>-<SHA1>\".\n+Normally the daemon will clean up shared memory objects when it exits.\n+But if it crashes, some objects could remain there and they can be\n+safely deleted with \"rm\" command. The following signals are used to\n+control the daemon:\n+\n+SIGHUP::\n+\tReread the index.\n+\n+SIGUSR1::\n+\tThis signal is counted as \"an access\", which keeps the daemon\n+\talive longer, unless `--exit-after=0` is used.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 2320de5..24734e6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -342,6 +342,8 @@ all::\n # return NULL when it receives a bogus time_t.\n #\n # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.\n+#\n+# Define HAVE_SHM if you platform support shm_* functions in librt.\n \n GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n@@ -876,6 +878,7 @@ LIB_OBJS += sha1-lookup.o\n LIB_OBJS += sha1_file.o\n LIB_OBJS += sha1_name.o\n LIB_OBJS += shallow.o\n+LIB_OBJS += shm.o\n LIB_OBJS += sideband.o\n LIB_OBJS += sigchain.o\n LIB_OBJS += split-index.o\n@@ -1469,6 +1472,12 @@ ifdef HAVE_DEV_TTY\n \tBASIC_CFLAGS += -DHAVE_DEV_TTY\n endif\n \n+ifdef HAVE_SHM\n+\tBASIC_CFLAGS\t+= -DHAVE_SHM\n+\tEXTLIBS\t        += -lrt\n+\tPROGRAM_OBJS\t+= index-helper.o\n+endif\n+\n ifdef DIR_HAS_BSD_GROUP_SEMANTICS\n \tCOMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS\n endif\ndiff --git a/cache.h b/cache.h\nindex 978e53d..29a0c53 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -499,6 +499,7 @@ extern int is_index_unborn(struct index_state *);\n extern int read_index_unmerged(struct index_state *);\n #define COMMIT_LOCK\t\t(1 << 0)\n #define CLOSE_LOCK\t\t(1 << 1)\n+#define REFRESH_DAEMON\t\t(1 << 2)\n extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);\n extern int discard_index(struct index_state *);\n extern int unmerged_index(const struct index_state *);\n@@ -621,6 +622,7 @@ extern size_t packed_git_limit;\n extern size_t delta_base_cache_limit;\n extern unsigned long big_file_threshold;\n extern unsigned long pack_size_limit_cfg;\n+extern int dont_poke_index_helper;\n \n /*\n  * Do replace refs need to be checked this run?  This variable is\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 00cf4c6..7e66d80 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)\n \tLIBC_CONTAINS_LIBINTL = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n \tHAVE_CLOCK_GETTIME = YesPlease\n+\tHAVE_SHM = YesPlease\n endif\n ifeq ($(uname_S),GNU/kFreeBSD)\n \tHAVE_ALLOCA_H = YesPlease\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 26e92f1..13eef92 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -417,6 +417,7 @@ static inline int ends_with(const char *str, const char *suffix)\n #define PROT_READ 1\n #define PROT_WRITE 2\n #define MAP_PRIVATE 1\n+#define MAP_SHARED 2\n #endif\n \n #define mmap git_mmap\ndiff --git a/index-helper.c b/index-helper.c\nnew file mode 100644\nindex 0000000..8fa0af9\n--- /dev/null\n+++ b/index-helper.c\n@@ -0,0 +1,157 @@\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"sigchain.h\"\n+#include \"exec_cmd.h\"\n+#include \"split-index.h\"\n+#include \"shm.h\"\n+\n+struct index_shm {\n+\tunsigned char sha1[20];\n+\tvoid *shm;\n+\tsize_t size;\n+};\n+\n+static struct index_shm shm_index;\n+static struct index_shm shm_base_index;\n+\n+static void free_index_shm(struct index_shm *is)\n+{\n+\tif (!is->shm)\n+\t\treturn;\n+\tmunmap(is->shm, is->size);\n+\tgit_shm_unlink(\"git-index-%s\", sha1_to_hex(is->sha1));\n+\tis->shm = NULL;\n+}\n+\n+static void cleanup_shm(void)\n+{\n+\tfree_index_shm(&shm_index);\n+\tfree_index_shm(&shm_base_index);\n+}\n+\n+static void cleanup(void)\n+{\n+\tunlink(git_path(\"index-helper.pid\"));\n+\tcleanup_shm();\n+}\n+\n+static void cleanup_on_signal(int sig)\n+{\n+\tcleanup();\n+\tsigchain_pop(sig);\n+\traise(sig);\n+}\n+\n+static void share_index(struct index_state *istate, struct index_shm *is)\n+{\n+\tvoid *new_mmap;\n+\tif (istate->mmap_size <= 20 ||\n+\t    hashcmp(istate->sha1,\n+\t\t    (unsigned char *)istate->mmap + istate->mmap_size - 20) ||\n+\t    !hashcmp(istate->sha1, is->sha1) ||\n+\t    git_shm_map(O_CREAT | O_EXCL | O_RDWR, 0700, istate->mmap_size,\n+\t\t\t&new_mmap, PROT_READ | PROT_WRITE, MAP_SHARED,\n+\t\t\t\"git-index-%s\", sha1_to_hex(istate->sha1)) < 0)\n+\t\treturn;\n+\n+\tfree_index_shm(is);\n+\tis->size = istate->mmap_size;\n+\tis->shm = new_mmap;\n+\thashcpy(is->sha1, istate->sha1);\n+\tmemcpy(new_mmap, istate->mmap, istate->mmap_size - 20);\n+\n+\t/*\n+\t * The trailing hash must be written last after everything is\n+\t * written. It's the indication that the shared memory is now\n+\t * ready.\n+\t */\n+\thashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);\n+}\n+\n+static void refresh(int sig)\n+{\n+\tthe_index.keep_mmap = 1;\n+\tif (read_cache() < 0)\n+\t\tdie(_(\"could not read index\"));\n+\tif (the_index.split_index && the_index.split_index->base)\n+\t\tshare_index(the_index.split_index->base, &shm_base_index);\n+\tshare_index(&the_index, &shm_index);\n+\tdiscard_index(&the_index);\n+}\n+\n+#ifdef HAVE_SHM\n+\n+static void do_nothing(int sig)\n+{\n+\t/*\n+\t * what we need is the signal received and interrupts\n+\t * sleep(). We don't need to do anything else when receving\n+\t * the signal\n+\t */\n+}\n+\n+static void loop(const char *pid_file, int idle_in_seconds)\n+{\n+\tsigchain_pop(SIGHUP);\t/* pushed by sigchain_push_common */\n+\tsigchain_push(SIGHUP, refresh);\n+\tsigchain_push(SIGUSR1, do_nothing);\n+\trefresh(0);\n+\twhile (sleep(idle_in_seconds))\n+\t\t; /* do nothing, all is handled by signal handlers already */\n+}\n+\n+#else\n+\n+static void loop(const char *pid_file, int idle_in_seconds)\n+{\n+\tdie(_(\"index-helper is not supported on this platform\"));\n+}\n+\n+#endif\n+\n+static const char * const usage_text[] = {\n+\tN_(\"git index-helper [options]\"),\n+\tNULL\n+};\n+\n+int main(int argc, char **argv)\n+{\n+\tstatic struct lock_file lock;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *prefix;\n+\tint fd, idle_in_minutes = 10;\n+\tstruct option options[] = {\n+\t\tOPT_INTEGER(0, \"exit-after\", &idle_in_minutes,\n+\t\t\t    N_(\"exit if not used after some minutes\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_extract_argv0_path(argv[0]);\n+\tgit_setup_gettext();\n+\tdont_poke_index_helper = 1; /* cyclic poking is no fun */\n+\n+\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n+\t\tusage_with_options(usage_text, options);\n+\n+\tprefix = setup_git_directory();\n+\tif (parse_options(argc, (const char **)argv, prefix,\n+\t\t\t  options, usage_text, 0))\n+\t\tdie(_(\"too many arguments\"));\n+\n+\tfd = hold_lock_file_for_update(&lock,\n+\t\t\t\t       git_path(\"index-helper.pid\"),\n+\t\t\t\t       LOCK_DIE_ON_ERROR);\n+\tstrbuf_addf(&sb,\n+\t\t    \"%\" PRIuMAX, (uintmax_t) getpid());\n+\twrite_in_full(fd, sb.buf, sb.len);\n+\tcommit_lock_file(&lock);\n+\n+\tatexit(cleanup);\n+\tsigchain_push_common(cleanup_on_signal);\n+\n+\tif (!idle_in_minutes)\n+\t\tidle_in_minutes = 0xffffffff / 60;\n+\tloop(sb.buf, idle_in_minutes * 60);\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\ndiff --git a/read-cache.c b/read-cache.c\nindex b679781..ff28142 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -16,6 +16,7 @@\n #include \"varint.h\"\n #include \"split-index.h\"\n #include \"sigchain.h\"\n+#include \"shm.h\"\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,\n \t\t\t\t\t       unsigned int options);\n@@ -45,6 +46,7 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,\n \n struct index_state the_index;\n static const char *alternate_index_output;\n+int dont_poke_index_helper;\n \n static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)\n {\n@@ -1465,6 +1467,65 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk,\n \treturn ce;\n }\n \n+static void poke_daemon(const struct stat *st, int refresh_cache)\n+{\n+\tint fd;\n+\tstruct strbuf sb;\n+\n+\tif (dont_poke_index_helper)\n+\t\treturn;\n+\tfd = open(git_path(\"index-helper.pid\"), O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn;\n+\tstrbuf_init(&sb, st->st_size + 1);\n+\tstrbuf_setlen(&sb, st->st_size);\n+\tif (read_in_full(fd, sb.buf, st->st_size) == st->st_size) {\n+#ifdef HAVE_SHM\n+\t\tchar *end = NULL;\n+\t\tpid_t pid = strtoul(sb.buf, &end, 10);\n+\t\tif (end && sb.buf + st->st_size == end)\n+\t\t\tkill(pid, refresh_cache ? SIGHUP : SIGUSR1);\n+#endif\n+\t}\n+\tclose(fd);\n+\tstrbuf_release(&sb);\n+}\n+\n+/*\n+ * Try to open and verify a cached shm index if available. Return 0 if\n+ * succeeds (istate->mmap and istate->mmap_size are updated). Return\n+ * negative otherwise.\n+ */\n+static int try_shm(struct index_state *istate)\n+{\n+\tvoid *new_mmap = NULL;\n+\tsize_t old_size = istate->mmap_size;\n+\tssize_t new_length;\n+\tconst unsigned char *sha1;\n+\tstruct stat st;\n+\n+\tif (old_size <= 20 ||\n+\t    stat(git_path(\"index-helper.pid\"), &st))\n+\t\treturn -1;\n+\tsha1 = (unsigned char *)istate->mmap + old_size - 20;\n+\tnew_length = git_shm_map(O_RDONLY, 0700, -1, &new_mmap,\n+\t\t\t\t PROT_READ, MAP_SHARED,\n+\t\t\t\t \"git-index-%s\", sha1_to_hex(sha1));\n+\tif (new_length <= 20 ||\n+\t    hashcmp((unsigned char *)istate->mmap + old_size - 20,\n+\t\t    (unsigned char *)new_mmap + new_length - 20)) {\n+\t\tif (new_mmap)\n+\t\t\tmunmap(new_mmap, new_length);\n+\t\tpoke_daemon(&st, 1);\n+\t\treturn -1;\n+\t}\n+\tmunmap(istate->mmap, istate->mmap_size);\n+\tistate->mmap = new_mmap;\n+\tistate->mmap_size = new_length;\n+\tpoke_daemon(&st, 0);\n+\treturn 0;\n+}\n+\n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int must_exist)\n {\n@@ -1498,15 +1559,17 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tmmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);\n \tif (mmap == MAP_FAILED)\n \t\tdie_errno(\"unable to map index file\");\n-\tif (istate->keep_mmap) {\n-\t\tistate->mmap = mmap;\n-\t\tistate->mmap_size = mmap_size;\n-\t}\n \tclose(fd);\n \n-\thdr = mmap;\n-\tif (verify_hdr(hdr, mmap_size) < 0)\n+\tistate->mmap = mmap;\n+\tistate->mmap_size = mmap_size;\n+\tif (try_shm(istate) &&\n+\t    verify_hdr(istate->mmap, istate->mmap_size) < 0)\n \t\tgoto unmap;\n+\thdr = mmap = istate->mmap;\n+\tmmap_size = istate->mmap_size;\n+\tif (!istate->keep_mmap)\n+\t\tistate->mmap = NULL;\n \n \thashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - 20);\n \tistate->version = ntohl(hdr->hdr_version);\n@@ -2054,9 +2117,13 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n \t\treturn ret;\n \tassert((flags & (COMMIT_LOCK | CLOSE_LOCK)) !=\n \t       (COMMIT_LOCK | CLOSE_LOCK));\n-\tif (flags & COMMIT_LOCK)\n-\t\treturn commit_locked_index(lock);\n-\telse if (flags & CLOSE_LOCK)\n+\tif (flags & COMMIT_LOCK) {\n+\t\tstruct stat st;\n+\t\tret = commit_locked_index(lock);\n+\t\tif (!ret && !stat(git_path(\"index-helper.pid\"), &st))\n+\t\t\tpoke_daemon(&st, 1);\n+\t\treturn ret;\n+\t} else if (flags & CLOSE_LOCK)\n \t\treturn close_lock_file(lock);\n \telse\n \t\treturn ret;\ndiff --git a/shm.c b/shm.c\nnew file mode 100644\nindex 0000000..4ec1a00\n--- /dev/null\n+++ b/shm.c\n@@ -0,0 +1,67 @@\n+#include \"git-compat-util.h\"\n+#include \"shm.h\"\n+\n+#ifdef HAVE_SHM\n+\n+#define SHM_PATH_LEN 72\t\t/* we don't create very long paths.. */\n+\n+ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n+\t\t    int prot, int flags, const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tchar path[SHM_PATH_LEN];\n+\tint fd;\n+\n+\tpath[0] = '/';\n+\tva_start(ap, fmt);\n+\tvsprintf(path + 1, fmt, ap);\n+\tva_end(ap);\n+\tfd = shm_open(path, oflag, perm);\n+\tif (fd < 0)\n+\t\treturn -1;\n+\tif (length > 0 && ftruncate(fd, length)) {\n+\t\tshm_unlink(path);\n+\t\tclose(fd);\n+\t\treturn -1;\n+\t}\n+\tif (length < 0 && !(oflag & O_CREAT)) {\n+\t\tstruct stat st;\n+\t\tif (fstat(fd, &st))\n+\t\t\tdie_errno(\"unable to stat %s\", path);\n+\t\tlength = st.st_size;\n+\t}\n+\t*mmap = xmmap(NULL, length, prot, flags, fd, 0);\n+\tclose(fd);\n+\tif (*mmap == MAP_FAILED) {\n+\t\t*mmap = NULL;\n+\t\tshm_unlink(path);\n+\t\treturn -1;\n+\t}\n+\treturn length;\n+}\n+\n+void git_shm_unlink(const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tchar path[SHM_PATH_LEN];\n+\n+\tpath[0] = '/';\n+\tva_start(ap, fmt);\n+\tvsprintf(path + 1, fmt, ap);\n+\tva_end(ap);\n+\tshm_unlink(path);\n+}\n+\n+#else\n+\n+ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n+\t\t    int prot, int flags, const char *fmt, ...)\n+{\n+\treturn -1;\n+}\n+\n+void git_shm_unlink(const char *fmt, ...)\n+{\n+}\n+\n+#endif\ndiff --git a/shm.h b/shm.h\nnew file mode 100644\nindex 0000000..798d3fd\n--- /dev/null\n+++ b/shm.h\n@@ -0,0 +1,23 @@\n+#ifndef SHM_H\n+#define SHM_H\n+\n+/*\n+ * Create or open a shared memory and mmap it. Return mmap size if\n+ * successful, -1 otherwise. If successful mmap contains the mmap'd\n+ * pointer. If oflag does not contain O_CREAT and length is negative,\n+ * the mmap size is retrieved from existing shared memory object.\n+ *\n+ * The mmap could be freed by munmap, even on Windows. Note that on\n+ * Windows, git_shm_unlink() is no-op, so the last unmap will destroy\n+ * the shared memory.\n+ */\n+ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n+\t\t    int prot, int flags, const char *fmt, ...);\n+\n+/*\n+ * Unlink a shared memory object. Only needed on POSIX platforms. On\n+ * Windows this is no-op.\n+ */\n+void git_shm_unlink(const char *fmt, ...);\n+\n+#endif\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246798","messageId":"1406548995-28549-6-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 5/9] trace.c: add GIT_TRACE_INDEX_STATS for index statistics","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:11Z","receivedAt":"2014-07-28T12:03:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git.txt |  1 +\n cache.h               |  1 +\n read-cache.c          | 16 ++++++++++++++++\n trace.c               |  5 ++++-\n 4 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex a124612..7077386 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -947,6 +947,7 @@ Unsetting the variable, or setting it to empty, \"0\" or\n \tSee 'GIT_TRACE' for available trace output options.\n \n 'GIT_TRACE_PACK_STATS'::\n+'GIT_TRACE_INDEX_STATS'::\n \tPrint various statistics.\n \n 'GIT_TRACE_SETUP'::\ndiff --git a/cache.h b/cache.h\nindex 29a0c53..4266771 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1508,5 +1508,6 @@ void stat_validity_update(struct stat_validity *sv, int fd);\n int versioncmp(const char *s1, const char *s2);\n \n void report_pack_stats(struct trace_key *key);\n+void report_index_stats(struct trace_key *key);\n \n #endif /* CACHE_H */\ndiff --git a/read-cache.c b/read-cache.c\nindex ff28142..4bd2abe 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -48,6 +48,10 @@ struct index_state the_index;\n static const char *alternate_index_output;\n int dont_poke_index_helper;\n \n+static unsigned int nr_read_index;\n+static unsigned int nr_read_shm_index;\n+static unsigned int nr_write_index;\n+\n static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)\n {\n \tistate->cache[nr] = ce;\n@@ -1523,6 +1527,7 @@ static int try_shm(struct index_state *istate)\n \tistate->mmap = new_mmap;\n \tistate->mmap_size = new_length;\n \tpoke_daemon(&st, 0);\n+\tnr_read_shm_index++;\n \treturn 0;\n }\n \n@@ -1619,6 +1624,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t}\n \tif (!istate->keep_mmap)\n \t\tmunmap(mmap, mmap_size);\n+\tnr_read_index++;\n \treturn istate->cache_nr;\n \n unmap:\n@@ -2087,6 +2093,7 @@ static int do_write_index(struct index_state *istate, int newfd,\n \t\treturn -1;\n \tistate->timestamp.sec = (unsigned int)st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n+\tnr_write_index++;\n \treturn 0;\n }\n \n@@ -2346,3 +2353,12 @@ void stat_validity_update(struct stat_validity *sv, int fd)\n \t\tfill_stat_data(sv->sd, &st);\n \t}\n }\n+\n+void report_index_stats(struct trace_key *key)\n+{\n+\ttrace_printf_key(key, \"\\n\"\n+\t\t\t \"index stats: file reads        = %10u\\n\"\n+\t\t\t \"index stats: cache reads       = %10u\\n\"\n+\t\t\t \"index stats: file writes       = %10u\\n\",\n+\t\t\t nr_read_index, nr_read_shm_index, nr_write_index);\n+}\ndiff --git a/trace.c b/trace.c\nindex c52da0f..eb3a11a 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -428,14 +428,17 @@ void trace_command_performance(const char **argv)\n }\n \n static struct trace_key trace_pack_stats = TRACE_KEY_INIT(PACK_STATS);\n+static struct trace_key trace_index_stats = TRACE_KEY_INIT(INDEX_STATS);\n \n static void print_stats_atexit(void)\n {\n \treport_pack_stats(&trace_pack_stats);\n+\treport_index_stats(&trace_index_stats);\n }\n \n void trace_stats(void)\n {\n-\tif (trace_want(&trace_pack_stats))\n+\tif (trace_want(&trace_pack_stats) ||\n+\t    trace_want(&trace_index_stats))\n \t\tatexit(print_stats_atexit);\n }\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246799","messageId":"1406548995-28549-7-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 6/9] index-helper: add --strict","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:12Z","receivedAt":"2014-07-28T12:03:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"There's are \"holes\" in the index-helper approach because the shared\nmemory is not verified again by git. If $USER is compromised, shared\nmemory could be modified. But then they can already modify\n$GIT_DIR/index. A more realistic risk is some bugs in index-helper\nproduce corrupt shared memory. --strict is added to avoid that\n\nStrictly speaking there's still a very small gap where corrupt shared\nmemory could still be read by git: after we write the trailing SHA-1 in\nthe shared memory (thus signaling \"this shm is ready\") and before\nverify_shm() detects an error.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-index-helper.txt |  9 ++++++++\n cache.h                            |  1 +\n index-helper.c                     | 47 ++++++++++++++++++++++++++++++++++++++\n read-cache.c                       |  9 +++++---\n 4 files changed, 63 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex da41c4d..406856d 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -22,6 +22,15 @@ OPTIONS\n \tExit if the cached index is not accessed for `<n>`\n \tminutes. Specify 0 to wait forever. Default is 10.\n \n+--strict::\n+--no-strict::\n+\tStrict mode makes index-helper verify the shared memory after\n+\tit's created. If the result does not match what's read from\n+\t$GIT_DIR/index, the shared memory is destroyed. This makes\n+\tindex-helper take more than double the amount of time required\n+\tfor reading an index, but because it will happen in the\n+\tbackground, it's not noticable. `--strict` is enabled by default.\n+\n NOTES\n -----\n On UNIX-like systems, $GIT_DIR/index-helper.pid contains the process\ndiff --git a/cache.h b/cache.h\nindex 4266771..0f515e5 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -307,6 +307,7 @@ struct index_state {\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n \t\t keep_mmap : 1,\n+\t\t always_verify_trailing_sha1 : 1,\n \t\t initialized : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\ndiff --git a/index-helper.c b/index-helper.c\nindex 8fa0af9..c82d307 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -13,6 +13,7 @@ struct index_shm {\n \n static struct index_shm shm_index;\n static struct index_shm shm_base_index;\n+static int to_verify = 1;\n \n static void free_index_shm(struct index_shm *is)\n {\n@@ -68,6 +69,48 @@ static void share_index(struct index_state *istate, struct index_shm *is)\n \thashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);\n }\n \n+static int verify_shm(void)\n+{\n+\tint i;\n+\tstruct index_state istate;\n+\tmemset(&istate, 0, sizeof(istate));\n+\tistate.always_verify_trailing_sha1 = 1;\n+\ti = read_index(&istate);\n+\tif (i != the_index.cache_nr)\n+\t\tgoto done;\n+\tfor (; i < the_index.cache_nr; i++) {\n+\t\tstruct cache_entry *base, *ce;\n+\t\t/* namelen is checked separately */\n+\t\tconst unsigned int ondisk_flags =\n+\t\t\tCE_STAGEMASK | CE_VALID | CE_EXTENDED_FLAGS;\n+\t\tunsigned int ce_flags, base_flags, ret;\n+\t\tbase = the_index.cache[i];\n+\t\tce = istate.cache[i];\n+\t\tif (ce->ce_namelen != base->ce_namelen ||\n+\t\t    strcmp(ce->name, base->name)) {\n+\t\t\twarning(\"mismatch at entry %d\", i);\n+\t\t\tbreak;\n+\t\t}\n+\t\tce_flags = ce->ce_flags;\n+\t\tbase_flags = base->ce_flags;\n+\t\t/* only on-disk flags matter */\n+\t\tce->ce_flags   &= ondisk_flags;\n+\t\tbase->ce_flags &= ondisk_flags;\n+\t\tret = memcmp(&ce->ce_stat_data, &base->ce_stat_data,\n+\t\t\t     offsetof(struct cache_entry, name) -\n+\t\t\t     offsetof(struct cache_entry, ce_stat_data));\n+\t\tce->ce_flags = ce_flags;\n+\t\tbase->ce_flags = base_flags;\n+\t\tif (ret) {\n+\t\t\twarning(\"mismatch at entry %d\", i);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+done:\n+\tdiscard_index(&istate);\n+\treturn i == the_index.cache_nr;\n+}\n+\n static void refresh(int sig)\n {\n \tthe_index.keep_mmap = 1;\n@@ -76,6 +119,8 @@ static void refresh(int sig)\n \tif (the_index.split_index && the_index.split_index->base)\n \t\tshare_index(the_index.split_index->base, &shm_base_index);\n \tshare_index(&the_index, &shm_index);\n+\tif (to_verify && !verify_shm())\n+\t\tcleanup_shm();\n \tdiscard_index(&the_index);\n }\n \n@@ -123,6 +168,8 @@ int main(int argc, char **argv)\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"exit-after\", &idle_in_minutes,\n \t\t\t    N_(\"exit if not used after some minutes\")),\n+\t\tOPT_BOOL(0, \"strict\", &to_verify,\n+\t\t\t \"verify shared memory after creating\"),\n \t\tOPT_END()\n \t};\n \ndiff --git a/read-cache.c b/read-cache.c\nindex 4bd2abe..d6eb17f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1568,9 +1568,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tistate->mmap = mmap;\n \tistate->mmap_size = mmap_size;\n-\tif (try_shm(istate) &&\n-\t    verify_hdr(istate->mmap, istate->mmap_size) < 0)\n-\t\tgoto unmap;\n+\tif (try_shm(istate)) {\n+\t\tif (verify_hdr(istate->mmap, istate->mmap_size) < 0)\n+\t\t\tgoto unmap;\n+\t} else if (istate->always_verify_trailing_sha1 &&\n+\t\t   verify_hdr(istate->mmap, istate->mmap_size) < 0)\n+\t\t\tgoto unmap;\n \thdr = mmap = istate->mmap;\n \tmmap_size = istate->mmap_size;\n \tif (!istate->keep_mmap)\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246800","messageId":"1406548995-28549-8-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 7/9] daemonize(): set a flag before exiting the main process","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:13Z","receivedAt":"2014-07-28T12:03:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This allows signal handlers and atexit functions to realize this\nsituation and not clean up.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/gc.c | 2 +-\n cache.h      | 2 +-\n daemon.c     | 2 +-\n setup.c      | 4 +++-\n 4 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 8d219d8..f120b4f 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -341,7 +341,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\t * failure to daemonize is ok, we'll continue\n \t\t\t * in foreground\n \t\t\t */\n-\t\t\tdaemonize();\n+\t\t\tdaemonize(NULL);\n \t\t}\n \t} else\n \t\tadd_repack_all_option();\ndiff --git a/cache.h b/cache.h\nindex 0f515e5..f568c8a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -467,7 +467,7 @@ extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);\n extern int init_db(const char *template_dir, unsigned int flags);\n \n extern void sanitize_stdfds(void);\n-extern int daemonize(void);\n+extern int daemonize(int *);\n \n #define alloc_nr(x) (((x)+16)*3/2)\n \ndiff --git a/daemon.c b/daemon.c\nindex e6b51ed..cfdf0f6 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1306,7 +1306,7 @@ int main(int argc, char **argv)\n \t\treturn execute();\n \n \tif (detach) {\n-\t\tif (daemonize())\n+\t\tif (daemonize(NULL))\n \t\t\tdie(\"--detach not supported on this platform\");\n \t} else\n \t\tsanitize_stdfds();\ndiff --git a/setup.c b/setup.c\nindex 0a22f8b..b509395 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -842,7 +842,7 @@ void sanitize_stdfds(void)\n \t\tclose(fd);\n }\n \n-int daemonize(void)\n+int daemonize(int *daemonized)\n {\n #ifdef NO_POSIX_GOODIES\n \terrno = ENOSYS;\n@@ -854,6 +854,8 @@ int daemonize(void)\n \t\tcase -1:\n \t\t\tdie_errno(\"fork failed\");\n \t\tdefault:\n+\t\t\tif (daemonized)\n+\t\t\t\t*daemonized = 1;\n \t\t\texit(0);\n \t}\n \tif (setsid() == -1)\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246801","messageId":"1406548995-28549-9-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 8/9] index-helper: add --detach","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:14Z","receivedAt":"2014-07-28T12:03:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-index-helper.txt |  3 +++\n index-helper.c                     | 10 ++++++++--\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex 406856d..d8fbdab 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -31,6 +31,9 @@ OPTIONS\n \tfor reading an index, but because it will happen in the\n \tbackground, it's not noticable. `--strict` is enabled by default.\n \n+--detach::\n+\tDetach from the shell.\n+\n NOTES\n -----\n On UNIX-like systems, $GIT_DIR/index-helper.pid contains the process\ndiff --git a/index-helper.c b/index-helper.c\nindex c82d307..4262678 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -13,7 +13,7 @@ struct index_shm {\n \n static struct index_shm shm_index;\n static struct index_shm shm_base_index;\n-static int to_verify = 1;\n+static int daemonized, to_verify = 1;\n \n static void free_index_shm(struct index_shm *is)\n {\n@@ -32,6 +32,8 @@ static void cleanup_shm(void)\n \n static void cleanup(void)\n {\n+\tif (daemonized)\n+\t\treturn;\n \tunlink(git_path(\"index-helper.pid\"));\n \tcleanup_shm();\n }\n@@ -164,12 +166,13 @@ int main(int argc, char **argv)\n \tstatic struct lock_file lock;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *prefix;\n-\tint fd, idle_in_minutes = 10;\n+\tint fd, idle_in_minutes = 10, detach = 0;\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"exit-after\", &idle_in_minutes,\n \t\t\t    N_(\"exit if not used after some minutes\")),\n \t\tOPT_BOOL(0, \"strict\", &to_verify,\n \t\t\t \"verify shared memory after creating\"),\n+\t\tOPT_BOOL(0, \"detach\", &detach, \"detach the process\"),\n \t\tOPT_END()\n \t};\n \n@@ -196,6 +199,9 @@ int main(int argc, char **argv)\n \tatexit(cleanup);\n \tsigchain_push_common(cleanup_on_signal);\n \n+\tif (detach && daemonize(&daemonized))\n+\t\tdie_errno(\"unable to detach\");\n+\n \tif (!idle_in_minutes)\n \t\tidle_in_minutes = 0xffffffff / 60;\n \tloop(sb.buf, idle_in_minutes * 60);\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246802","messageId":"1406548995-28549-10-git-send-email-pclouds@gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 9/9] index-helper: add Windows support","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-28T12:03:15Z","receivedAt":"2014-07-28T12:03:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Windows supports shared memory, but the semantics is a bit different\nthan POSIX shm. The most noticeable thing is there's no way to get the\nshared memory's size by the reader, and wrapping fstat to do that\nwould be hell. So the shm size is added near the end, hidden away from\nshm users (storing it in headers would cause more problems with munmap,\nstoring it as a separate shm is even worse).\n\nPostMessage is used instead of UNIX signals for notification.\nLightweight (at least code-wise) on the client side.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n config.mak.uname |  2 ++\n index-helper.c   | 48 ++++++++++++++++++++++++++++\n read-cache.c     |  6 ++++\n shm.c            | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 152 insertions(+)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 7e66d80..74e764f 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -377,6 +377,7 @@ ifndef DEBUG\n else\n \tBASIC_CFLAGS += -Zi -MDd\n endif\n+\tPROGRAM_OBJS += index-helper.o\n \tX = .exe\n endif\n ifeq ($(uname_S),Interix)\n@@ -532,6 +533,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))\n else\n \tNO_CURL = YesPlease\n endif\n+\tPROGRAM_OBJS += index-helper.o\n endif\n ifeq ($(uname_S),QNX)\n \tCOMPAT_CFLAGS += -DSA_RESTART=0\ndiff --git a/index-helper.c b/index-helper.c\nindex 4262678..7c2a0e7 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -147,6 +147,51 @@ static void loop(const char *pid_file, int idle_in_seconds)\n \t\t; /* do nothing, all is handled by signal handlers already */\n }\n \n+#elif defined(GIT_WINDOWS_NATIVE)\n+\n+static void loop(const char *pid_file, int idle_in_seconds)\n+{\n+\tHWND hwnd;\n+\tUINT_PTR timer = 0;\n+\tMSG msg;\n+\tHINSTANCE hinst = GetModuleHandle(NULL);\n+\tWNDCLASS wc;\n+\n+\t/*\n+\t * Emulate UNIX signals by sending WM_USER+x to a\n+\t * window. Register window class and create a new window to\n+\t * catch these messages.\n+\t */\n+\tmemset(&wc, 0, sizeof(wc));\n+\twc.lpfnWndProc\t = DefWindowProc;\n+\twc.hInstance\t = hinst;\n+\twc.lpszClassName = \"git-index-helper\";\n+\tif (!RegisterClass(&wc))\n+\t\tdie_errno(_(\"could not register new window class\"));\n+\n+\thwnd = CreateWindow(\"git-index-helper\", pid_file,\n+\t\t\t    0, 0, 0, 1, 1, NULL, NULL, hinst, NULL);\n+\tif (!hwnd)\n+\t\tdie_errno(_(\"could not register new window\"));\n+\n+\trefresh(0);\n+\twhile (1) {\n+\t\ttimer = SetTimer(hwnd, timer, idle_in_seconds * 1000, NULL);\n+\t\tif (!timer)\n+\t\t\tdie(_(\"no timer!\"));\n+\t\tif (!GetMessage(&msg, hwnd, 0, 0) || msg.message == WM_TIMER)\n+\t\t\tbreak;\n+\t\tswitch (msg.message) {\n+\t\tcase WM_USER:\n+\t\t\trefresh(0);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\t/* just reset the timer */\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n #else\n \n static void loop(const char *pid_file, int idle_in_seconds)\n@@ -192,6 +237,9 @@ int main(int argc, char **argv)\n \t\t\t\t       git_path(\"index-helper.pid\"),\n \t\t\t\t       LOCK_DIE_ON_ERROR);\n \tstrbuf_addf(&sb,\n+#ifdef GIT_WINDOWS_NATIVE\n+\t\t    \"HWND\"\n+#endif\n \t\t    \"%\" PRIuMAX, (uintmax_t) getpid());\n \twrite_in_full(fd, sb.buf, sb.len);\n \tcommit_lock_file(&lock);\ndiff --git a/read-cache.c b/read-cache.c\nindex d6eb17f..0ca3345 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1489,6 +1489,12 @@ static void poke_daemon(const struct stat *st, int refresh_cache)\n \t\tpid_t pid = strtoul(sb.buf, &end, 10);\n \t\tif (end && sb.buf + st->st_size == end)\n \t\t\tkill(pid, refresh_cache ? SIGHUP : SIGUSR1);\n+#elif defined(GIT_WINDOWS_NATIVE)\n+\t\tif (starts_with(sb.buf, \"HWND\")) {\n+\t\t\tHWND hwnd = FindWindow(\"git-index-helper\", sb.buf);\n+\t\t\tPostMessage(hwnd, refresh_cache ? WM_USER : WM_USER + 1,\n+\t\t\t\t    0, 0);\n+\t\t}\n #endif\n \t}\n \tclose(fd);\ndiff --git a/shm.c b/shm.c\nindex 4ec1a00..04d8a35 100644\n--- a/shm.c\n+++ b/shm.c\n@@ -52,6 +52,102 @@ void git_shm_unlink(const char *fmt, ...)\n \tshm_unlink(path);\n }\n \n+#elif defined(GIT_WINDOWS_NATIVE)\n+\n+#define SHM_PATH_LEN 82\t/* a little bit longer than POSIX because of \"Local\\\\\" */\n+\n+static ssize_t create_shm_map(int oflag, int perm, ssize_t length,\n+\t\t\t      void **mmap, int prot, int flags,\n+\t\t\t      const char *path, unsigned long page_size)\n+{\n+\tsize_t real_length;\n+\tvoid *last_page;\n+\tHANDLE h;\n+\n+\tassert(perm   == 0700);\n+\tassert(oflag  == (O_CREAT | O_EXCL | O_RDWR));\n+\tassert(prot   == (PROT_READ | PROT_WRITE));\n+\tassert(flags  == MAP_SHARED);\n+\tassert(length >= 0);\n+\n+\treal_length = length;\n+\tif (real_length % page_size)\n+\t\treal_length += page_size - (real_length % page_size);\n+\treal_length += page_size;\n+\th = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0,\n+\t\t\t      real_length, path);\n+\tif (!h)\n+\t\treturn -1;\n+\t*mmap = MapViewOfFile(h, FILE_MAP_ALL_ACCESS, 0, 0, real_length);\n+\tCloseHandle(h);\n+\tif (!*mmap)\n+\t\treturn -1;\n+\tlast_page = (unsigned char *)*mmap + real_length - page_size;\n+\t*(unsigned long *)last_page = length;\n+\treturn length;\n+}\n+\n+static ssize_t open_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n+\t\t\t    int prot, int flags, const char *path,\n+\t\t\t    unsigned long page_size)\n+{\n+\tvoid *last_page;\n+\tHANDLE h;\n+\n+\tassert(perm   == 0700);\n+\tassert(oflag  == O_RDONLY);\n+\tassert(prot   == PROT_READ);\n+\tassert(flags  == MAP_SHARED);\n+\tassert(length <= 0);\n+\n+\th = OpenFileMapping(FILE_MAP_READ, FALSE, path);\n+\tif (!h)\n+\t\treturn -1;\n+\t*mmap = MapViewOfFile(h, FILE_MAP_READ, 0, 0, 0);\n+\tCloseHandle(h);\n+\tif (!*mmap)\n+\t\treturn -1;\n+\tif (length < 0) {\n+\t\tMEMORY_BASIC_INFORMATION mbi;\n+\t\tif (!VirtualQuery(*mmap, &mbi, sizeof(mbi))) {\n+\t\t\tUnmapViewOfFile(*mmap);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tif (mbi.RegionSize % page_size)\n+\t\t\tdie(\"expected size %lu to be %lu aligned\",\n+\t\t\t\t    mbi.RegionSize, page_size);\n+\t\tlast_page = (unsigned char *)*mmap + mbi.RegionSize - page_size;\n+\t\tlength = *(unsigned long *)last_page;\n+\t}\n+\treturn length;\n+}\n+\n+ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n+\t\t    int prot, int flags, const char *fmt, ...)\n+{\n+\tSYSTEM_INFO si;\n+\tva_list ap;\n+\tchar path[SHM_PATH_LEN];\n+\n+\tGetSystemInfo(&si);\n+\n+\tstrcpy(path, \"Local\\\\\");\n+\tva_start(ap, fmt);\n+\tvsprintf(path + strlen(path), fmt, ap);\n+\tva_end(ap);\n+\n+\tif (oflag & O_CREAT)\n+\t\treturn create_shm_map(oflag, perm, length, mmap, prot,\n+\t\t\t\t      flags, path, si.dwPageSize);\n+\telse\n+\t\treturn open_shm_map(oflag, perm, length, mmap, prot,\n+\t\t\t\t    flags, path, si.dwPageSize);\n+}\n+\n+void git_shm_unlink(const char *fmt, ...)\n+{\n+}\n+\n #else\n \n ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n-- \n2.1.0.rc0.66.gb9187ad\n"},{"id":"246956","messageId":"CAPig+cTDxTpo53WhfkQbt8EuY15Ttj1NHbh2Do-TKYq2xVvP+g@mail.gmail.com","threadId":"37231","inReplyTo":"1406548995-28549-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 4/9] index-helper: new daemon for caching index and related stuff","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-07-30T08:08:33Z","receivedAt":"2014-07-30T08:08:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Monday, July 28, 2014, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> Instead of reading the index from disk and worrying about disk\n> corruption, the index is cached in memory (memory bit-flips happen\n> too, but hopefully less often). The result is faster read.\n>\n> The biggest gain is not having to verify the trailing SHA-1, which\n> takes lots of time especially on large index files. But this also\n> opens doors for further optimiztions:\n\ns/optimiztions/optimizations/\n\n>  - we could create an in-memory format that's essentially the memory\n>    dump of the index to eliminate most of parsing/allocation\n>    overhead. The mmap'd memory can be used straight away.\n>\n>  - we could cache non-index info such as name hash\n>\n> The shared memory's name folows the template \"git-<object>-<SHA1>\"\n\ns/folows/follows/\n\n> where <SHA1> is the trailing SHA-1 of the index file. <object> is\n> \"index\" for cached index files (and may be \"name-hash\" for name-hash\n> cache). If such shared memory exists, it contains the same index\n> content as on disk. The content is already validated by the daemon and\n> git won't validate it again (except comparing the trailing SHA-1s).\n>\n> Git can poke the daemon to tell it to refresh the index cache, or to\n> keep it alive some more minutes via UNIX signals. It can't give any\n> real index data directly to the daemon. Real data goes to disk first,\n> then the daemon reads and verifies it from there.\n>\n> $GIT_DIR/index-helper.pid contains a reference to daemon process (and\n> it's pid on *nix). The file's mtime is updated every time it's accessed\n\ns/it's pid/its pid/\n\n> (or should be updated often enough). Old index-helper.pid is considered\n> stale and ignored.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n> diff --git a/index-helper.c b/index-helper.c\n> new file mode 100644\n> index 0000000..8fa0af9\n> --- /dev/null\n> +++ b/index-helper.c\n> @@ -0,0 +1,157 @@\n> +static void share_index(struct index_state *istate, struct index_shm *is)\n> +{\n> +       void *new_mmap;\n> +       if (istate->mmap_size <= 20 ||\n> +           hashcmp(istate->sha1,\n> +                   (unsigned char *)istate->mmap + istate->mmap_size - 20) ||\n> +           !hashcmp(istate->sha1, is->sha1) ||\n> +           git_shm_map(O_CREAT | O_EXCL | O_RDWR, 0700, istate->mmap_size,\n> +                       &new_mmap, PROT_READ | PROT_WRITE, MAP_SHARED,\n> +                       \"git-index-%s\", sha1_to_hex(istate->sha1)) < 0)\n\nDo any of these failure conditions deserve a diagnostic message to let\nthe user know that there was a problem?\n\n> +               return;\n> +\n> +       free_index_shm(is);\n> +       is->size = istate->mmap_size;\n> +       is->shm = new_mmap;\n> +       hashcpy(is->sha1, istate->sha1);\n> +       memcpy(new_mmap, istate->mmap, istate->mmap_size - 20);\n> +\n> +       /*\n> +        * The trailing hash must be written last after everything is\n> +        * written. It's the indication that the shared memory is now\n> +        * ready.\n> +        */\n> +       hashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);\n> +}\n> diff --git a/read-cache.c b/read-cache.c\n> index b679781..ff28142 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -1465,6 +1467,65 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk,\n>         return ce;\n>  }\n>\n> +/*\n> + * Try to open and verify a cached shm index if available. Return 0 if\n> + * succeeds (istate->mmap and istate->mmap_size are updated). Return\n> + * negative otherwise.\n> + */\n> +static int try_shm(struct index_state *istate)\n> +{\n> +       void *new_mmap = NULL;\n> +       size_t old_size = istate->mmap_size;\n\nIs old_size needed? Can you not simply reference istate->mmap_size\ndirectly each place old_size is mentioned?\n\n> +       ssize_t new_length;\n\nNit: 'size' vs. 'length' inconsistency in variable name choices.\n\n> +       const unsigned char *sha1;\n> +       struct stat st;\n> +\n> +       if (old_size <= 20 ||\n> +           stat(git_path(\"index-helper.pid\"), &st))\n> +               return -1;\n> +       sha1 = (unsigned char *)istate->mmap + old_size - 20;\n> +       new_length = git_shm_map(O_RDONLY, 0700, -1, &new_mmap,\n> +                                PROT_READ, MAP_SHARED,\n> +                                \"git-index-%s\", sha1_to_hex(sha1));\n> +       if (new_length <= 20 ||\n> +           hashcmp((unsigned char *)istate->mmap + old_size - 20,\n> +                   (unsigned char *)new_mmap + new_length - 20)) {\n> +               if (new_mmap)\n> +                       munmap(new_mmap, new_length);\n> +               poke_daemon(&st, 1);\n> +               return -1;\n> +       }\n> +       munmap(istate->mmap, istate->mmap_size);\n> +       istate->mmap = new_mmap;\n> +       istate->mmap_size = new_length;\n> +       poke_daemon(&st, 0);\n> +       return 0;\n> +}\n> +\n>  /* remember to discard_cache() before reading a different cache! */\n>  int do_read_index(struct index_state *istate, const char *path, int must_exist)\n>  {\n> diff --git a/shm.c b/shm.c\n> new file mode 100644\n> index 0000000..4ec1a00\n> --- /dev/null\n> +++ b/shm.c\n> @@ -0,0 +1,67 @@\n> +#include \"git-compat-util.h\"\n> +#include \"shm.h\"\n> +\n> +#ifdef HAVE_SHM\n> +\n> +#define SHM_PATH_LEN 72                /* we don't create very long paths.. */\n> +\n> +ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n> +                   int prot, int flags, const char *fmt, ...)\n> +{\n> +       va_list ap;\n> +       char path[SHM_PATH_LEN];\n\nIs there a reason to avoid strbuf here?\n\n> +       int fd;\n> +\n> +       path[0] = '/';\n> +       va_start(ap, fmt);\n> +       vsprintf(path + 1, fmt, ap);\n> +       va_end(ap);\n> +       fd = shm_open(path, oflag, perm);\n> +       if (fd < 0)\n> +               return -1;\n> +       if (length > 0 && ftruncate(fd, length)) {\n> +               shm_unlink(path);\n> +               close(fd);\n> +               return -1;\n> +       }\n> +       if (length < 0 && !(oflag & O_CREAT)) {\n> +               struct stat st;\n> +               if (fstat(fd, &st))\n> +                       die_errno(\"unable to stat %s\", path);\n> +               length = st.st_size;\n> +       }\n> +       *mmap = xmmap(NULL, length, prot, flags, fd, 0);\n> +       close(fd);\n> +       if (*mmap == MAP_FAILED) {\n> +               *mmap = NULL;\n> +               shm_unlink(path);\n> +               return -1;\n> +       }\n> +       return length;\n> +}\n"},{"id":"246961","messageId":"CACsJy8Bngi8-eMSvNAPmT4_o4A_f1dq-Hs+dC7FNiVHNmoL7+g@mail.gmail.com","threadId":"37231","inReplyTo":"CAPig+cTDxTpo53WhfkQbt8EuY15Ttj1NHbh2Do-TKYq2xVvP+g@mail.gmail.com","subject":"Re: [PATCH v3 4/9] index-helper: new daemon for caching index and related stuff","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-07-30T10:39:08Z","receivedAt":"2014-07-30T10:39:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 30, 2014 at 3:08 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n>> +static void share_index(struct index_state *istate, struct index_shm *is)\n>> +{\n>> +       void *new_mmap;\n>> +       if (istate->mmap_size <= 20 ||\n>> +           hashcmp(istate->sha1,\n>> +                   (unsigned char *)istate->mmap + istate->mmap_size - 20) ||\n>> +           !hashcmp(istate->sha1, is->sha1) ||\n>> +           git_shm_map(O_CREAT | O_EXCL | O_RDWR, 0700, istate->mmap_size,\n>> +                       &new_mmap, PROT_READ | PROT_WRITE, MAP_SHARED,\n>> +                       \"git-index-%s\", sha1_to_hex(istate->sha1)) < 0)\n>\n> Do any of these failure conditions deserve a diagnostic message to let\n> the user know that there was a problem?\n\nHmm.. diagnostic messages are a problem already. This most likely runs\nas a daemon, nowhere to print to. But if running on foreground, some\nmessages would help.\n\n>> +static int try_shm(struct index_state *istate)\n>> +{\n>> +       void *new_mmap = NULL;\n>> +       size_t old_size = istate->mmap_size;\n>\n> Is old_size needed? Can you not simply reference istate->mmap_size\n> directly each place old_size is mentioned?\n>\n>> +       ssize_t new_length;\n>\n> Nit: 'size' vs. 'length' inconsistency in variable name choices.\n\nLeftovers after many iterations. Will fix.\n\n>> +#define SHM_PATH_LEN 72                /* we don't create very long paths.. */\n>> +\n>> +ssize_t git_shm_map(int oflag, int perm, ssize_t length, void **mmap,\n>> +                   int prot, int flags, const char *fmt, ...)\n>> +{\n>> +       va_list ap;\n>> +       char path[SHM_PATH_LEN];\n>\n> Is there a reason to avoid strbuf here?\n\nLaziness (which is ironic as I'm working on patches to clean up fixed\nsize buffers)\n-- \nDuy\n"},{"id":"247011","messageId":"1406774496.4215.3.camel@leckie","threadId":"37231","inReplyTo":"1406548995-28549-5-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 4/9] index-helper: new daemon for caching index and related stuff","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-07-31T02:41:36Z","receivedAt":"2014-07-31T02:41:36Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"\nOn Mon, 2014-07-28 at 19:03 +0700, Nguyễn Thái Ngọc Duy wrote:\n> +# Define HAVE_SHM if you platform support shm_* functions in librt.\n\ns/you/your/\n\n> +static void free_index_shm(struct index_shm *is)\n\nDoes not actually free its argument; should be release_index_shm.  \n"}]}