{"thread":{"id":"30203","subject":"[PATCH v3 0/3] Multithread index-pack","startedAt":"2012-04-11T05:49:45Z","lastAt":"2012-05-08T00:19:49Z","messageCount":13,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"188925","messageId":"1334123388-6083-1-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":null,"subject":"[PATCH v3 0/3] Multithread index-pack","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-11T05:49:45Z","receivedAt":"2012-04-11T05:49:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This round hopefully fixes MinGW brekages for real. I also added a\nperf test but not so sure if this is a correct way to do.\n\nNguyễn Thái Ngọc Duy (2):\n  index-pack: split second pass obj handling into own function\n  index-pack: support multithreaded delta resolving\n\nRamsay Allan Jones (1):\n  compat/win32/pthread.h: Add an pthread_key_delete() implementation\n\n Documentation/git-index-pack.txt |   10 ++\n Makefile                         |    2 +-\n builtin/index-pack.c             |  229 +++++++++++++++++++++++++++++++++-----\n compat/win32/pthread.h           |    5 +\n t/perf/p5302-pack-index.sh       |   40 +++++++\n 5 files changed, 257 insertions(+), 29 deletions(-)\n create mode 100755 t/perf/p5302-pack-index.sh\n\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"188926","messageId":"1334123388-6083-2-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"1334123388-6083-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 1/3] compat/win32/pthread.h: Add an pthread_key_delete() implementation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-11T05:49:46Z","receivedAt":"2012-04-11T05:49:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n compat/win32/pthread.h |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nindex 2e20548..8ad1873 100644\n--- a/compat/win32/pthread.h\n+++ b/compat/win32/pthread.h\n@@ -86,6 +86,11 @@ static inline int pthread_key_create(pthread_key_t *keyp, void (*destructor)(voi\n \treturn (*keyp = TlsAlloc()) == TLS_OUT_OF_INDEXES ? EAGAIN : 0;\n }\n \n+static inline int pthread_key_delete(pthread_key_t key)\n+{\n+\treturn TlsFree(key) ? 0 : EINVAL;\n+}\n+\n static inline int pthread_setspecific(pthread_key_t key, const void *value)\n {\n \treturn TlsSetValue(key, (void *)value) ? 0 : EINVAL;\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"188927","messageId":"1334123388-6083-3-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"1334123388-6083-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 2/3] index-pack: split second pass obj handling into own function","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-11T05:49:47Z","receivedAt":"2012-04-11T05:49:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/index-pack.c |   31 ++++++++++++++++++-------------\n 1 files changed, 18 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex dd1c5c9..918684f 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -682,6 +682,23 @@ static int compare_delta_entry(const void *a, const void *b)\n \t\t\t\t   objects[delta_b->obj_no].type);\n }\n \n+/*\n+ * Second pass:\n+ * - for all non-delta objects, look if it is used as a base for\n+ *   deltas;\n+ * - if used as a base, uncompress the object and apply all deltas,\n+ *   recursively checking if the resulting object is used as a base\n+ *   for some more deltas.\n+ */\n+static void second_pass(struct object_entry *obj)\n+{\n+\tstruct base_data *base_obj = alloc_base_data();\n+\tbase_obj->obj = obj;\n+\tbase_obj->data = NULL;\n+\tfind_unresolved_deltas(base_obj);\n+\tdisplay_progress(progress, nr_resolved_deltas);\n+}\n+\n /* Parse all objects and return the pack content SHA1 hash */\n static void parse_pack_objects(unsigned char *sha1)\n {\n@@ -736,26 +753,14 @@ static void parse_pack_objects(unsigned char *sha1)\n \tqsort(deltas, nr_deltas, sizeof(struct delta_entry),\n \t      compare_delta_entry);\n \n-\t/*\n-\t * Second pass:\n-\t * - for all non-delta objects, look if it is used as a base for\n-\t *   deltas;\n-\t * - if used as a base, uncompress the object and apply all deltas,\n-\t *   recursively checking if the resulting object is used as a base\n-\t *   for some more deltas.\n-\t */\n \tif (verbose)\n \t\tprogress = start_progress(\"Resolving deltas\", nr_deltas);\n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *obj = &objects[i];\n-\t\tstruct base_data *base_obj = alloc_base_data();\n \n \t\tif (is_delta_type(obj->type))\n \t\t\tcontinue;\n-\t\tbase_obj->obj = obj;\n-\t\tbase_obj->data = NULL;\n-\t\tfind_unresolved_deltas(base_obj);\n-\t\tdisplay_progress(progress, nr_resolved_deltas);\n+\t\tsecond_pass(obj);\n \t}\n }\n \n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"188928","messageId":"1334123388-6083-4-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"1334123388-6083-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v3 3/3] index-pack: support multithreaded delta resolving","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-04-11T05:49:48Z","receivedAt":"2012-04-11T05:49:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This puts delta resolving on each base on a separate thread, one base\ncache per thread. Per-thread data is grouped in struct thread_local.\nWhen running with nr_threads == 1, no pthreads calls are made. The\nsystem essentially runs in non-thread mode.\n\nAn experiment on a Xeon 24 core machine with git.git shows that\nperformance does not increase proportional to the number of cores. So\nby default, we use maximum 3 cores. Some numbers with --threads from 1\nto 16:\n\n1..4\nreal    0m8.003s  0m5.307s  0m4.321s  0m3.830s\nuser    0m7.720s  0m8.009s  0m8.133s  0m8.305s\nsys     0m0.224s  0m0.372s  0m0.360s  0m0.360s\n\n5..8\nreal    0m3.727s  0m3.604s  0m3.332s  0m3.369s\nuser    0m9.361s  0m9.817s  0m9.525s  0m9.769s\nsys     0m0.584s  0m0.624s  0m0.540s  0m0.560s\n\n9..12\nreal    0m3.036s  0m3.139s  0m3.177s  0m2.961s\nuser    0m8.977s  0m10.205s 0m9.737s  0m10.073s\nsys     0m0.596s  0m0.680s  0m0.684s  0m0.680s\n\n13..16\nreal    0m2.985s  0m2.894s  0m2.975s  0m2.971s\nuser    0m9.825s  0m10.573s 0m10.833s 0m11.361s\nsys     0m0.788s  0m0.732s  0m0.904s  0m1.016s\n\nOn an Intel dual core and linux-2.6.git\n\n1..4\nreal    2m37.789s 2m7.963s  2m0.920s  1m58.213s\nuser    2m28.415s 2m52.325s 2m50.176s 2m41.187s\nsys     0m7.808s  0m11.181s 0m11.224s 0m10.731s\n\nThanks Ramsay Jones for troubleshooting and support on MinGW platform.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-index-pack.txt |   10 ++\n Makefile                         |    2 +-\n builtin/index-pack.c             |  202 ++++++++++++++++++++++++++++++++++---\n t/perf/p5302-pack-index.sh       |   40 ++++++++\n 4 files changed, 236 insertions(+), 18 deletions(-)\n create mode 100755 t/perf/p5302-pack-index.sh\n\ndiff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt\nindex 909687f..39e6d0d 100644\n--- a/Documentation/git-index-pack.txt\n+++ b/Documentation/git-index-pack.txt\n@@ -74,6 +74,16 @@ OPTIONS\n --strict::\n \tDie, if the pack contains broken objects or links.\n \n+--threads=<n>::\n+\tSpecifies the number of threads to spawn when resolving\n+\tdeltas. This requires that index-pack be compiled with\n+\tpthreads otherwise this option is ignored with a warning.\n+\tThis is meant to reduce packing time on multiprocessor\n+\tmachines. The required amount of memory for the delta search\n+\twindow is however multiplied by the number of threads.\n+\tSpecifying 0 will cause git to auto-detect the number of CPU's\n+\tand use maximum 3 threads.\n+\n \n Note\n ----\ndiff --git a/Makefile b/Makefile\nindex be1957a..4d7a3de 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2160,7 +2160,7 @@ builtin/branch.o builtin/checkout.o builtin/clone.o builtin/reset.o branch.o tra\n builtin/bundle.o bundle.o transport.o: bundle.h\n builtin/bisect--helper.o builtin/rev-list.o bisect.o: bisect.h\n builtin/clone.o builtin/fetch-pack.o transport.o: fetch-pack.h\n-builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.h\n+builtin/index-pack.o builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.h\n builtin/send-pack.o transport.o: send-pack.h\n builtin/log.o builtin/shortlog.o: shortlog.h\n builtin/prune.o builtin/reflog.o reachable.o: reachable.h\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 918684f..45bc5db 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -9,6 +9,7 @@\n #include \"progress.h\"\n #include \"fsck.h\"\n #include \"exec_cmd.h\"\n+#include \"thread-utils.h\"\n \n static const char index_pack_usage[] =\n \"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])\";\n@@ -38,6 +39,14 @@ struct base_data {\n \tint ofs_first, ofs_last;\n };\n \n+struct thread_local {\n+#ifndef NO_PTHREADS\n+\tpthread_t thread;\n+#endif\n+\tstruct base_data *base_cache;\n+\tsize_t base_cache_used;\n+};\n+\n /*\n  * Even if sizeof(union delta_base) == 24 on 64-bit archs, we really want\n  * to memcmp() only the first 20 bytes.\n@@ -54,11 +63,13 @@ struct delta_entry {\n \n static struct object_entry *objects;\n static struct delta_entry *deltas;\n-static struct base_data *base_cache;\n-static size_t base_cache_used;\n+static struct thread_local nothread_data;\n static int nr_objects;\n+static int nr_processed;\n static int nr_deltas;\n static int nr_resolved_deltas;\n+static int nr_threads;\n+static int threads_active;\n \n static int from_stdin;\n static int strict;\n@@ -75,6 +86,75 @@ static git_SHA_CTX input_ctx;\n static uint32_t input_crc32;\n static int input_fd, output_fd, pack_fd;\n \n+#ifndef NO_PTHREADS\n+\n+static struct thread_local *thread_data;\n+\n+static pthread_mutex_t read_mutex;\n+#define read_lock()\t\tlock_mutex(&read_mutex)\n+#define read_unlock()\t\tunlock_mutex(&read_mutex)\n+\n+static pthread_mutex_t counter_mutex;\n+#define counter_lock()\t\tlock_mutex(&counter_mutex)\n+#define counter_unlock()\tunlock_mutex(&counter_mutex)\n+\n+static pthread_mutex_t work_mutex;\n+#define work_lock()\t\tlock_mutex(&work_mutex)\n+#define work_unlock()\t\tunlock_mutex(&work_mutex)\n+\n+static pthread_key_t key;\n+\n+static inline void lock_mutex(pthread_mutex_t *mutex)\n+{\n+\tif (threads_active)\n+\t\tpthread_mutex_lock(mutex);\n+}\n+\n+static inline void unlock_mutex(pthread_mutex_t *mutex)\n+{\n+\tif (threads_active)\n+\t\tpthread_mutex_unlock(mutex);\n+}\n+\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_thread(void)\n+{\n+\tinit_recursive_mutex(&read_mutex);\n+\tpthread_mutex_init(&counter_mutex, NULL);\n+\tpthread_mutex_init(&work_mutex, NULL);\n+\tpthread_key_create(&key, NULL);\n+\tthread_data = xcalloc(nr_threads, sizeof(*thread_data));\n+\tthreads_active = 1;\n+}\n+\n+static void cleanup_thread(void)\n+{\n+\tif (!threads_active)\n+\t\treturn;\n+\tthreads_active = 0;\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&counter_mutex);\n+\tpthread_mutex_destroy(&work_mutex);\n+\tpthread_key_delete(key);\n+\tfree(thread_data);\n+}\n+\n+#else\n+\n+#define read_lock()\n+#define read_unlock()\n+\n+#define counter_lock()\n+#define counter_unlock()\n+\n+#define work_lock()\n+#define work_unlock()\n+\n+#endif\n+\n+\n static int mark_link(struct object *obj, int type, void *data)\n {\n \tif (!obj)\n@@ -223,6 +303,17 @@ static NORETURN void bad_object(unsigned long offset, const char *format, ...)\n \tdie(\"pack has bad object at offset %lu: %s\", offset, buf);\n }\n \n+static struct thread_local *get_thread_data()\n+{\n+#ifndef NO_PTHREADS\n+\tif (threads_active)\n+\t\treturn pthread_getspecific(key);\n+#endif\n+\tassert(!threads_active &&\n+\t       \"This should only be reached when all threads are gone\");\n+\treturn &nothread_data;\n+}\n+\n static struct base_data *alloc_base_data(void)\n {\n \tstruct base_data *base = xmalloc(sizeof(struct base_data));\n@@ -237,15 +328,16 @@ static void free_base_data(struct base_data *c)\n \tif (c->data) {\n \t\tfree(c->data);\n \t\tc->data = NULL;\n-\t\tbase_cache_used -= c->size;\n+\t\tget_thread_data()->base_cache_used -= c->size;\n \t}\n }\n \n static void prune_base_data(struct base_data *retain)\n {\n \tstruct base_data *b;\n-\tfor (b = base_cache;\n-\t     base_cache_used > delta_base_cache_limit && b;\n+\tstruct thread_local *data = get_thread_data();\n+\tfor (b = data->base_cache;\n+\t     data->base_cache_used > delta_base_cache_limit && b;\n \t     b = b->child) {\n \t\tif (b->data && b != retain)\n \t\t\tfree_base_data(b);\n@@ -257,12 +349,12 @@ static void link_base_data(struct base_data *base, struct base_data *c)\n \tif (base)\n \t\tbase->child = c;\n \telse\n-\t\tbase_cache = c;\n+\t\tget_thread_data()->base_cache = c;\n \n \tc->base = base;\n \tc->child = NULL;\n \tif (c->data)\n-\t\tbase_cache_used += c->size;\n+\t\tget_thread_data()->base_cache_used += c->size;\n \tprune_base_data(c);\n }\n \n@@ -272,7 +364,7 @@ static void unlink_base_data(struct base_data *c)\n \tif (base)\n \t\tbase->child = NULL;\n \telse\n-\t\tbase_cache = NULL;\n+\t\tget_thread_data()->base_cache = NULL;\n \tfree_base_data(c);\n }\n \n@@ -461,19 +553,24 @@ static void sha1_object(const void *data, unsigned long size,\n \t\t\tenum object_type type, unsigned char *sha1)\n {\n \thash_sha1_file(data, size, typename(type), sha1);\n+\tread_lock();\n \tif (has_sha1_file(sha1)) {\n \t\tvoid *has_data;\n \t\tenum object_type has_type;\n \t\tunsigned long has_size;\n \t\thas_data = read_sha1_file(sha1, &has_type, &has_size);\n+\t\tread_unlock();\n \t\tif (!has_data)\n \t\t\tdie(\"cannot read existing object %s\", sha1_to_hex(sha1));\n \t\tif (size != has_size || type != has_type ||\n \t\t    memcmp(data, has_data, size) != 0)\n \t\t\tdie(\"SHA1 COLLISION FOUND WITH %s !\", sha1_to_hex(sha1));\n \t\tfree(has_data);\n-\t}\n+\t} else\n+\t\tread_unlock();\n+\n \tif (strict) {\n+\t\tread_lock();\n \t\tif (type == OBJ_BLOB) {\n \t\t\tstruct blob *blob = lookup_blob(sha1);\n \t\t\tif (blob)\n@@ -507,6 +604,7 @@ static void sha1_object(const void *data, unsigned long size,\n \t\t\t}\n \t\t\tobj->flags |= FLAG_CHECKED;\n \t\t}\n+\t\tread_unlock();\n \t}\n }\n \n@@ -552,7 +650,7 @@ static void *get_base_data(struct base_data *c)\n \t\tif (!delta_nr) {\n \t\t\tc->data = get_data_from_pack(obj);\n \t\t\tc->size = obj->size;\n-\t\t\tbase_cache_used += c->size;\n+\t\t\tget_thread_data()->base_cache_used += c->size;\n \t\t\tprune_base_data(c);\n \t\t}\n \t\tfor (; delta_nr > 0; delta_nr--) {\n@@ -568,7 +666,7 @@ static void *get_base_data(struct base_data *c)\n \t\t\tfree(raw);\n \t\t\tif (!c->data)\n \t\t\t\tbad_object(obj->idx.offset, \"failed to apply delta\");\n-\t\t\tbase_cache_used += c->size;\n+\t\t\tget_thread_data()->base_cache_used += c->size;\n \t\t\tprune_base_data(c);\n \t\t}\n \t\tfree(delta);\n@@ -596,7 +694,9 @@ static void resolve_delta(struct object_entry *delta_obj,\n \t\tbad_object(delta_obj->idx.offset, \"failed to apply delta\");\n \tsha1_object(result->data, result->size, delta_obj->real_type,\n \t\t    delta_obj->idx.sha1);\n+\tcounter_lock();\n \tnr_resolved_deltas++;\n+\tcounter_unlock();\n }\n \n static struct base_data *find_unresolved_deltas_1(struct base_data *base,\n@@ -696,7 +796,31 @@ static void second_pass(struct object_entry *obj)\n \tbase_obj->obj = obj;\n \tbase_obj->data = NULL;\n \tfind_unresolved_deltas(base_obj);\n-\tdisplay_progress(progress, nr_resolved_deltas);\n+}\n+\n+static void *threaded_second_pass(void *arg)\n+{\n+#ifndef NO_PTHREADS\n+\tif (threads_active)\n+\t\tpthread_setspecific(key, arg);\n+#endif\n+\tfor (;;) {\n+\t\tint i;\n+\t\twork_lock();\n+\t\tdisplay_progress(progress, nr_resolved_deltas);\n+\t\twhile (nr_processed < nr_objects &&\n+\t\t       is_delta_type(objects[nr_processed].type))\n+\t\t\tnr_processed++;\n+\t\tif (nr_processed >= nr_objects) {\n+\t\t\twork_unlock();\n+\t\t\tbreak;\n+\t\t}\n+\t\ti = nr_processed++;\n+\t\twork_unlock();\n+\n+\t\tsecond_pass(&objects[i]);\n+\t}\n+\treturn NULL;\n }\n \n /* Parse all objects and return the pack content SHA1 hash */\n@@ -755,13 +879,25 @@ static void parse_pack_objects(unsigned char *sha1)\n \n \tif (verbose)\n \t\tprogress = start_progress(\"Resolving deltas\", nr_deltas);\n-\tfor (i = 0; i < nr_objects; i++) {\n-\t\tstruct object_entry *obj = &objects[i];\n \n-\t\tif (is_delta_type(obj->type))\n-\t\t\tcontinue;\n-\t\tsecond_pass(obj);\n+\tnr_processed = 0;\n+#ifndef NO_PTHREADS\n+\tif (nr_threads > 1 || getenv(\"GIT_FORCE_THREADS\")) {\n+\t\tinit_thread();\n+\t\tfor (i = 0; i < nr_threads; i++) {\n+\t\t\tint ret = pthread_create(&thread_data[i].thread, NULL,\n+\t\t\t\t\t\t threaded_second_pass, thread_data + i);\n+\t\t\tif (ret)\n+\t\t\t\tdie(\"unable to create thread: %s\", strerror(ret));\n+\t\t}\n+\t\tfor (i = 0; i < nr_threads; i++)\n+\t\t\tpthread_join(thread_data[i].thread, NULL);\n+\n+\t\tcleanup_thread();\n+\t\treturn;\n \t}\n+#endif\n+\tthreaded_second_pass(NULL);\n }\n \n static int write_compressed(struct sha1file *f, void *in, unsigned int size)\n@@ -967,6 +1103,18 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\t\tdie(\"bad pack.indexversion=%\"PRIu32, opts->version);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"pack.threads\")) {\n+\t\tnr_threads = git_config_int(k, v);\n+\t\tif (nr_threads < 0)\n+\t\t\tdie(\"invalid number of threads specified (%d)\",\n+\t\t\t    nr_threads);\n+#ifdef NO_PTHREADS\n+\t\tif (nr_threads != 1)\n+\t\t\twarning(\"no threads support, ignoring %s\", k);\n+\t\tnr_threads = 1;\n+#endif\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(k, v, cb);\n }\n \n@@ -1125,6 +1273,17 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tkeep_msg = \"\";\n \t\t\t} else if (!prefixcmp(arg, \"--keep=\")) {\n \t\t\t\tkeep_msg = arg + 7;\n+\t\t\t} else if (!prefixcmp(arg, \"--threads=\")) {\n+\t\t\t\tchar *end;\n+\t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n+\t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n+\t\t\t\t\tusage(index_pack_usage);\n+#ifdef NO_PTHREADS\n+\t\t\t\tif (nr_threads != 1)\n+\t\t\t\t\twarning(\"no threads support, \"\n+\t\t\t\t\t\t\"ignoring %s\", arg);\n+\t\t\t\tnr_threads = 1;\n+#endif\n \t\t\t} else if (!prefixcmp(arg, \"--pack_header=\")) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n@@ -1196,6 +1355,15 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (strict)\n \t\topts.flags |= WRITE_IDX_STRICT;\n \n+#ifndef NO_PTHREADS\n+\tif (!nr_threads) {\n+\t\tnr_threads = online_cpus();\n+\t\t/* An experiment showed that more threads does not mean faster */\n+\t\tif (nr_threads > 3)\n+\t\t\tnr_threads = 3;\n+\t}\n+#endif\n+\n \tcurr_pack = open_pack_file(pack_name);\n \tparse_pack_header();\n \tobjects = xcalloc(nr_objects + 1, sizeof(struct object_entry));\ndiff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh\nnew file mode 100755\nindex 0000000..6cb5b0d\n--- /dev/null\n+++ b/t/perf/p5302-pack-index.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description=\"Tests index-pack performance\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+\n+test_expect_success 'repack' '\n+\tgit repack -ad &&\n+\tPACK=`ls .git/objects/pack/*.pack | head -n1` &&\n+\ttest -f \"$PACK\" &&\n+\texport PACK\n+'\n+\n+test_perf 'index-pack 0 threads' '\n+\tGIT_DIR=t1 git index-pack --threads=1 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 1 thread ' '\n+\tGIT_DIR=t2 GIT_FORCE_THREADS=1 git index-pack --threads=1 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 2 threads' '\n+\tGIT_DIR=t3 git index-pack --threads=2 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 4 threads' '\n+\tGIT_DIR=t4 git index-pack --threads=4 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 8 threads' '\n+\tGIT_DIR=t5 git index-pack --threads=8 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack default number of threads' '\n+\tGIT_DIR=t6 git index-pack --stdin < $PACK\n+'\n+\n+test_done\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"190695","messageId":"7v62ccex0x.fsf@alter.siamese.dyndns.org","threadId":"30203","inReplyTo":"1334123388-6083-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v3 3/3] index-pack: support multithreaded delta resolving","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-03T22:10:22Z","receivedAt":"2012-05-03T22:10:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> @@ -696,7 +796,31 @@ static void second_pass(struct object_entry *obj)\n>  \tbase_obj->obj = obj;\n>  \tbase_obj->data = NULL;\n>  \tfind_unresolved_deltas(base_obj);\n> -\tdisplay_progress(progress, nr_resolved_deltas);\n> +}\n> +\n> +static void *threaded_second_pass(void *arg)\n> +{\n> +#ifndef NO_PTHREADS\n> +\tif (threads_active)\n> +\t\tpthread_setspecific(key, arg);\n> +#endif\n> +\tfor (;;) {\n> +\t\tint i;\n> +\t\twork_lock();\n> +\t\tdisplay_progress(progress, nr_resolved_deltas);\n> +\t\twhile (nr_processed < nr_objects &&\n> +\t\t       is_delta_type(objects[nr_processed].type))\n> +\t\t\tnr_processed++;\n> +\t\tif (nr_processed >= nr_objects) {\n> +\t\t\twork_unlock();\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\ti = nr_processed++;\n> +\t\twork_unlock();\n> +\n> +\t\tsecond_pass(&objects[i]);\n> +\t}\n> +\treturn NULL;\n>  }\n\nIt may be just the matter of naming, but the above is taking the\nabstraction backwards, I think.  Shouldn't it be structured in such a way\nthat the caller may call second_pass() and its implementation may turn out\nto be threaded (or not)?\n\nThe naming of \"arg\" made things worse.  I wasted 5 minutes scratching my\nhead thinking \"arg\" was a single specific object that was to be given to\nsecond_pass(), and wondered why it is made into thread-local data.  Name\nit \"thread_data\" or something.\n\nAnd I think the root cause of this confusion is the way \"second_pass\" was\nsplit out in the earlier patch.  It is not the entire second-pass, but is\nmerely a single step of it (the whole \"for (i = 0; i < nr_objects; i++)\"\nis the second-pass, in other words), and its implementation detail might\nchange to either thread (i.e. instead of a single line of control\niterating from 0 to nr_objects, each thread grab the next available task\nand work on it until everything is exhausted) or not.\n\nBy the way, if one object is very heavy and takes a lot of time until\ncompletion, could it be possible that objects[0] is still being processed\nfor its base data but objects[1] has already completed and an available\nthread could work on objects[2]?  How does it learn to process objects[2]\nin such a case, or does it wait until the thread working on objects[0] is\ndone?\n"},{"id":"190723","messageId":"7vipgccvp9.fsf@alter.siamese.dyndns.org","threadId":"30203","inReplyTo":"7v62ccex0x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/3] index-pack: support multithreaded delta resolving","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-04T06:21:54Z","receivedAt":"2012-05-04T06:21:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> @@ -696,7 +796,31 @@ static void second_pass(struct object_entry *obj)\n>>  \tbase_obj->obj = obj;\n>>  \tbase_obj->data = NULL;\n>>  \tfind_unresolved_deltas(base_obj);\n>> -\tdisplay_progress(progress, nr_resolved_deltas);\n>> +}\n>> +\n>> +static void *threaded_second_pass(void *arg)\n>> +{\n>> +#ifndef NO_PTHREADS\n>> +\tif (threads_active)\n>> +\t\tpthread_setspecific(key, arg);\n>> +#endif\n>> +\tfor (;;) {\n>> +\t\tint i;\n>> +\t\twork_lock();\n>> +\t\tdisplay_progress(progress, nr_resolved_deltas);\n>> +\t\twhile (nr_processed < nr_objects &&\n>> +\t\t       is_delta_type(objects[nr_processed].type))\n>> +\t\t\tnr_processed++;\n>> +\t\tif (nr_processed >= nr_objects) {\n>> +\t\t\twork_unlock();\n>> +\t\t\tbreak;\n>> +\t\t}\n>> +\t\ti = nr_processed++;\n>> +\t\twork_unlock();\n>> +\n>> +\t\tsecond_pass(&objects[i]);\n>> +\t}\n>> +\treturn NULL;\n>>  }\n>\n> It may be just the matter of naming, but the above is taking the\n> abstraction backwards, I think.  Shouldn't it be structured in such a way\n> that the caller may call second_pass() and its implementation may turn out\n> to be threaded (or not)?\n>\n> The naming of \"arg\" made things worse.  I wasted 5 minutes scratching my\n> head thinking \"arg\" was a single specific object that was to be given to\n> second_pass(), and wondered why it is made into thread-local data.  Name\n> it \"thread_data\" or something.\n>\n> And I think the root cause of this confusion is the way \"second_pass\" was\n> split out in the earlier patch.  It is not the entire second-pass, but is\n> merely a single step of it (the whole \"for (i = 0; i < nr_objects; i++)\"\n> is the second-pass, in other words), and its implementation detail might\n> change to either thread (i.e. instead of a single line of control\n> iterating from 0 to nr_objects, each thread grab the next available task\n> and work on it until everything is exhausted) or not.\n>\n> By the way, if one object is very heavy and takes a lot of time until\n> completion, could it be possible that objects[0] is still being processed\n> for its base data but objects[1] has already completed and an available\n> thread could work on objects[2]?  How does it learn to process objects[2]\n> in such a case, or does it wait until the thread working on objects[0] is\n> done?\n\nPlease disregard the \"By the way\" part, except that my confusion that led\nto the \"By the way\" comment was caused by another misnaming, namely,\n\"nr_processed\".  It is not counting \"How many of them have we already\nprocessed?\"---it merely counts \"How many of them have we dispatched?\" and\ncompletion of the task does not matter in this critical section, which I\nmissed.  If it were named \"nr_dispatched\", I wouldn't have wasted my time\nwondering about the loop and writing the \"By the way\" review comment.\n"},{"id":"190746","messageId":"CACsJy8DHtBZdhoh1BPwn2_-3wVDRWypJOg3kR9wjd2o1bLUW-Q@mail.gmail.com","threadId":"30203","inReplyTo":"7vipgccvp9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 3/3] index-pack: support multithreaded delta resolving","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-04T12:50:26Z","receivedAt":"2012-05-04T12:50:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Junio, can you make a patch (or update current ones) with better\nnaming? I obviously did not see why these names were bad. You may\nprovide more convincing explanation than me in the commit message.\n\nOn Fri, May 4, 2012 at 1:21 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>>\n>>> @@ -696,7 +796,31 @@ static void second_pass(struct object_entry *obj)\n>>>      base_obj->obj = obj;\n>>>      base_obj->data = NULL;\n>>>      find_unresolved_deltas(base_obj);\n>>> -    display_progress(progress, nr_resolved_deltas);\n>>> +}\n>>> +\n>>> +static void *threaded_second_pass(void *arg)\n>>> +{\n>>> +#ifndef NO_PTHREADS\n>>> +    if (threads_active)\n>>> +            pthread_setspecific(key, arg);\n>>> +#endif\n>>> +    for (;;) {\n>>> +            int i;\n>>> +            work_lock();\n>>> +            display_progress(progress, nr_resolved_deltas);\n>>> +            while (nr_processed < nr_objects &&\n>>> +                   is_delta_type(objects[nr_processed].type))\n>>> +                    nr_processed++;\n>>> +            if (nr_processed >= nr_objects) {\n>>> +                    work_unlock();\n>>> +                    break;\n>>> +            }\n>>> +            i = nr_processed++;\n>>> +            work_unlock();\n>>> +\n>>> +            second_pass(&objects[i]);\n>>> +    }\n>>> +    return NULL;\n>>>  }\n>>\n>> It may be just the matter of naming, but the above is taking the\n>> abstraction backwards, I think.  Shouldn't it be structured in such a way\n>> that the caller may call second_pass() and its implementation may turn out\n>> to be threaded (or not)?\n>>\n>> The naming of \"arg\" made things worse.  I wasted 5 minutes scratching my\n>> head thinking \"arg\" was a single specific object that was to be given to\n>> second_pass(), and wondered why it is made into thread-local data.  Name\n>> it \"thread_data\" or something.\n>>\n>> And I think the root cause of this confusion is the way \"second_pass\" was\n>> split out in the earlier patch.  It is not the entire second-pass, but is\n>> merely a single step of it (the whole \"for (i = 0; i < nr_objects; i++)\"\n>> is the second-pass, in other words), and its implementation detail might\n>> change to either thread (i.e. instead of a single line of control\n>> iterating from 0 to nr_objects, each thread grab the next available task\n>> and work on it until everything is exhausted) or not.\n>>\n>> By the way, if one object is very heavy and takes a lot of time until\n>> completion, could it be possible that objects[0] is still being processed\n>> for its base data but objects[1] has already completed and an available\n>> thread could work on objects[2]?  How does it learn to process objects[2]\n>> in such a case, or does it wait until the thread working on objects[0] is\n>> done?\n>\n> Please disregard the \"By the way\" part, except that my confusion that led\n> to the \"By the way\" comment was caused by another misnaming, namely,\n> \"nr_processed\".  It is not counting \"How many of them have we already\n> processed?\"---it merely counts \"How many of them have we dispatched?\" and\n> completion of the task does not matter in this critical section, which I\n> missed.  If it were named \"nr_dispatched\", I wouldn't have wasted my time\n> wondering about the loop and writing the \"By the way\" review comment.\n>\n\n\n\n-- \nDuy\n"},{"id":"190758","messageId":"7v8vh8c6mx.fsf@alter.siamese.dyndns.org","threadId":"30203","inReplyTo":"CACsJy8DHtBZdhoh1BPwn2_-3wVDRWypJOg3kR9wjd2o1bLUW-Q@mail.gmail.com","subject":"Re: [PATCH v3 3/3] index-pack: support multithreaded delta resolving","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-04T15:23:18Z","receivedAt":"2012-05-04T15:23:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> Junio, can you make a patch (or update current ones) with better\n> naming? I obviously did not see why these names were bad. You may\n> provide more convincing explanation than me in the commit message.\n\nI have already explained s/nr_processed/nr_dispatched/.\n\nI do not think second_pass vs threaded_second_pass is merely a naming\nissue. The code structure (\"taking the abstraction backwards\") is the\nbigger issue.\n\nThe original before the series has:\n\n\tparse_pack_objects()\n\t{\n\t\t... a lot of code for the first pass ...\n\t\t\n\t\t... a lot of code for the second pass which looks like\n\t\tfor (all object entries) {\n\t\t\tset global state base_obj->obj to it\n\t\t\tfind unresolved deltas for that single base_obj\n\t\t\tdisplay progress\n\t\t}\n\t}\n\nand you first split one iteration of the second pass, resulting in\n\n\tparse_pack_objects()\n\t{\n\t\t... a lot of code for the first pass ...\n\t\t\n\t\tfor (all object entries) {\n\t\t\tset global state base_obj->obj to it\n\t\t\tsecond_pass()\n\t\t}\n\t}\n\nwhich I think is a wrong way to do it, because here is what your next step\nends up with because of that:\n\n\tparse_pack_objects()\n\t{\n\t\t... a lot of code for the first pass ...\n\t\t\n\t#if WE SUPPORT THREADING\n\t\tfor (number of threads we are going to spawn) {\n\t\t\tspawn thread and let it run threaded_second_pass\n\t\t}\n\t\tfor (all threads) {\n\t\t\tcull them when they are done\n\t\t}\n\t\treturn\n\t#endif\n\t\t... when not threading ...\n\t\trun threaded_second_pass() in the main process\n\t}\n\nIt could (and should) be like this instead from the beginning, I think:\n\n\tparse_pack_objects()\n\t{\n\t\t... a lot of code for the first pass ...\n\t\tsecond_pass()\n\t}\n\nto encapsulate the whole second_pass() logic in the refactored function,\nwhose implementation (i.e. how the objects to be processed are picked up\nand worked on) will change in the threaded version.  In the first\nrefactoring patch, second_pass() would still iterate over object entries:\n\n\tsecond_pass()\n\t{\n\t\tfor (all object entries) {\n\t\t\tresolve_deltas(base_obj)\n\t\t}\n\t}\n\nAnd at this point, introduce a helper function \"resolve_deltas(base)\"\nor something that is your \"second_pass()\".  This deals with only one\nfamily of objects that use the given object as their (recursive) base\nobjects, and use it in the above loop.\n\nAnd then multi-threading support can turn that into something like\n\n\tsecond_pass()\n\t{\n\t#if WE SUPPORT THREADING\n\t\tfor (number of threads we are going to spawn) {\n\t\t\tspawn thread and let it run threaded_second_pass\n\t\t}\n\t\tfor (all threads) {\n\t\t\tcull them when they are done\n\t\t}\n\t\treturn\n\t#endif\n\t\t... when not threading ...\n\t\tfor (all object entries) {\n\t\t\tresolve_deltas(base_obj)\n\t\t}\n\t}\n\nto add support for threaded case.  The threaded_second_pass function does\nonly one thing and one thing well:\n\n\tthreaded_second_pass()\n\t{\n\t\tset thread local\n\t\tloop forever {\n\t\t\ttake work item under critical section\n\t\t\tbreak if there is no more work\n\t\t\tresolve_deltas(the work item)\n\t\t}\n\t}\n\nWouldn't the result be much more readable that way?\n\nYou may have saved a few lines by making unthreaded code call your\nthreaded_second_pass() and pretend that the single main process still has\nto pick up a work item and work on it in a loop like you did, but I think\nit made the logic unnecessarily harder to follow.\n"},{"id":"190901","messageId":"1336307516-1809-1-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"7v8vh8c6mx.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/4] compat/win32/pthread.h: Add an pthread_key_delete() implementation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-06T12:31:53Z","receivedAt":"2012-05-06T12:31:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n\nSigned-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n compat/win32/pthread.h |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nindex 2e20548..8ad1873 100644\n--- a/compat/win32/pthread.h\n+++ b/compat/win32/pthread.h\n@@ -86,6 +86,11 @@ static inline int pthread_key_create(pthread_key_t *keyp, void (*destructor)(voi\n \treturn (*keyp = TlsAlloc()) == TLS_OUT_OF_INDEXES ? EAGAIN : 0;\n }\n \n+static inline int pthread_key_delete(pthread_key_t key)\n+{\n+\treturn TlsFree(key) ? 0 : EINVAL;\n+}\n+\n static inline int pthread_setspecific(pthread_key_t key, const void *value)\n {\n \treturn TlsSetValue(key, (void *)value) ? 0 : EINVAL;\n-- \n1.7.8.36.g69ee2\n"},{"id":"190902","messageId":"1336307516-1809-2-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"1336307516-1809-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/4] index-pack: restructure pack processing into three main functions","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-06T12:31:54Z","receivedAt":"2012-05-06T12:31:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The second pass in parse_pack_objects() are split into\nresolve_deltas(). The final phase, fixing thin pack or just seal the\npack, is now in conclude_pack() function. Main pack processing is now\na sequence of these functions:\n\n - parse_pack_objects() reads through the input pack\n - resolve_deltas()     makes sure all deltas can be resolved\n - conclude_pack()      seals the output pack\n - write_idx_file()     writes companion index file\n - final()              moves the pack/index to proper place\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/index-pack.c |  128 +++++++++++++++++++++++++++++---------------------\n 1 files changed, 75 insertions(+), 53 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex dd1c5c9..a4be4a6 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -682,19 +682,26 @@ static int compare_delta_entry(const void *a, const void *b)\n \t\t\t\t   objects[delta_b->obj_no].type);\n }\n \n-/* Parse all objects and return the pack content SHA1 hash */\n+static void resolve_base(struct object_entry *obj)\n+{\n+\tstruct base_data *base_obj = alloc_base_data();\n+\tbase_obj->obj = obj;\n+\tbase_obj->data = NULL;\n+\tfind_unresolved_deltas(base_obj);\n+}\n+\n+/*\n+ * First pass:\n+ * - find locations of all objects;\n+ * - calculate SHA1 of all non-delta objects;\n+ * - remember base (SHA1 or offset) for all deltas.\n+ */\n static void parse_pack_objects(unsigned char *sha1)\n {\n \tint i;\n \tstruct delta_entry *delta = deltas;\n \tstruct stat st;\n \n-\t/*\n-\t * First pass:\n-\t * - find locations of all objects;\n-\t * - calculate SHA1 of all non-delta objects;\n-\t * - remember base (SHA1 or offset) for all deltas.\n-\t */\n \tif (verbose)\n \t\tprogress = start_progress(\n \t\t\t\tfrom_stdin ? \"Receiving objects\" : \"Indexing objects\",\n@@ -728,6 +735,19 @@ static void parse_pack_objects(unsigned char *sha1)\n \tif (S_ISREG(st.st_mode) &&\n \t\t\tlseek(input_fd, 0, SEEK_CUR) - input_len != st.st_size)\n \t\tdie(\"pack has junk at the end\");\n+}\n+\n+/*\n+ * Second pass:\n+ * - for all non-delta objects, look if it is used as a base for\n+ *   deltas;\n+ * - if used as a base, uncompress the object and apply all deltas,\n+ *   recursively checking if the resulting object is used as a base\n+ *   for some more deltas.\n+ */\n+static void resolve_deltas(void)\n+{\n+\tint i;\n \n \tif (!nr_deltas)\n \t\treturn;\n@@ -736,29 +756,63 @@ static void parse_pack_objects(unsigned char *sha1)\n \tqsort(deltas, nr_deltas, sizeof(struct delta_entry),\n \t      compare_delta_entry);\n \n-\t/*\n-\t * Second pass:\n-\t * - for all non-delta objects, look if it is used as a base for\n-\t *   deltas;\n-\t * - if used as a base, uncompress the object and apply all deltas,\n-\t *   recursively checking if the resulting object is used as a base\n-\t *   for some more deltas.\n-\t */\n \tif (verbose)\n \t\tprogress = start_progress(\"Resolving deltas\", nr_deltas);\n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *obj = &objects[i];\n-\t\tstruct base_data *base_obj = alloc_base_data();\n \n \t\tif (is_delta_type(obj->type))\n \t\t\tcontinue;\n-\t\tbase_obj->obj = obj;\n-\t\tbase_obj->data = NULL;\n-\t\tfind_unresolved_deltas(base_obj);\n+\t\tresolve_base(obj);\n \t\tdisplay_progress(progress, nr_resolved_deltas);\n \t}\n }\n \n+/*\n+ * Third pass:\n+ * - append objects to convert thin pack to full pack if required\n+ * - write the final 20-byte SHA-1\n+ */\n+static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved);\n+static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned char *pack_sha1)\n+{\n+\tif (nr_deltas == nr_resolved_deltas) {\n+\t\tstop_progress(&progress);\n+\t\t/* Flush remaining pack final 20-byte SHA1. */\n+\t\tflush();\n+\t\treturn;\n+\t}\n+\n+\tif (fix_thin_pack) {\n+\t\tstruct sha1file *f;\n+\t\tunsigned char read_sha1[20], tail_sha1[20];\n+\t\tchar msg[48];\n+\t\tint nr_unresolved = nr_deltas - nr_resolved_deltas;\n+\t\tint nr_objects_initial = nr_objects;\n+\t\tif (nr_unresolved <= 0)\n+\t\t\tdie(\"confusion beyond insanity\");\n+\t\tobjects = xrealloc(objects,\n+\t\t\t\t   (nr_objects + nr_unresolved + 1)\n+\t\t\t\t   * sizeof(*objects));\n+\t\tf = sha1fd(output_fd, curr_pack);\n+\t\tfix_unresolved_deltas(f, nr_unresolved);\n+\t\tsprintf(msg, \"completed with %d local objects\",\n+\t\t\tnr_objects - nr_objects_initial);\n+\t\tstop_progress_msg(&progress, msg);\n+\t\tsha1close(f, tail_sha1, 0);\n+\t\thashcpy(read_sha1, pack_sha1);\n+\t\tfixup_pack_header_footer(output_fd, pack_sha1,\n+\t\t\t\t\t curr_pack, nr_objects,\n+\t\t\t\t\t read_sha1, consumed_bytes-20);\n+\t\tif (hashcmp(read_sha1, tail_sha1) != 0)\n+\t\t\tdie(\"Unexpected tail checksum for %s \"\n+\t\t\t    \"(disk corruption?)\", curr_pack);\n+\t}\n+\tif (nr_deltas != nr_resolved_deltas)\n+\t\tdie(\"pack has %d unresolved deltas\",\n+\t\t    nr_deltas - nr_resolved_deltas);\n+}\n+\n static int write_compressed(struct sha1file *f, void *in, unsigned int size)\n {\n \tgit_zstream stream;\n@@ -1196,40 +1250,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tobjects = xcalloc(nr_objects + 1, sizeof(struct object_entry));\n \tdeltas = xcalloc(nr_objects, sizeof(struct delta_entry));\n \tparse_pack_objects(pack_sha1);\n-\tif (nr_deltas == nr_resolved_deltas) {\n-\t\tstop_progress(&progress);\n-\t\t/* Flush remaining pack final 20-byte SHA1. */\n-\t\tflush();\n-\t} else {\n-\t\tif (fix_thin_pack) {\n-\t\t\tstruct sha1file *f;\n-\t\t\tunsigned char read_sha1[20], tail_sha1[20];\n-\t\t\tchar msg[48];\n-\t\t\tint nr_unresolved = nr_deltas - nr_resolved_deltas;\n-\t\t\tint nr_objects_initial = nr_objects;\n-\t\t\tif (nr_unresolved <= 0)\n-\t\t\t\tdie(\"confusion beyond insanity\");\n-\t\t\tobjects = xrealloc(objects,\n-\t\t\t\t\t   (nr_objects + nr_unresolved + 1)\n-\t\t\t\t\t   * sizeof(*objects));\n-\t\t\tf = sha1fd(output_fd, curr_pack);\n-\t\t\tfix_unresolved_deltas(f, nr_unresolved);\n-\t\t\tsprintf(msg, \"completed with %d local objects\",\n-\t\t\t\tnr_objects - nr_objects_initial);\n-\t\t\tstop_progress_msg(&progress, msg);\n-\t\t\tsha1close(f, tail_sha1, 0);\n-\t\t\thashcpy(read_sha1, pack_sha1);\n-\t\t\tfixup_pack_header_footer(output_fd, pack_sha1,\n-\t\t\t\t\t\t curr_pack, nr_objects,\n-\t\t\t\t\t\t read_sha1, consumed_bytes-20);\n-\t\t\tif (hashcmp(read_sha1, tail_sha1) != 0)\n-\t\t\t\tdie(\"Unexpected tail checksum for %s \"\n-\t\t\t\t    \"(disk corruption?)\", curr_pack);\n-\t\t}\n-\t\tif (nr_deltas != nr_resolved_deltas)\n-\t\t\tdie(\"pack has %d unresolved deltas\",\n-\t\t\t    nr_deltas - nr_resolved_deltas);\n-\t}\n+\tresolve_deltas();\n+\tconclude_pack(fix_thin_pack, curr_pack, pack_sha1);\n \tfree(deltas);\n \tif (strict)\n \t\tcheck_objects();\n-- \n1.7.8.36.g69ee2\n"},{"id":"190903","messageId":"1336307516-1809-3-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"1336307516-1809-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/4] index-pack: support multithreaded delta resolving","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-06T12:31:55Z","receivedAt":"2012-05-06T12:31:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This puts delta resolving on each base on a separate thread, one base\ncache per thread. Per-thread data is grouped in struct thread_local.\nWhen running with nr_threads == 1, no pthreads calls are made. The\nsystem essentially runs in non-thread mode.\n\nAn experiment on a Xeon 24 core machine with git.git shows that\nperformance does not increase proportional to the number of cores. So\nby default, we use maximum 3 cores. Some numbers with --threads from 1\nto 16:\n\n1..4\nreal    0m8.003s  0m5.307s  0m4.321s  0m3.830s\nuser    0m7.720s  0m8.009s  0m8.133s  0m8.305s\nsys     0m0.224s  0m0.372s  0m0.360s  0m0.360s\n\n5..8\nreal    0m3.727s  0m3.604s  0m3.332s  0m3.369s\nuser    0m9.361s  0m9.817s  0m9.525s  0m9.769s\nsys     0m0.584s  0m0.624s  0m0.540s  0m0.560s\n\n9..12\nreal    0m3.036s  0m3.139s  0m3.177s  0m2.961s\nuser    0m8.977s  0m10.205s 0m9.737s  0m10.073s\nsys     0m0.596s  0m0.680s  0m0.684s  0m0.680s\n\n13..16\nreal    0m2.985s  0m2.894s  0m2.975s  0m2.971s\nuser    0m9.825s  0m10.573s 0m10.833s 0m11.361s\nsys     0m0.788s  0m0.732s  0m0.904s  0m1.016s\n\nOn an Intel dual core and linux-2.6.git\n\n1..4\nreal    2m37.789s 2m7.963s  2m0.920s  1m58.213s\nuser    2m28.415s 2m52.325s 2m50.176s 2m41.187s\nsys     0m7.808s  0m11.181s 0m11.224s 0m10.731s\n\nThanks Ramsay Jones for troubleshooting and support on MinGW platform.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Changes against 'pu':\n\n - nr_processed -> nr_dispatched\n - both nr_dispatched threads_active are moved to #ifndef NO_PTHREADS block\n - old second_pass() -> resolve_base()\n\n Documentation/git-index-pack.txt |   10 ++\n Makefile                         |    2 +-\n builtin/index-pack.c             |  204 +++++++++++++++++++++++++++++++++++--\n t/perf/p5302-pack-index.sh       |   40 ++++++++\n 4 files changed, 244 insertions(+), 12 deletions(-)\n create mode 100755 t/perf/p5302-pack-index.sh\n\ndiff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt\nindex 909687f..39e6d0d 100644\n--- a/Documentation/git-index-pack.txt\n+++ b/Documentation/git-index-pack.txt\n@@ -74,6 +74,16 @@ OPTIONS\n --strict::\n \tDie, if the pack contains broken objects or links.\n \n+--threads=<n>::\n+\tSpecifies the number of threads to spawn when resolving\n+\tdeltas. This requires that index-pack be compiled with\n+\tpthreads otherwise this option is ignored with a warning.\n+\tThis is meant to reduce packing time on multiprocessor\n+\tmachines. The required amount of memory for the delta search\n+\twindow is however multiplied by the number of threads.\n+\tSpecifying 0 will cause git to auto-detect the number of CPU's\n+\tand use maximum 3 threads.\n+\n \n Note\n ----\ndiff --git a/Makefile b/Makefile\nindex cf2c40b..e41955f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2160,7 +2160,7 @@ builtin/branch.o builtin/checkout.o builtin/clone.o builtin/reset.o branch.o tra\n builtin/bundle.o bundle.o transport.o: bundle.h\n builtin/bisect--helper.o builtin/rev-list.o bisect.o: bisect.h\n builtin/clone.o builtin/fetch-pack.o transport.o: fetch-pack.h\n-builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.h\n+builtin/index-pack.o builtin/grep.o builtin/pack-objects.o transport-helper.o thread-utils.o: thread-utils.h\n builtin/send-pack.o transport.o: send-pack.h\n builtin/log.o builtin/shortlog.o: shortlog.h\n builtin/prune.o builtin/reflog.o reachable.o: reachable.h\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex a4be4a6..d4685c5 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -9,6 +9,7 @@\n #include \"progress.h\"\n #include \"fsck.h\"\n #include \"exec_cmd.h\"\n+#include \"thread-utils.h\"\n \n static const char index_pack_usage[] =\n \"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])\";\n@@ -38,6 +39,14 @@ struct base_data {\n \tint ofs_first, ofs_last;\n };\n \n+struct thread_local {\n+#ifndef NO_PTHREADS\n+\tpthread_t thread;\n+#endif\n+\tstruct base_data *base_cache;\n+\tsize_t base_cache_used;\n+};\n+\n /*\n  * Even if sizeof(union delta_base) == 24 on 64-bit archs, we really want\n  * to memcmp() only the first 20 bytes.\n@@ -54,11 +63,11 @@ struct delta_entry {\n \n static struct object_entry *objects;\n static struct delta_entry *deltas;\n-static struct base_data *base_cache;\n-static size_t base_cache_used;\n+static struct thread_local nothread_data;\n static int nr_objects;\n static int nr_deltas;\n static int nr_resolved_deltas;\n+static int nr_threads;\n \n static int from_stdin;\n static int strict;\n@@ -75,6 +84,77 @@ static git_SHA_CTX input_ctx;\n static uint32_t input_crc32;\n static int input_fd, output_fd, pack_fd;\n \n+#ifndef NO_PTHREADS\n+\n+static struct thread_local *thread_data;\n+static int nr_dispatched;\n+static int threads_active;\n+\n+static pthread_mutex_t read_mutex;\n+#define read_lock()\t\tlock_mutex(&read_mutex)\n+#define read_unlock()\t\tunlock_mutex(&read_mutex)\n+\n+static pthread_mutex_t counter_mutex;\n+#define counter_lock()\t\tlock_mutex(&counter_mutex)\n+#define counter_unlock()\tunlock_mutex(&counter_mutex)\n+\n+static pthread_mutex_t work_mutex;\n+#define work_lock()\t\tlock_mutex(&work_mutex)\n+#define work_unlock()\t\tunlock_mutex(&work_mutex)\n+\n+static pthread_key_t key;\n+\n+static inline void lock_mutex(pthread_mutex_t *mutex)\n+{\n+\tif (threads_active)\n+\t\tpthread_mutex_lock(mutex);\n+}\n+\n+static inline void unlock_mutex(pthread_mutex_t *mutex)\n+{\n+\tif (threads_active)\n+\t\tpthread_mutex_unlock(mutex);\n+}\n+\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_thread(void)\n+{\n+\tinit_recursive_mutex(&read_mutex);\n+\tpthread_mutex_init(&counter_mutex, NULL);\n+\tpthread_mutex_init(&work_mutex, NULL);\n+\tpthread_key_create(&key, NULL);\n+\tthread_data = xcalloc(nr_threads, sizeof(*thread_data));\n+\tthreads_active = 1;\n+}\n+\n+static void cleanup_thread(void)\n+{\n+\tif (!threads_active)\n+\t\treturn;\n+\tthreads_active = 0;\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&counter_mutex);\n+\tpthread_mutex_destroy(&work_mutex);\n+\tpthread_key_delete(key);\n+\tfree(thread_data);\n+}\n+\n+#else\n+\n+#define read_lock()\n+#define read_unlock()\n+\n+#define counter_lock()\n+#define counter_unlock()\n+\n+#define work_lock()\n+#define work_unlock()\n+\n+#endif\n+\n+\n static int mark_link(struct object *obj, int type, void *data)\n {\n \tif (!obj)\n@@ -223,6 +303,25 @@ static NORETURN void bad_object(unsigned long offset, const char *format, ...)\n \tdie(\"pack has bad object at offset %lu: %s\", offset, buf);\n }\n \n+static inline struct thread_local *get_thread_data(void)\n+{\n+#ifndef NO_PTHREADS\n+\tif (threads_active)\n+\t\treturn pthread_getspecific(key);\n+\tassert(!threads_active &&\n+\t       \"This should only be reached when all threads are gone\");\n+#endif\n+\treturn &nothread_data;\n+}\n+\n+#ifndef NO_PTHREADS\n+static void set_thread_data(struct thread_local *data)\n+{\n+\tif (threads_active)\n+\t\tpthread_setspecific(key, data);\n+}\n+#endif\n+\n static struct base_data *alloc_base_data(void)\n {\n \tstruct base_data *base = xmalloc(sizeof(struct base_data));\n@@ -237,15 +336,16 @@ static void free_base_data(struct base_data *c)\n \tif (c->data) {\n \t\tfree(c->data);\n \t\tc->data = NULL;\n-\t\tbase_cache_used -= c->size;\n+\t\tget_thread_data()->base_cache_used -= c->size;\n \t}\n }\n \n static void prune_base_data(struct base_data *retain)\n {\n \tstruct base_data *b;\n-\tfor (b = base_cache;\n-\t     base_cache_used > delta_base_cache_limit && b;\n+\tstruct thread_local *data = get_thread_data();\n+\tfor (b = data->base_cache;\n+\t     data->base_cache_used > delta_base_cache_limit && b;\n \t     b = b->child) {\n \t\tif (b->data && b != retain)\n \t\t\tfree_base_data(b);\n@@ -257,12 +357,12 @@ static void link_base_data(struct base_data *base, struct base_data *c)\n \tif (base)\n \t\tbase->child = c;\n \telse\n-\t\tbase_cache = c;\n+\t\tget_thread_data()->base_cache = c;\n \n \tc->base = base;\n \tc->child = NULL;\n \tif (c->data)\n-\t\tbase_cache_used += c->size;\n+\t\tget_thread_data()->base_cache_used += c->size;\n \tprune_base_data(c);\n }\n \n@@ -272,7 +372,7 @@ static void unlink_base_data(struct base_data *c)\n \tif (base)\n \t\tbase->child = NULL;\n \telse\n-\t\tbase_cache = NULL;\n+\t\tget_thread_data()->base_cache = NULL;\n \tfree_base_data(c);\n }\n \n@@ -461,19 +561,24 @@ static void sha1_object(const void *data, unsigned long size,\n \t\t\tenum object_type type, unsigned char *sha1)\n {\n \thash_sha1_file(data, size, typename(type), sha1);\n+\tread_lock();\n \tif (has_sha1_file(sha1)) {\n \t\tvoid *has_data;\n \t\tenum object_type has_type;\n \t\tunsigned long has_size;\n \t\thas_data = read_sha1_file(sha1, &has_type, &has_size);\n+\t\tread_unlock();\n \t\tif (!has_data)\n \t\t\tdie(\"cannot read existing object %s\", sha1_to_hex(sha1));\n \t\tif (size != has_size || type != has_type ||\n \t\t    memcmp(data, has_data, size) != 0)\n \t\t\tdie(\"SHA1 COLLISION FOUND WITH %s !\", sha1_to_hex(sha1));\n \t\tfree(has_data);\n-\t}\n+\t} else\n+\t\tread_unlock();\n+\n \tif (strict) {\n+\t\tread_lock();\n \t\tif (type == OBJ_BLOB) {\n \t\t\tstruct blob *blob = lookup_blob(sha1);\n \t\t\tif (blob)\n@@ -507,6 +612,7 @@ static void sha1_object(const void *data, unsigned long size,\n \t\t\t}\n \t\t\tobj->flags |= FLAG_CHECKED;\n \t\t}\n+\t\tread_unlock();\n \t}\n }\n \n@@ -552,7 +658,7 @@ static void *get_base_data(struct base_data *c)\n \t\tif (!delta_nr) {\n \t\t\tc->data = get_data_from_pack(obj);\n \t\t\tc->size = obj->size;\n-\t\t\tbase_cache_used += c->size;\n+\t\t\tget_thread_data()->base_cache_used += c->size;\n \t\t\tprune_base_data(c);\n \t\t}\n \t\tfor (; delta_nr > 0; delta_nr--) {\n@@ -568,7 +674,7 @@ static void *get_base_data(struct base_data *c)\n \t\t\tfree(raw);\n \t\t\tif (!c->data)\n \t\t\t\tbad_object(obj->idx.offset, \"failed to apply delta\");\n-\t\t\tbase_cache_used += c->size;\n+\t\t\tget_thread_data()->base_cache_used += c->size;\n \t\t\tprune_base_data(c);\n \t\t}\n \t\tfree(delta);\n@@ -596,7 +702,9 @@ static void resolve_delta(struct object_entry *delta_obj,\n \t\tbad_object(delta_obj->idx.offset, \"failed to apply delta\");\n \tsha1_object(result->data, result->size, delta_obj->real_type,\n \t\t    delta_obj->idx.sha1);\n+\tcounter_lock();\n \tnr_resolved_deltas++;\n+\tcounter_unlock();\n }\n \n static struct base_data *find_unresolved_deltas_1(struct base_data *base,\n@@ -690,6 +798,30 @@ static void resolve_base(struct object_entry *obj)\n \tfind_unresolved_deltas(base_obj);\n }\n \n+#ifndef NO_PTHREADS\n+static void *threaded_second_pass(void *data)\n+{\n+\tset_thread_data(data);\n+\tfor (;;) {\n+\t\tint i;\n+\t\twork_lock();\n+\t\tdisplay_progress(progress, nr_resolved_deltas);\n+\t\twhile (nr_dispatched < nr_objects &&\n+\t\t       is_delta_type(objects[nr_dispatched].type))\n+\t\t\tnr_dispatched++;\n+\t\tif (nr_dispatched >= nr_objects) {\n+\t\t\twork_unlock();\n+\t\t\tbreak;\n+\t\t}\n+\t\ti = nr_dispatched++;\n+\t\twork_unlock();\n+\n+\t\tresolve_base(&objects[i]);\n+\t}\n+\treturn NULL;\n+}\n+#endif\n+\n /*\n  * First pass:\n  * - find locations of all objects;\n@@ -758,6 +890,24 @@ static void resolve_deltas(void)\n \n \tif (verbose)\n \t\tprogress = start_progress(\"Resolving deltas\", nr_deltas);\n+\n+#ifndef NO_PTHREADS\n+\tnr_dispatched = 0;\n+\tif (nr_threads > 1 || getenv(\"GIT_FORCE_THREADS\")) {\n+\t\tinit_thread();\n+\t\tfor (i = 0; i < nr_threads; i++) {\n+\t\t\tint ret = pthread_create(&thread_data[i].thread, NULL,\n+\t\t\t\t\t\t threaded_second_pass, thread_data + i);\n+\t\t\tif (ret)\n+\t\t\t\tdie(\"unable to create thread: %s\", strerror(ret));\n+\t\t}\n+\t\tfor (i = 0; i < nr_threads; i++)\n+\t\t\tpthread_join(thread_data[i].thread, NULL);\n+\t\tcleanup_thread();\n+\t\treturn;\n+\t}\n+#endif\n+\n \tfor (i = 0; i < nr_objects; i++) {\n \t\tstruct object_entry *obj = &objects[i];\n \n@@ -1016,6 +1166,18 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\t\tdie(\"bad pack.indexversion=%\"PRIu32, opts->version);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"pack.threads\")) {\n+\t\tnr_threads = git_config_int(k, v);\n+\t\tif (nr_threads < 0)\n+\t\t\tdie(\"invalid number of threads specified (%d)\",\n+\t\t\t    nr_threads);\n+#ifdef NO_PTHREADS\n+\t\tif (nr_threads != 1)\n+\t\t\twarning(\"no threads support, ignoring %s\", k);\n+\t\tnr_threads = 1;\n+#endif\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(k, v, cb);\n }\n \n@@ -1174,6 +1336,17 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tkeep_msg = \"\";\n \t\t\t} else if (!prefixcmp(arg, \"--keep=\")) {\n \t\t\t\tkeep_msg = arg + 7;\n+\t\t\t} else if (!prefixcmp(arg, \"--threads=\")) {\n+\t\t\t\tchar *end;\n+\t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n+\t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n+\t\t\t\t\tusage(index_pack_usage);\n+#ifdef NO_PTHREADS\n+\t\t\t\tif (nr_threads != 1)\n+\t\t\t\t\twarning(\"no threads support, \"\n+\t\t\t\t\t\t\"ignoring %s\", arg);\n+\t\t\t\tnr_threads = 1;\n+#endif\n \t\t\t} else if (!prefixcmp(arg, \"--pack_header=\")) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n@@ -1245,6 +1418,15 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (strict)\n \t\topts.flags |= WRITE_IDX_STRICT;\n \n+#ifndef NO_PTHREADS\n+\tif (!nr_threads) {\n+\t\tnr_threads = online_cpus();\n+\t\t/* An experiment showed that more threads does not mean faster */\n+\t\tif (nr_threads > 3)\n+\t\t\tnr_threads = 3;\n+\t}\n+#endif\n+\n \tcurr_pack = open_pack_file(pack_name);\n \tparse_pack_header();\n \tobjects = xcalloc(nr_objects + 1, sizeof(struct object_entry));\ndiff --git a/t/perf/p5302-pack-index.sh b/t/perf/p5302-pack-index.sh\nnew file mode 100755\nindex 0000000..6cb5b0d\n--- /dev/null\n+++ b/t/perf/p5302-pack-index.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description=\"Tests index-pack performance\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+\n+test_expect_success 'repack' '\n+\tgit repack -ad &&\n+\tPACK=`ls .git/objects/pack/*.pack | head -n1` &&\n+\ttest -f \"$PACK\" &&\n+\texport PACK\n+'\n+\n+test_perf 'index-pack 0 threads' '\n+\tGIT_DIR=t1 git index-pack --threads=1 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 1 thread ' '\n+\tGIT_DIR=t2 GIT_FORCE_THREADS=1 git index-pack --threads=1 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 2 threads' '\n+\tGIT_DIR=t3 git index-pack --threads=2 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 4 threads' '\n+\tGIT_DIR=t4 git index-pack --threads=4 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack 8 threads' '\n+\tGIT_DIR=t5 git index-pack --threads=8 --stdin < $PACK\n+'\n+\n+test_perf 'index-pack default number of threads' '\n+\tGIT_DIR=t6 git index-pack --stdin < $PACK\n+'\n+\n+test_done\n-- \n1.7.8.36.g69ee2\n"},{"id":"190904","messageId":"1336307516-1809-4-git-send-email-pclouds@gmail.com","threadId":"30203","inReplyTo":"1336307516-1809-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/4] index-pack: disable threading if NO_PREAD is defined","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-06T12:31:56Z","receivedAt":"2012-05-06T12:31:56Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"NO_PREAD simulates pread() as a sequence of seek, read, seek in\ncompat/pread.c. The simulation is not thread-safe because another\nthread could move the file offset away in the middle of pread\noperation. Do not allow threading in that case.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/index-pack.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex d4685c5..807ee56 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -39,6 +39,11 @@ struct base_data {\n \tint ofs_first, ofs_last;\n };\n \n+#if !defined(NO_PTHREADS) && defined(NO_PREAD)\n+/* NO_PREAD uses compat/pread.c, which is not thread-safe. Disable threading. */\n+#define NO_PTHREADS\n+#endif\n+\n struct thread_local {\n #ifndef NO_PTHREADS\n \tpthread_t thread;\n-- \n1.7.8.36.g69ee2\n"},{"id":"191059","messageId":"7vy5p31q3e.fsf@alter.siamese.dyndns.org","threadId":"30203","inReplyTo":"1336307516-1809-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 2/4] index-pack: restructure pack processing into three main functions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-08T00:19:49Z","receivedAt":"2012-05-08T00:19:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> The second pass in parse_pack_objects() are split into\n> resolve_deltas(). The final phase, fixing thin pack or just seal the\n> pack, is now in conclude_pack() function. Main pack processing is now a\n> sequence of these functions:\n>\n>  - parse_pack_objects() reads through the input pack\n>  - resolve_deltas()     makes sure all deltas can be resolved\n>  - conclude_pack()      seals the output pack\n>  - write_idx_file()     writes companion index file\n>  - final()              moves the pack/index to proper place\n\nThe resulting code looks much more streamlined, instead of just splitting\nout the second phase alone.\n\nVery nice.  Thanks.\n"}]}