{"thread":{"id":"12053","subject":"[PATCH] pack-objects: Add runtime detection of online CPU's","startedAt":"2008-02-12T08:20:29Z","lastAt":"2008-02-26T23:04:52Z","messageCount":22,"participants":["Andreas Ericsson","Shawn O. Pearce","Johannes Sixt","Bert Wesarg","Michael Hendricks","Brandon Casey","Jeff King","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"68494","messageId":"47B156CD.1010209@op5.se","threadId":"12053","inReplyTo":null,"subject":"[PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-02-12T08:20:29Z","receivedAt":"2008-02-12T08:20:29Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Packing objects can be done in parallell nowadays, but it's\nonly done if the config option pack.threads is set to a value\nabove 1. Because of that, the code-path used is often not the\nmost optimal one.\n\nThis patch adds a routine to detect the number of online CPU's\nat runtime (online_cpus()). When pack.threads (or --threads=) is\ngiven a value of 0, the number of threads is set to the number of\nonline CPU's. This feature is also documented.\n\nAs per Nicolas Pitre's recommendations, the default is still to\nrun pack-objects single-threaded unless explicitly activated,\neither by configuration or by command line parameter.\n\nThe routine online_cpus() is a rework of \"numcpus.c\", written by\none Philip Willoughby . numcpus.c is in the\npublic domain and can presently be downloaded from\nhttp://csgsoft.doc.ic.ac.uk/numcpus/\n\nSigned-off-by: Andreas Ericsson \n---\n\nThis patch was built on todays master as of 5 minutes ago\n(40aab8119f38c622f58d8e612e7a632eb1f3ded2).\nAs far as I understood all of Nicolas' comments to my original\npatch and the one sent in by Mr Casey, this implements all the\nsuggestions made to both sets.\n\n Documentation/config.txt | 2 +\n Documentation/git-pack-objects.txt | 2 +\n Makefile | 2 +-\n builtin-pack-objects.c | 13 ++++++-----\n thread-utils.c | 39 ++++++++++++++++++++++++++++++++++++\n thread-utils.h | 19 +++++++++++++++++\n 6 files changed, 70 insertions(+), 7 deletions(-)\n create mode 100644 thread-utils.c\n create mode 100644 thread-utils.h\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f9bdb16..e9f26ed 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -756,6 +756,8 @@ pack.threads::\n \twarning. This is meant to reduce packing time on multiprocessor\n \tmachines. The required amount of memory for the delta search window\n \tis however multiplied by the number of threads.\n+\tSpecifying 0 will cause git to auto-detect the number of CPU's\n+\tand set the number of threads accordingly.\n \n pack.indexVersion::\n \tSpecify the default pack index version. Valid values are 1 for\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8353be1..5c1bd3b 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -177,6 +177,8 @@ base-name::\n \tThis is meant to reduce packing time on multiprocessor machines.\n \tThe required amount of memory for the delta search window is\n \thowever multiplied by the number of threads.\n+\tSpecifying 0 will cause git to auto-detect the number of CPU's\n+\tand set the number of threads accordingly.\n \n --index-version=[,]::\n \tThis is intended to be used by the test suite only. It allows\ndiff --git a/Makefile b/Makefile\nindex 92341c4..9ea378a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -306,7 +306,7 @@ DIFF_OBJS = \\\n \n LIB_OBJS = \\\n \tblob.o commit.o connect.o csum-file.o cache-tree.o base85.o \\\n-\tdate.o diff-delta.o entry.o exec_cmd.o ident.o \\\n+\tdate.o diff-delta.o entry.o exec_cmd.o ident.o thread-utils.o \\\n \tpretty.o interpolate.o hash.o \\\n \tlockfile.o \\\n \tpatch-ids.o \\\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex acb0555..a7ffb53 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -14,10 +14,7 @@\n #include \"revision.h\"\n #include \"list-objects.h\"\n #include \"progress.h\"\n-\n-#ifdef THREADED_DELTA_SEARCH\n-#include \n-#endif\n+#include \"thread-utils.h\"\n \n static const char pack_usage[] = \"\\\n git-pack-objects [{ -q | --progress | --all-progress }] \\n\\\n@@ -1861,7 +1858,7 @@ static int git_pack_config(const char *k, const char *v)\n \t}\n \tif (!strcmp(k, \"pack.threads\")) {\n \t\tdelta_search_threads = git_config_int(k, v);\n-\t\tif (delta_search_threads < 1)\n+\t\tif (delta_search_threads < 0)\n \t\t\tdie(\"invalid number of threads specified (%d)\",\n \t\t\t delta_search_threads);\n #ifndef THREADED_DELTA_SEARCH\n@@ -2076,6 +2073,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!pack_compression_seen && core_compression_seen)\n \t\tpack_compression_level = core_compression_level;\n \n+\tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n+\t\tdelta_search_threads = online_cpus();\n+\n \tprogress = isatty(2);\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -2130,7 +2130,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tif (!prefixcmp(arg, \"--threads=\")) {\n \t\t\tchar *end;\n \t\t\tdelta_search_threads = strtoul(arg+10, &end, 0);\n-\t\t\tif (!arg[10] || *end || delta_search_threads < 1)\n+\n+\t\t\tif (!arg[10] || *end || delta_search_threads < 0)\n \t\t\t\tusage(pack_usage);\n #ifndef THREADED_DELTA_SEARCH\n \t\t\tif (delta_search_threads > 1)\ndiff --git a/thread-utils.c b/thread-utils.c\nnew file mode 100644\nindex 0000000..b19243c\n--- /dev/null\n+++ b/thread-utils.c\n@@ -0,0 +1,39 @@\n+#include \"thread-utils.h\"\n+\n+/*\n+ * By doing this in two steps we can at least get\n+ * get the function to be somewhat coherent, even\n+ * with this disgusting nest of #ifdefs.\n+ */\n+#ifndef _SC_NPROCESSORS_ONLN\n+# ifdef _SC_NPROC_ONLN\n+# define _SC_NPROCESSORS_ONLN _SC_NPROC_ONLN\n+# elif defined _SC_CRAY_NCPU\n+# define _SC_NPROCESSORS_ONLN _SC_CRAY_NCPU\n+# endif\n+#endif\n+int online_cpus(void)\n+{\n+#ifdef THREADED_DELTA_SEARCH\n+# ifdef _SC_NPROCESSORS_ONLN\n+\tlong ncpus;\n+\n+\tif ((ncpus = (long)sysconf(_SC_NPROCESSORS_ONLN)) > 0)\n+\t\treturn (int)ncpus;\n+# else\n+# ifdef _WIN32\n+\tSYSTEM_INFO info;\n+\tGetSystemInfo(&info);\n+\n+\treturn (int)info.dwNumberOfProcessors;\n+# endif /* _WIN32 */\n+# if defined(hpux) || defined(__hpux) || defined(_hpux)\n+\tstruct pst_dynamic psd;\n+\n+\tif (!pstat_getdynamic(&psd, sizeof(psd), (size_t)1, 0))\n+\t\treturn (int)psd.psd_proc_cnt;\n+# endif /* hpux */\n+# endif /* _SC_NPROCESSORS_ONLN */\n+#endif /* THREADED_DELTA_SEARCH */\n+\treturn 1;\n+}\ndiff --git a/thread-utils.h b/thread-utils.h\nnew file mode 100644\nindex 0000000..53754b3\n--- /dev/null\n+++ b/thread-utils.h\n@@ -0,0 +1,19 @@\n+#ifndef THREAD_COMPAT_H\n+#define THREAD_COMPAT_H\n+\n+#include \"cache.h\"\n+\n+#ifdef THREADED_DELTA_SEARCH\n+#include \n+# ifdef _WIN32\n+# define WIN32_LEAN_AND_MEAN\n+# include \n+# endif\n+# if defined(hpux) || defined(__hpux) || defined(_hpux)\n+# include \n+# endif\n+#endif\n+\n+extern int online_cpus(void);\n+\n+#endif /* THREAD_COMPAT_H */\n-- \n1.5.4.rc5.11.g0eab8\n\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"68498","messageId":"20080212082734.GC24004@spearce.org","threadId":"12053","inReplyTo":"47B156CD.1010209@op5.se","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-02-12T08:27:34Z","receivedAt":"2008-02-12T08:27:34Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Andreas Ericsson wrote:\n> diff --git a/thread-utils.h b/thread-utils.h\n> new file mode 100644\n> index 0000000..53754b3\n> --- /dev/null\n> +++ b/thread-utils.h\n> @@ -0,0 +1,19 @@\n> +#ifndef THREAD_COMPAT_H\n> +#define THREAD_COMPAT_H\n> +\n> +#include \"cache.h\"\n> +\n> +#ifdef THREADED_DELTA_SEARCH\n> +#include \n> +# ifdef _WIN32\n> +# define WIN32_LEAN_AND_MEAN\n> +# include \n> +# endif\n> +# if defined(hpux) || defined(__hpux) || defined(_hpux)\n> +# include \n> +# endif\n> +#endif\n\nDo we have to expose this mess of namespaces to those who include\nthread-utils.h? Seems like we don't, as online_cpus has a pretty\nsimple definition:\n\n> +\n> +extern int online_cpus(void);\n> +\n> +#endif /* THREAD_COMPAT_H */\n> -- \n> 1.5.4.rc5.11.g0eab8\n\n-- \nShawn.\n"},{"id":"68500","messageId":"47B15D8C.1000600@viscovery.net","threadId":"12053","inReplyTo":"47B156CD.1010209@op5.se","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-02-12T08:49:16Z","receivedAt":"2008-02-12T08:49:16Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andreas Ericsson schrieb:\n> +#ifndef THREAD_COMPAT_H\n> +#define THREAD_COMPAT_H\n> +\n> +#include \"cache.h\"\n> +\n> +#ifdef THREADED_DELTA_SEARCH\n> +#include \n> +# ifdef _WIN32\n> +# define WIN32_LEAN_AND_MEAN\n> +# include \n> +# endif\n\nAnd now what?\n\nIf you don't provide a pthread_* implementation for Windows, you can just\ndrop the #ifdef _WIN32 part as long as the THREADED_DELTA_SEARCH guards\nremain.\n\n-- Hannes\n"},{"id":"68507","messageId":"36ca99e90802120318y5099b06cta3f8488dc758f6@mail.gmail.com","threadId":"12053","inReplyTo":"47B156CD.1010209@op5.se","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2008-02-12T11:18:57Z","receivedAt":"2008-02-12T11:18:57Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Hi,\n\nOn Feb 12, 2008 9:20 AM, Andreas Ericsson wrote:\n> +#ifdef THREADED_DELTA_SEARCH\n> +# ifdef _SC_NPROCESSORS_ONLN\n> + long ncpus;\n> +\n> + if ((ncpus = (long)sysconf(_SC_NPROCESSORS_ONLN)) > 0)\n> + return (int)ncpus;\n> +# else\nI can't find the right pointer, but for linux it would be more usable\nto use sched_getaffinity(). Than you can do thinks like this:\n\n$ taskset 0x3 git gc ...\n\nand you will get 2 cpus, even 4 are online.\n\nBert\n"},{"id":"68513","messageId":"47B18F5B.7020106@op5.se","threadId":"12053","inReplyTo":"36ca99e90802120318y5099b06cta3f8488dc758f6@mail.gmail.com","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-02-12T12:21:47Z","receivedAt":"2008-02-12T12:21:47Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Bert Wesarg wrote:\n> Hi,\n> \n> On Feb 12, 2008 9:20 AM, Andreas Ericsson wrote:\n>> +#ifdef THREADED_DELTA_SEARCH\n>> +# ifdef _SC_NPROCESSORS_ONLN\n>> + long ncpus;\n>> +\n>> + if ((ncpus = (long)sysconf(_SC_NPROCESSORS_ONLN)) > 0)\n>> + return (int)ncpus;\n>> +# else\n> I can't find the right pointer, but for linux it would be more usable\n> to use sched_getaffinity(). Than you can do thinks like this:\n> \n> $ taskset 0x3 git gc ...\n> \n> and you will get 2 cpus, even 4 are online.\n> \n\nSince you can do roughly the same by saying \"git pack-objects --threads=2\",\nI'd rather not add a GNU/Linux specific hack for this.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"68522","messageId":"20080212145200.GB20686@ginosko.local","threadId":"12053","inReplyTo":"47B156CD.1010209@op5.se","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Michael Hendricks","fromEmail":"michael@ndrix.org","sentAt":"2008-02-12T14:52:01Z","receivedAt":"2008-02-12T14:52:01Z","isPatch":true,"sender":{"key":"michael@ndrix.org","avatar":"https://gravatar.com/avatar/315311e6daa79f24e5648f9534420c24ec48eada42efd4110f1d17167ff44fa8?d=mp&s=160"},"body":"On Tue, Feb 12, 2008 at 09:20:29AM +0100, Andreas Ericsson wrote:\n> + * By doing this in two steps we can at least get\n> + * get the function to be somewhat coherent, even\n> + * with this disgusting nest of #ifdefs.\n\n\"we can at least get get the ...\"\n\nAn extra \"get\" snuck in there.\n\n-- \nMichael\n"},{"id":"68525","messageId":"47B1BEC6.6080906@nrlssc.navy.mil","threadId":"12053","inReplyTo":"47B156CD.1010209@op5.se","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-02-12T15:44:06Z","receivedAt":"2008-02-12T15:44:06Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Andreas Ericsson wrote:\n\n> @@ -1861,7 +1858,7 @@ static int git_pack_config(const char *k, const\n> char *v)\n> }\n> if (!strcmp(k, \"pack.threads\")) {\n> delta_search_threads = git_config_int(k, v);\n> - if (delta_search_threads < 1)\n> + if (delta_search_threads < 0)\n> die(\"invalid number of threads specified (%d)\",\n> delta_search_threads);\n> #ifndef THREADED_DELTA_SEARCH\n\n\tif (delta_search_threads != 1)\n\t\twarning(\"no threads support, ignoring %s\", k);\n\nI changed this to '!= 1' since that is the only time the user gets what they\nasked for when THREADED_DELTA_SEARCH is not enabled. If the user requested\nnthreads == ncpus by setting delta_search_threads = 0, I think we should\nlet the user know that thread support is not enabled, and we are ignoring\ntheir request.\n\n> @@ -2076,6 +2073,9 @@ int cmd_pack_objects(int argc, const char **argv,\n> const char *prefix)\n> if (!pack_compression_seen && core_compression_seen)\n> pack_compression_level = core_compression_level;\n> \n> + if (!delta_search_threads) /* --threads=0 means autodetect */\n> + delta_search_threads = online_cpus();\n\n\nThis is in the wrong place. It should be _after_ command line arguments are\nprocessed to handle --threads=0\n\n\n> +\n> progress = isatty(2);\n> for (i = 1; i < argc; i++) {\n> const char *arg = argv[i];\n> @@ -2130,7 +2130,8 @@ int cmd_pack_objects(int argc, const char **argv,\n> const char *prefix)\n> if (!prefixcmp(arg, \"--threads=\")) {\n> char *end;\n> delta_search_threads = strtoul(arg+10, &end, 0);\n> - if (!arg[10] || *end || delta_search_threads < 1)\n> +\n> + if (!arg[10] || *end || delta_search_threads < 0)\n> usage(pack_usage);\n> #ifndef THREADED_DELTA_SEARCH\n> if (delta_search_threads > 1)\n\nSame comment as above about warning when delta_search_threads != 1.\n\n-brandon\n"},{"id":"69642","messageId":"47BF80EC.4080608@nrlssc.navy.mil","threadId":"12053","inReplyTo":"47B1BEC6.6080906@nrlssc.navy.mil","subject":"[PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-02-23T02:11:56Z","receivedAt":"2008-02-23T02:11:56Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Andreas Ericsson \n\nPacking objects can be done in parallell nowadays, but it's\nonly done if the config option pack.threads is set to a value\nabove 1. Because of that, the code-path used is often not the\nmost optimal one.\n\nThis patch adds a routine to detect the number of online CPU's\nat runtime (online_cpus()). When pack.threads (or --threads=) is\ngiven a value of 0, the number of threads is set to the number of\nonline CPU's. This feature is also documented.\n\nAs per Nicolas Pitre's recommendations, the default is still to\nrun pack-objects single-threaded unless explicitly activated,\neither by configuration or by command line parameter.\n\nThe routine online_cpus() is a rework of \"numcpus.c\", written by\none Philip Willoughby . numcpus.c is in the\npublic domain and can presently be downloaded from\nhttp://csgsoft.doc.ic.ac.uk/numcpus/\n\nSigned-off-by: Andreas Ericsson \nSigned-off-by: Brandon Casey \n---\n\n\nI reworked this patch from Andreas for detecting the number of online CPU's.\nI kept the commit message and the Signed-off-by and added my own. I'm not sure\nwhat the procedure is here.\n\n-brandon\n\n\n Documentation/config.txt | 2 +\n Documentation/git-pack-objects.txt | 2 +\n Makefile | 1 +\n builtin-pack-objects.c | 14 +++++++---\n thread-utils.c | 48 ++++++++++++++++++++++++++++++++++++\n thread-utils.h | 6 ++++\n 6 files changed, 69 insertions(+), 4 deletions(-)\n create mode 100644 thread-utils.c\n create mode 100644 thread-utils.h\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7b67671..62b697c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -808,6 +808,8 @@ pack.threads::\n \twarning. This is meant to reduce packing time on multiprocessor\n \tmachines. The required amount of memory for the delta search window\n \tis however multiplied by the number of threads.\n+\tSpecifying 0 will cause git to auto-detect the number of CPU's\n+\tand set the number of threads accordingly.\n \n pack.indexVersion::\n \tSpecify the default pack index version. Valid values are 1 for\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 8353be1..5c1bd3b 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -177,6 +177,8 @@ base-name::\n \tThis is meant to reduce packing time on multiprocessor machines.\n \tThe required amount of memory for the delta search window is\n \thowever multiplied by the number of threads.\n+\tSpecifying 0 will cause git to auto-detect the number of CPU's\n+\tand set the number of threads accordingly.\n \n --index-version=[,]::\n \tThis is intended to be used by the test suite only. It allows\ndiff --git a/Makefile b/Makefile\nindex d33a556..2dc8247 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -741,6 +741,7 @@ endif\n ifdef THREADED_DELTA_SEARCH\n \tBASIC_CFLAGS += -DTHREADED_DELTA_SEARCH\n \tEXTLIBS += -lpthread\n+\tLIB_OBJS += thread-utils.o\n endif\n \n ifeq ($(TCLTK_PATH),)\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex d2bb12e..586ae11 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -16,6 +16,7 @@\n #include \"progress.h\"\n \n #ifdef THREADED_DELTA_SEARCH\n+#include \"thread-utils.h\"\n #include \n #endif\n \n@@ -1852,11 +1853,11 @@ static int git_pack_config(const char *k, const char *v)\n \t}\n \tif (!strcmp(k, \"pack.threads\")) {\n \t\tdelta_search_threads = git_config_int(k, v);\n-\t\tif (delta_search_threads < 1)\n+\t\tif (delta_search_threads < 0)\n \t\t\tdie(\"invalid number of threads specified (%d)\",\n \t\t\t delta_search_threads);\n #ifndef THREADED_DELTA_SEARCH\n-\t\tif (delta_search_threads > 1)\n+\t\tif (delta_search_threads != 1)\n \t\t\twarning(\"no threads support, ignoring %s\", k);\n #endif\n \t\treturn 0;\n@@ -2122,10 +2123,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tif (!prefixcmp(arg, \"--threads=\")) {\n \t\t\tchar *end;\n \t\t\tdelta_search_threads = strtoul(arg+10, &end, 0);\n-\t\t\tif (!arg[10] || *end || delta_search_threads < 1)\n+\t\t\tif (!arg[10] || *end || delta_search_threads < 0)\n \t\t\t\tusage(pack_usage);\n #ifndef THREADED_DELTA_SEARCH\n-\t\t\tif (delta_search_threads > 1)\n+\t\t\tif (delta_search_threads != 1)\n \t\t\t\twarning(\"no threads support, \"\n \t\t\t\t\t\"ignoring %s\", arg);\n #endif\n@@ -2235,6 +2236,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (!pack_to_stdout && thin)\n \t\tdie(\"--thin cannot be used to build an indexable pack.\");\n \n+#ifdef THREADED_DELTA_SEARCH\n+\tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n+\t\tdelta_search_threads = online_cpus();\n+#endif\n+\n \tprepare_packed_git();\n \n \tif (progress)\ndiff --git a/thread-utils.c b/thread-utils.c\nnew file mode 100644\nindex 0000000..55e7e29\n--- /dev/null\n+++ b/thread-utils.c\n@@ -0,0 +1,48 @@\n+#include \"cache.h\"\n+\n+#ifdef _WIN32\n+# define WIN32_LEAN_AND_MEAN\n+# include \n+#elif defined(hpux) || defined(__hpux) || defined(_hpux)\n+# include \n+#endif\n+\n+/*\n+ * By doing this in two steps we can at least get\n+ * the function to be somewhat coherent, even\n+ * with this disgusting nest of #ifdefs.\n+ */\n+#ifndef _SC_NPROCESSORS_ONLN\n+# ifdef _SC_NPROC_ONLN\n+# define _SC_NPROCESSORS_ONLN _SC_NPROC_ONLN\n+# elif defined _SC_CRAY_NCPU\n+# define _SC_NPROCESSORS_ONLN _SC_CRAY_NCPU\n+# endif\n+#endif\n+\n+int online_cpus(void)\n+{\n+#ifdef _SC_NPROCESSORS_ONLN\n+\tlong ncpus;\n+#endif\n+\n+#ifdef _WIN32\n+\tSYSTEM_INFO info;\n+\tGetSystemInfo(&info);\n+\n+\tif ((int)info.dwNumberOfProcessors > 0)\n+\t\treturn (int)info.dwNumberOfProcessors;\n+#elif defined(hpux) || defined(__hpux) || defined(_hpux)\n+\tstruct pst_dynamic psd;\n+\n+\tif (!pstat_getdynamic(&psd, sizeof(psd), (size_t)1, 0))\n+\t\treturn (int)psd.psd_proc_cnt;\n+#endif\n+\n+#ifdef _SC_NPROCESSORS_ONLN\n+\tif ((ncpus = (long)sysconf(_SC_NPROCESSORS_ONLN)) > 0)\n+\t\treturn (int)ncpus;\n+#endif\n+\n+\treturn 1;\n+}\ndiff --git a/thread-utils.h b/thread-utils.h\nnew file mode 100644\nindex 0000000..cce4b77\n--- /dev/null\n+++ b/thread-utils.h\n@@ -0,0 +1,6 @@\n+#ifndef THREAD_COMPAT_H\n+#define THREAD_COMPAT_H\n+\n+extern int online_cpus(void);\n+\n+#endif /* THREAD_COMPAT_H */\n-- \n1.5.4.2.199.g0941\n"},{"id":"69643","messageId":"47BF812A.4020205@nrlssc.navy.mil","threadId":"12053","inReplyTo":"1203732369-30314-1-git-send-email-casey@nrlssc.navy.mil","subject":"[PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-02-23T02:12:58Z","receivedAt":"2008-02-23T02:12:58Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Signed-off-by: Brandon Casey \n---\n builtin-pack-objects.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 586ae11..5a22f49 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -2239,6 +2239,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n #ifdef THREADED_DELTA_SEARCH\n \tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n \t\tdelta_search_threads = online_cpus();\n+\tif (progress)\n+\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n+\t\t\tdelta_search_threads);\n #endif\n \n \tprepare_packed_git();\n-- \n1.5.4.2.199.g0941\n"},{"id":"69661","messageId":"47BFD6CF.3040300@op5.se","threadId":"12053","inReplyTo":"47BF80EC.4080608@nrlssc.navy.mil","subject":"Re: [PATCH] pack-objects: Add runtime detection of online CPU's","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-02-23T08:18:23Z","receivedAt":"2008-02-23T08:18:23Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Brandon Casey wrote:\n> From: Andreas Ericsson \n> \n> Packing objects can be done in parallell nowadays, but it's\n> only done if the config option pack.threads is set to a value\n> above 1. Because of that, the code-path used is often not the\n> most optimal one.\n> \n> This patch adds a routine to detect the number of online CPU's\n> at runtime (online_cpus()). When pack.threads (or --threads=) is\n> given a value of 0, the number of threads is set to the number of\n> online CPU's. This feature is also documented.\n> \n> As per Nicolas Pitre's recommendations, the default is still to\n> run pack-objects single-threaded unless explicitly activated,\n> either by configuration or by command line parameter.\n> \n> The routine online_cpus() is a rework of \"numcpus.c\", written by\n> one Philip Willoughby . numcpus.c is in the\n> public domain and can presently be downloaded from\n> http://csgsoft.doc.ic.ac.uk/numcpus/\n> \n> Signed-off-by: Andreas Ericsson \n> Signed-off-by: Brandon Casey \n> ---\n> \n> \n> I reworked this patch from Andreas for detecting the number of online CPU's.\n> I kept the commit message and the Signed-off-by and added my own. I'm not sure\n> what the procedure is here.\n> \n\nThe changes are small enough that maintaining original authorship is probably\nthe right thing to do. For anything larger it would probably have made sense\nto send something on top of it. For a rewrite or when implementing a feature\nthat was thought up by someone else, mentioning in the message who the original\nidea was from is enough.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"69963","messageId":"20080226074933.GA3485@coredump.intra.peff.net","threadId":"12053","inReplyTo":"47BF812A.4020205@nrlssc.navy.mil","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-26T07:49:33Z","receivedAt":"2008-02-26T07:49:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2008 at 08:12:58PM -0600, Brandon Casey wrote:\n\n> +\tif (progress)\n> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n> +\t\t\tdelta_search_threads);\n\nI just noticed that this was in next. Do we really need to display this\nmessage? A considerable amount of discussion went into reducing git's\nchattiness and clutter during push and fetch, and I feel like this is a\nstep backwards (yes, I know most people won't see it if they don't build\nwith THREADED_DELTA_SEARCH).\n\nCan we show it only if threads != 1? Only if we auto-detected the number\nof threads and it wasn't 1?\n\n-Peff\n"},{"id":"69964","messageId":"7vhcfwb116.fsf@gitster.siamese.dyndns.org","threadId":"12053","inReplyTo":"20080226074933.GA3485@coredump.intra.peff.net","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-26T08:00:05Z","receivedAt":"2008-02-26T08:00:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Feb 22, 2008 at 08:12:58PM -0600, Brandon Casey wrote:\n>\n>> +\tif (progress)\n>> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n>> +\t\t\tdelta_search_threads);\n>\n> I just noticed that this was in next.\n\nPlease send in a fix-up patch to remove it. I noticed it while\nreviewing the patch, and even commented on it, but I somehow\nforgot that this leftover debugging message disqualified the\nseries from 'next' when I was merging topics to 'next'.\n"},{"id":"69965","messageId":"20080226080634.GA4129@coredump.intra.peff.net","threadId":"12053","inReplyTo":"7vhcfwb116.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-26T08:06:34Z","receivedAt":"2008-02-26T08:06:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2008 at 12:00:05AM -0800, Junio C Hamano wrote:\n\n> >> +\tif (progress)\n> >> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n> >> +\t\t\tdelta_search_threads);\n> >\n> > I just noticed that this was in next.\n> \n> Please send in a fix-up patch to remove it. I noticed it while\n> reviewing the patch, and even commented on it, but I somehow\n> forgot that this leftover debugging message disqualified the\n> series from 'next' when I was merging topics to 'next'.\n\nAre you sure you are thinking of the same message? This one was\nsubmitted in a patch by itself, and I didn't see any followup\ndiscussion. It's in next as:\n\n 6c723f5 pack-objects: Print a message describing the number of\n threads for packing\n\n-Peff\n"},{"id":"69967","messageId":"7vablo848d.fsf@gitster.siamese.dyndns.org","threadId":"12053","inReplyTo":"20080226080634.GA4129@coredump.intra.peff.net","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-26T09:19:14Z","receivedAt":"2008-02-26T09:19:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Feb 26, 2008 at 12:00:05AM -0800, Junio C Hamano wrote:\n>\n>> >> +\tif (progress)\n>> >> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n>> >> +\t\t\tdelta_search_threads);\n>> >\n>> > I just noticed that this was in next.\n>> \n>> Please send in a fix-up patch to remove it. I noticed it while\n>> reviewing the patch, and even commented on it, but I somehow\n>> forgot that this leftover debugging message disqualified the\n>> series from 'next' when I was merging topics to 'next'.\n>\n> Are you sure you are thinking of the same message?\n\nAh, no.\n\nBut now you mention it, I tend to agree with you. This is\nprimarily of interest for git developers and I do not think the\nend users would care. Maybe under --verbose or --debug option\n(but I do not think we have --debug option anywhere).\n"},{"id":"69972","messageId":"20080226093300.GA5812@coredump.intra.peff.net","threadId":"12053","inReplyTo":"7vablo848d.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-26T09:33:01Z","receivedAt":"2008-02-26T09:33:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2008 at 01:19:14AM -0800, Junio C Hamano wrote:\n\n> But now you mention it, I tend to agree with you. This is\n> primarily of interest for git developers and I do not think the\n> end users would care. Maybe under --verbose or --debug option\n> (but I do not think we have --debug option anywhere).\n\nI wrote up a --verbose patch, but it just seemed silly. Who would\nactually turn it on?\n\nHow about this instead?\n\n-- >8 --\npack-objects: show \"using N threads\" only when autodetected\n\nEvery other case is uninteresting, since either:\n - it is the default of 1, in which case we are always just\n printing \"using 1 thread\"\n - it is whatever the user set it to, in which case they\n already know\n\nBut with --threads=0, they might want to be informed of the\nnumber of CPUs detected.\n---\nIf we ever change the default to autodetect, this logic might change,\nbut we can deal with that then.\n\nBTW, I seem to remember some work recently on coalescing hunks in merge\nconflicts separated by a small number of lines. It seems to me that the\ndiff below would be easier to read with a similar tactic.\n\n builtin-pack-objects.c | 9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex b70b2e5..516eb24 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -2236,11 +2236,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tdie(\"--thin cannot be used to build an indexable pack.\");\n \n #ifdef THREADED_DELTA_SEARCH\n-\tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n+\tif (!delta_search_threads) {\t/* --threads=0 means autodetect */\n \t\tdelta_search_threads = online_cpus();\n-\tif (progress)\n-\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n-\t\t\tdelta_search_threads);\n+\t\tif (progress)\n+\t\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n+\t\t\t\t\tdelta_search_threads);\n+\t}\n #endif\n \n \tprepare_packed_git();\n-- \n1.5.4.3.340.gda2e.dirty\n"},{"id":"69973","messageId":"47C3DE73.2030507@op5.se","threadId":"12053","inReplyTo":"7vhcfwb116.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2008-02-26T09:40:03Z","receivedAt":"2008-02-26T09:40:03Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> On Fri, Feb 22, 2008 at 08:12:58PM -0600, Brandon Casey wrote:\n>>\n>>> +\tif (progress)\n>>> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n>>> +\t\t\tdelta_search_threads);\n>> I just noticed that this was in next.\n> \n> Please send in a fix-up patch to remove it. I noticed it while\n> reviewing the patch, and even commented on it, but I somehow\n> forgot that this leftover debugging message disqualified the\n> series from 'next' when I was merging topics to 'next'.\n\nFWIW, it wasn't in the original patch I sent in, but only in\nthe one sent by Brandon Casey. I believe that may have added\nto the confusion.\n\nI like Jeff's suggestion of only showing it when we autodetect\nthough, but I won't have time to send a patch until this weekend\nat the earliest.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"70017","messageId":"47C435DC.2070508@nrlssc.navy.mil","threadId":"12053","inReplyTo":"20080226074933.GA3485@coredump.intra.peff.net","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-02-26T15:53:00Z","receivedAt":"2008-02-26T15:53:00Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Jeff King wrote:\n> On Fri, Feb 22, 2008 at 08:12:58PM -0600, Brandon Casey wrote:\n> \n>> +\tif (progress)\n>> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n>> +\t\t\tdelta_search_threads);\n> \n> I just noticed that this was in next. Do we really need to display this\n> message? A considerable amount of discussion went into reducing git's\n> chattiness and clutter during push and fetch, and I feel like this is a\n> step backwards (yes, I know most people won't see it if they don't build\n> with THREADED_DELTA_SEARCH).\n> \n> Can we show it only if threads != 1? Only if we auto-detected the number\n> of threads and it wasn't 1?\n\nI like the message and thought it was useful especially for non-developers.\n\nEven if the number of threads was not auto-detected, it is a confirmation\nthat the number of threads used is the number of threads configured.\n\nFor example, it seems easy to do this:\n\n\tgit config pack.thread 4\n\tgit repack\n\nThe user would immediately know something was wrong when they saw the\nmessage \"Using 1 pack threads\" instead of the \"4\" they thought they\nconfigured. Also, since it's only printed in the THREADED_DELTA_SEARCH\ncase, it's also a confirmation that this option was indeed used for a\nparticular build of git.\n\nMainly, I thought it was a harmless message that other users would \"enjoy\"\nseeing, but if others disagree, I won't argue. Notice I quoted \"enjoy\" to\nemphasize it.\n\nI'd also say that if the message is too noisy in the \"user explicitly\nassigned number of threads\" case, then it's just as noisy in the \"auto assign\"\ncase, so just remove the message completely.\n\nWe're saying:\n\nIf I set pack.threads to 4, I know git is using 4 threads to repack since\nI told it to use 4 threads. I don't need to see a noisy message telling\nme so.\n\nIf I set pack.threads to 0, I know git is using 4 threads to repack since\nI have 4 cpus. I don't need to see a noisy message telling me so.\n\n-brandon\n"},{"id":"70008","messageId":"alpine.LFD.1.00.0802261149220.3167@xanadu.home","threadId":"12053","inReplyTo":"47C435DC.2070508@nrlssc.navy.mil","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-26T17:05:46Z","receivedAt":"2008-02-26T17:05:46Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 26 Feb 2008, Brandon Casey wrote:\n\n> Jeff King wrote:\n> > On Fri, Feb 22, 2008 at 08:12:58PM -0600, Brandon Casey wrote:\n> > \n> >> +\tif (progress)\n> >> +\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n> >> +\t\t\tdelta_search_threads);\n> > \n> > I just noticed that this was in next. Do we really need to display this\n> > message? A considerable amount of discussion went into reducing git's\n> > chattiness and clutter during push and fetch, and I feel like this is a\n> > step backwards (yes, I know most people won't see it if they don't build\n> > with THREADED_DELTA_SEARCH).\n> > \n> > Can we show it only if threads != 1? Only if we auto-detected the number\n> > of threads and it wasn't 1?\n> \n> I like the message and thought it was useful especially for non-developers.\n> \n> Even if the number of threads was not auto-detected, it is a confirmation\n> that the number of threads used is the number of threads configured.\n> \n> For example, it seems easy to do this:\n> \n> \tgit config pack.thread 4\n> \tgit repack\n> \n> The user would immediately know something was wrong when they saw the\n> message \"Using 1 pack threads\" instead of the \"4\" they thought they\n> configured.\n\nMaybe a message for any unrecognized config option should be displayed \ninstead.\n\n> Also, since it's only printed in the THREADED_DELTA_SEARCH\n> case, it's also a confirmation that this option was indeed used for a\n> particular build of git.\n> \n> Mainly, I thought it was a harmless message that other users would \"enjoy\"\n> seeing, but if others disagree, I won't argue. Notice I quoted \"enjoy\" to\n> emphasize it.\n\nThis is enjoyable maybe the first time, but that might get \nuseless/annoying after a while. I think that displaying it in the \nautodetection case is a good compromize, and then simply specifying the \nnumber of threads explicitly will silence it.\n\nAlso, I think that such message should absolutely not be sent over in \nthe context of a fetch/clone. This is a local matter only, and should \nbe displayed only when those progress messages are meant for the local \nuser.\n\nTherefore I propose this patch instead:\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex b70b2e5..6dcb4e2 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -2236,11 +2236,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tdie(\"--thin cannot be used to build an indexable pack.\");\n \n #ifdef THREADED_DELTA_SEARCH\n-\tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n+\tif (!delta_search_threads) {\t/* --threads=0 means autodetect */\n \t\tdelta_search_threads = online_cpus();\n-\tif (progress)\n-\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n-\t\t\tdelta_search_threads);\n+\t\tif (progress > pack_to_stdout)\n+\t\t\tfprintf(stderr, \"Using %d pack threads.\\n\",\n+\t\t\t\tdelta_search_threads);\n+\t}\n #endif\n \n \tprepare_packed_git();\n"},{"id":"70030","messageId":"20080226212118.GA32530@sigill.intra.peff.net","threadId":"12053","inReplyTo":"47C435DC.2070508@nrlssc.navy.mil","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-26T21:21:18Z","receivedAt":"2008-02-26T21:21:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2008 at 09:53:00AM -0600, Brandon Casey wrote:\n\n> \tgit config pack.thread 4\n> \tgit repack\n> \n> The user would immediately know something was wrong when they saw the\n> message \"Using 1 pack threads\" instead of the \"4\" they thought they\n\nThere are hundreds of ways the user can fail to configure git correctly;\nI don't think it's worth printing output so verbose that the user can\nmanually check that every config option was respected.\n\nAt any rate, I think your reasoning is not a good guideline for user\noutput. You are making output to notice a mistake that happens one time\n(the time of config), but you are showing the output to the reader many\ntimes (every time they repack from here to eternity). But there are also\nmistakes that could be made in the \"many times\" case, and you are taking\ntheir attention away from that.\n\nIn the case of repack, it is probably not a big deal. But in the case of\n'push', for example, I think we want as little output as possible taking\nattention away from the useful information: which refs were pushed,\nwhich were rejected, and so forth. That's why Nicolas made the\npack-objects output considerably more terse last November.\n\n> configured. Also, since it's only printed in the THREADED_DELTA_SEARCH\n> case, it's also a confirmation that this option was indeed used for a\n> particular build of git.\n\nSame reasoning as above. You configure THREADED_DELTA_SEARCH once; you\ndon't need to check that it was enabled every time you repack.\n\n> I'd also say that if the message is too noisy in the \"user explicitly\n> assigned number of threads\" case, then it's just as noisy in the \"auto assign\"\n> case, so just remove the message completely.\n\nI am not opposed to that; the \"auto assign\" case is nice to see the\nfirst time you repack (\"did it find all of my CPUs?\"), but yes, it\nprobably will be the same every time after.\n\n-Peff\n"},{"id":"70032","messageId":"20080226212516.GB32530@sigill.intra.peff.net","threadId":"12053","inReplyTo":"alpine.LFD.1.00.0802261149220.3167@xanadu.home","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-26T21:25:16Z","receivedAt":"2008-02-26T21:25:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2008 at 12:05:46PM -0500, Nicolas Pitre wrote:\n\n> > For example, it seems easy to do this:\n> > \n> > \tgit config pack.thread 4\n> > \tgit repack\n> > \n> > The user would immediately know something was wrong when they saw the\n> > message \"Using 1 pack threads\" instead of the \"4\" they thought they\n> > configured.\n> \n> Maybe a message for any unrecognized config option should be displayed \n> instead.\n\nI think that is generally useful, though it is somewhat hard with our\nconfig parsing mechanism. You could handle specific cases like \"I'm in\ngit_pack_config, this is a pack.* variable, and I don't understand it\".\nBut it would be nice to have a general \"no callback claimed to\nunderstand this variable\" which I think is impossible (since we do\nthings like parsing the config just to grab a small part of it).\n\n> Also, I think that such message should absolutely not be sent over in \n> the context of a fetch/clone. This is a local matter only, and should \n> be displayed only when those progress messages are meant for the local \n> user.\n> \n> Therefore I propose this patch instead:\n\nI think that is a nice addition to my patch, but I am also fine with\nsimply reverting 6c723f5e entirely, as Brandon suggested.\n\n-Peff\n"},{"id":"70044","messageId":"47C497BF.8080900@nrlssc.navy.mil","threadId":"12053","inReplyTo":"20080226212118.GA32530@sigill.intra.peff.net","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-02-26T22:50:39Z","receivedAt":"2008-02-26T22:50:39Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Jeff King wrote:\n\n> I don't think it's worth printing output so verbose that the user can\n> manually check that every config option was respected.\n\nIt's hard coming up with examples that someone cannot take to the N'th\ndegree and make look ridiculous. Maybe impossible.\n\n> At any rate, I think your reasoning is not a good guideline for user\n> output.\n\nMy reasoning was that I liked it. I thought it was interesting and useful\nto show the number of threads that pack-objects was using. The example I\ngave was to show how it could be useful to non-developers since Junio\nsuggested that the information was primarily of interest for git developers\nand he didn't think that end users would care.\n\nIn any case, I'm not attached to the patch. I am thankful for Nicolas's\npatch though, since it forced me to learn the relationship between progress\nand pack_to_stdout.\n\n-brandon\n"},{"id":"70043","messageId":"20080226230452.GA6721@sigill.intra.peff.net","threadId":"12053","inReplyTo":"47C497BF.8080900@nrlssc.navy.mil","subject":"Re: [PATCH] pack-objects: Print a message describing the number of threads for packing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-02-26T23:04:52Z","receivedAt":"2008-02-26T23:04:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2008 at 04:50:39PM -0600, Brandon Casey wrote:\n\n> > I don't think it's worth printing output so verbose that the user can\n> > manually check that every config option was respected.\n> \n> It's hard coming up with examples that someone cannot take to the N'th\n> degree and make look ridiculous. Maybe impossible.\n\nI know. I didn't mean to say \"this message is ridiculous.\" I meant to\nsay \"we should strive for consistency in interface, and I don't see\nanything that makes this config option any different than, say,\ncore.followSymlinks.\"\n\n-Peff\n"}]}