{"thread":{"id":"21472","subject":"[PATCH 0/1] Port of pthreads to Windows API threads","startedAt":"2009-11-03T21:30:01Z","lastAt":"2009-11-06T08:25:33Z","messageCount":62,"participants":["Andrzej K. Haczewski","Johannes Schindelin","Joshua Jensen","Johannes Sixt","Michael Wookey","Erik Faye-Lund","Paolo Bonzini","Nicolas Pitre","Daniel Barkalow","Junio C Hamano","Dmitry Potapov"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"126677","messageId":"1257283802-29726-1-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":null,"subject":"[PATCH 0/1] Port of pthreads to Windows API threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-03T21:30:01Z","receivedAt":"2009-11-03T21:30:01Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"Here is a small patch that allows use of native Windows API threads for\nwhere git uses pthreads. I don't have full msysgit environment to test it,\nall I know is it compiles, so someone else could test it a little. I enabled\nit for MSVC only (since msysgit uses win32-pthreads and cygwin has it's own).\nI assume that patch could help with getting rid of that one dependency for\nmsysgit.\n\nPS. I'm new here (actually new for whole kernel-like-development-cycle-with-\npatches-flying-low-on-mailing-lists), so please be kind to my stupidity for\na while... please? ;-)\n\nAndrzej K. Haczewski (1):\n MSVC: port pthread code to native Windows threads\n\n Makefile | 2 +-\n builtin-pack-objects.c | 42 +++++++++-\n compat/winthread.h | 219 ++++++++++++++++++++++++++++++++++++++++++++++++\n preload-index.c | 12 +++\n 4 files changed, 272 insertions(+), 3 deletions(-)\n create mode 100644 compat/winthread.h\n"},{"id":"126678","messageId":"1257283802-29726-2-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":"1257283802-29726-1-git-send-email-ahaczewski@gmail.com","subject":"[PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-03T21:30:02Z","receivedAt":"2009-11-03T21:30:02Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"---\n Makefile | 2 +-\n builtin-pack-objects.c | 42 +++++++++-\n compat/winthread.h | 219 ++++++++++++++++++++++++++++++++++++++++++++++++\n preload-index.c | 12 +++\n 4 files changed, 272 insertions(+), 3 deletions(-)\n create mode 100644 compat/winthread.h\n\ndiff --git a/Makefile b/Makefile\nindex 28d6ecf..126ab43 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -940,7 +940,7 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..a8a4f59 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -18,8 +18,12 @@\n #include \"refs.h\"\n \n #ifdef THREADED_DELTA_SEARCH\n-#include \"thread-utils.h\"\n-#include \n+# include \"thread-utils.h\"\n+# ifndef _WIN32\n+# include \n+# else\n+# include \n+# endif\n #endif\n \n static const char pack_usage[] =\n@@ -1592,7 +1596,11 @@ struct thread_params {\n \n static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n \n+#ifndef _WIN32\n static void *threaded_find_deltas(void *arg)\n+#else\n+static DWORD WINAPI threaded_find_deltas(LPVOID arg)\n+#endif\n {\n \tstruct thread_params *me = arg;\n \n@@ -1620,7 +1628,11 @@ static void *threaded_find_deltas(void *arg)\n \t\tpthread_mutex_unlock(&me->mutex);\n \t}\n \t/* leave ->working 1 so that this doesn't get more work assigned */\n+#ifndef _WIN32\n \treturn NULL;\n+#else\n+\treturn 0;\n+#endif\n }\n \n static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n@@ -2327,6 +2339,18 @@ 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+\n+#ifdef _WIN32\n+\t/*\n+\t * Windows require initialization of mutex (CRITICAL_SECTION)\n+\t * and conditional variable.\n+\t */\n+\tpthread_mutex_init(&read_mutex);\n+\tpthread_mutex_init(&cache_mutex);\n+\tpthread_mutex_init(&progress_mutex);\n+\twin32_cond_init(&progress_cond);\n+#endif\n+\n #endif\n \n \tprepare_packed_git();\n@@ -2345,7 +2369,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tstop_progress(&progress_state);\n \n \tif (non_empty && !nr_result)\n+#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)\n+\t\tgoto cleanup;\n+#else\n \t\treturn 0;\n+#endif\n \tif (nr_result)\n \t\tprepare_pack(window, depth);\n \twrite_pack_file();\n@@ -2353,5 +2381,15 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tfprintf(stderr, \"Total %\"PRIu32\" (delta %\"PRIu32\"),\"\n \t\t\t\" reused %\"PRIu32\" (delta %\"PRIu32\")\\n\",\n \t\t\twritten, written_delta, reused, reused_delta);\n+\n+#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)\n+cleanup:\n+\t/* cleanup Windows threads thingies */\n+\twin32_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+#endif\n+\n \treturn 0;\n }\ndiff --git a/compat/winthread.h b/compat/winthread.h\nnew file mode 100644\nindex 0000000..32c9010\n--- /dev/null\n+++ b/compat/winthread.h\n@@ -0,0 +1,219 @@\n+/*\n+ * Header used to \"adapt\" pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef WINTHREAD_H\n+#define WINTHREAD_H\n+\n+#define WIN32_LEAN_AND_MEAN\n+#include \n+\n+/* Implement simple condition variable for Windows threads, based on ACE implementation */\n+typedef struct win32_cond {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} win32_cond_t;\n+\n+#define PTHREAD_COND_INITIALIZER { 0, { 0 }, NULL }\n+\n+static __inline int win32_cond_init(win32_cond_t *cond)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (NULL == cond->sema)\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static __inline int win32_cond_destroy(win32_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+static __inline int win32_cond_wait(win32_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\tDWORD result;\n+\tint ret = 0;\n+\n+\t/* we're waiting... */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* unlock external mutex and wait for signal */\n+\tLeaveCriticalSection(mutex);\n+\tresult = WaitForSingleObject(cond->sema, INFINITE);\n+\n+\tif (0 != result)\n+\t\tret = -1;\n+\n+\t/* one waiter less */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn ret;\n+}\n+\n+static __inline int win32_cond_signal(win32_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;\n+\telse\n+\t\treturn 0;\n+}\n+\n+#define pthread_t HANDLE\n+#define pthread_mutex_t CRITICAL_SECTION\n+#define pthread_cond_t win32_cond_t\n+\n+#define PTHREAD_MUTEX_INITIALIZER { 0 }\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+#define pthread_cond_init(a,b) win32_cond_init((a))\n+#define pthread_cond_destroy win32_cond_destroy\n+#define pthread_cond_wait(a,b) win32_cond_wait((a), (b))\n+#define pthread_cond_signal win32_cond_signal\n+\n+#define pthread_create(a,b,c,d) ((NULL == (*(a) = CreateThread(NULL, 0, (c), (d), 0, NULL))) ? (errno = err_win_to_posix(), -1) : (errno = 0, 0))\n+#define pthread_join(a,b) ((WAIT_OBJECT_0 == WaitForSingleObject((a), INFINITE)) ? 0 : -1)\n+\n+/* almost copy-paste code of mingw.c */\n+static int err_win_to_posix()\n+{\n+\tint error = ENOSYS;\n+\tswitch(GetLastError()) {\n+\tcase ERROR_ACCESS_DENIED: error = EACCES; break;\n+\tcase ERROR_ACCOUNT_DISABLED: error = EACCES; break;\n+\tcase ERROR_ACCOUNT_RESTRICTION: error = EACCES; break;\n+\tcase ERROR_ALREADY_ASSIGNED: error = EBUSY; break;\n+\tcase ERROR_ALREADY_EXISTS: error = EEXIST; break;\n+\tcase ERROR_ARITHMETIC_OVERFLOW: error = ERANGE; break;\n+\tcase ERROR_BAD_COMMAND: error = EIO; break;\n+\tcase ERROR_BAD_DEVICE: error = ENODEV; break;\n+\tcase ERROR_BAD_DRIVER_LEVEL: error = ENXIO; break;\n+\tcase ERROR_BAD_EXE_FORMAT: error = ENOEXEC; break;\n+\tcase ERROR_BAD_FORMAT: error = ENOEXEC; break;\n+\tcase ERROR_BAD_LENGTH: error = EINVAL; break;\n+\tcase ERROR_BAD_PATHNAME: error = ENOENT; break;\n+\tcase ERROR_BAD_PIPE: error = EPIPE; break;\n+\tcase ERROR_BAD_UNIT: error = ENODEV; break;\n+\tcase ERROR_BAD_USERNAME: error = EINVAL; break;\n+\tcase ERROR_BROKEN_PIPE: error = EPIPE; break;\n+\tcase ERROR_BUFFER_OVERFLOW: error = ENAMETOOLONG; break;\n+\tcase ERROR_BUSY: error = EBUSY; break;\n+\tcase ERROR_BUSY_DRIVE: error = EBUSY; break;\n+\tcase ERROR_CALL_NOT_IMPLEMENTED: error = ENOSYS; break;\n+\tcase ERROR_CANNOT_MAKE: error = EACCES; break;\n+\tcase ERROR_CANTOPEN: error = EIO; break;\n+\tcase ERROR_CANTREAD: error = EIO; break;\n+\tcase ERROR_CANTWRITE: error = EIO; break;\n+\tcase ERROR_CRC: error = EIO; break;\n+\tcase ERROR_CURRENT_DIRECTORY: error = EACCES; break;\n+\tcase ERROR_DEVICE_IN_USE: error = EBUSY; break;\n+\tcase ERROR_DEV_NOT_EXIST: error = ENODEV; break;\n+\tcase ERROR_DIRECTORY: error = EINVAL; break;\n+\tcase ERROR_DIR_NOT_EMPTY: error = ENOTEMPTY; break;\n+\tcase ERROR_DISK_CHANGE: error = EIO; break;\n+\tcase ERROR_DISK_FULL: error = ENOSPC; break;\n+\tcase ERROR_DRIVE_LOCKED: error = EBUSY; break;\n+\tcase ERROR_ENVVAR_NOT_FOUND: error = EINVAL; break;\n+\tcase ERROR_EXE_MARKED_INVALID: error = ENOEXEC; break;\n+\tcase ERROR_FILENAME_EXCED_RANGE: error = ENAMETOOLONG; break;\n+\tcase ERROR_FILE_EXISTS: error = EEXIST; break;\n+\tcase ERROR_FILE_INVALID: error = ENODEV; break;\n+\tcase ERROR_FILE_NOT_FOUND: error = ENOENT; break;\n+\tcase ERROR_GEN_FAILURE: error = EIO; break;\n+\tcase ERROR_HANDLE_DISK_FULL: error = ENOSPC; break;\n+\tcase ERROR_INSUFFICIENT_BUFFER: error = ENOMEM; break;\n+\tcase ERROR_INVALID_ACCESS: error = EACCES; break;\n+\tcase ERROR_INVALID_ADDRESS: error = EFAULT; break;\n+\tcase ERROR_INVALID_BLOCK: error = EFAULT; break;\n+\tcase ERROR_INVALID_DATA: error = EINVAL; break;\n+\tcase ERROR_INVALID_DRIVE: error = ENODEV; break;\n+\tcase ERROR_INVALID_EXE_SIGNATURE: error = ENOEXEC; break;\n+\tcase ERROR_INVALID_FLAGS: error = EINVAL; break;\n+\tcase ERROR_INVALID_FUNCTION: error = ENOSYS; break;\n+\tcase ERROR_INVALID_HANDLE: error = EBADF; break;\n+\tcase ERROR_INVALID_LOGON_HOURS: error = EACCES; break;\n+\tcase ERROR_INVALID_NAME: error = EINVAL; break;\n+\tcase ERROR_INVALID_OWNER: error = EINVAL; break;\n+\tcase ERROR_INVALID_PARAMETER: error = EINVAL; break;\n+\tcase ERROR_INVALID_PASSWORD: error = EPERM; break;\n+\tcase ERROR_INVALID_PRIMARY_GROUP: error = EINVAL; break;\n+\tcase ERROR_INVALID_SIGNAL_NUMBER: error = EINVAL; break;\n+\tcase ERROR_INVALID_TARGET_HANDLE: error = EIO; break;\n+\tcase ERROR_INVALID_WORKSTATION: error = EACCES; break;\n+\tcase ERROR_IO_DEVICE: error = EIO; break;\n+\tcase ERROR_IO_INCOMPLETE: error = EINTR; break;\n+\tcase ERROR_LOCKED: error = EBUSY; break;\n+\tcase ERROR_LOCK_VIOLATION: error = EACCES; break;\n+\tcase ERROR_LOGON_FAILURE: error = EACCES; break;\n+\tcase ERROR_MAPPED_ALIGNMENT: error = EINVAL; break;\n+\tcase ERROR_META_EXPANSION_TOO_LONG: error = E2BIG; break;\n+\tcase ERROR_MORE_DATA: error = EPIPE; break;\n+\tcase ERROR_NEGATIVE_SEEK: error = ESPIPE; break;\n+\tcase ERROR_NOACCESS: error = EFAULT; break;\n+\tcase ERROR_NONE_MAPPED: error = EINVAL; break;\n+\tcase ERROR_NOT_ENOUGH_MEMORY: error = ENOMEM; break;\n+\tcase ERROR_NOT_READY: error = EAGAIN; break;\n+\tcase ERROR_NOT_SAME_DEVICE: error = EXDEV; break;\n+\tcase ERROR_NO_DATA: error = EPIPE; break;\n+\tcase ERROR_NO_MORE_SEARCH_HANDLES: error = EIO; break;\n+\tcase ERROR_NO_PROC_SLOTS: error = EAGAIN; break;\n+\tcase ERROR_NO_SUCH_PRIVILEGE: error = EACCES; break;\n+\tcase ERROR_OPEN_FAILED: error = EIO; break;\n+\tcase ERROR_OPEN_FILES: error = EBUSY; break;\n+\tcase ERROR_OPERATION_ABORTED: error = EINTR; break;\n+\tcase ERROR_OUTOFMEMORY: error = ENOMEM; break;\n+\tcase ERROR_PASSWORD_EXPIRED: error = EACCES; break;\n+\tcase ERROR_PATH_BUSY: error = EBUSY; break;\n+\tcase ERROR_PATH_NOT_FOUND: error = ENOENT; break;\n+\tcase ERROR_PIPE_BUSY: error = EBUSY; break;\n+\tcase ERROR_PIPE_CONNECTED: error = EPIPE; break;\n+\tcase ERROR_PIPE_LISTENING: error = EPIPE; break;\n+\tcase ERROR_PIPE_NOT_CONNECTED: error = EPIPE; break;\n+\tcase ERROR_PRIVILEGE_NOT_HELD: error = EACCES; break;\n+\tcase ERROR_READ_FAULT: error = EIO; break;\n+\tcase ERROR_SEEK: error = EIO; break;\n+\tcase ERROR_SEEK_ON_DEVICE: error = ESPIPE; break;\n+\tcase ERROR_SHARING_BUFFER_EXCEEDED: error = ENFILE; break;\n+\tcase ERROR_SHARING_VIOLATION: error = EACCES; break;\n+\tcase ERROR_STACK_OVERFLOW: error = ENOMEM; break;\n+\tcase ERROR_SWAPERROR: error = ENOENT; break;\n+\tcase ERROR_TOO_MANY_MODULES: error = EMFILE; break;\n+\tcase ERROR_TOO_MANY_OPEN_FILES: error = EMFILE; break;\n+\tcase ERROR_UNRECOGNIZED_MEDIA: error = ENXIO; break;\n+\tcase ERROR_UNRECOGNIZED_VOLUME: error = ENODEV; break;\n+\tcase ERROR_WAIT_NO_CHILDREN: error = ECHILD; break;\n+\tcase ERROR_WRITE_FAULT: error = EIO; break;\n+\tcase ERROR_WRITE_PROTECT: error = EROFS; break;\n+\t}\n+\treturn error;\n+}\n+\n+#endif /* WINTHREAD_H */\ndiff --git a/preload-index.c b/preload-index.c\nindex 9289933..6d69a8d 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -10,7 +10,11 @@ static void preload_index(struct index_state *index, const char **pathspec)\n }\n #else\n \n+#ifndef _WIN32\n #include \n+#else\n+#include \n+#endif\n \n /*\n * Mostly randomly chosen maximum thread counts: we\n@@ -28,7 +32,11 @@ struct thread_data {\n \tint offset, nr;\n };\n \n+#ifndef _WIN32\n static void *preload_thread(void *_data)\n+#else\n+static DWORD WINAPI preload_thread(LPVOID _data)\n+#endif\n {\n \tint nr;\n \tstruct thread_data *p = _data;\n@@ -59,7 +67,11 @@ static void *preload_thread(void *_data)\n \t\t\tcontinue;\n \t\tce_mark_uptodate(ce);\n \t} while (--nr > 0);\n+#ifndef _WIN32\n \treturn NULL;\n+#else\n+\treturn 0;\n+#endif\n }\n \n static void preload_index(struct index_state *index, const char **pathspec)\n-- \n1.6.5.2\n"},{"id":"126688","messageId":"alpine.DEB.1.00.0911040031210.4985@pacific.mpi-cbg.de","threadId":"21472","inReplyTo":"1257283802-29726-2-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-11-03T23:38:13Z","receivedAt":"2009-11-03T23:38:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 3 Nov 2009, Andrzej K. Haczewski wrote:\n\n> ---\n\nCould you please add the reasoning from the cover letter to this commit \nmessage? And add a sign-off?\n\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 02f9246..a8a4f59 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -18,8 +18,12 @@\n> #include \"refs.h\"\n> \n> #ifdef THREADED_DELTA_SEARCH\n> -#include \"thread-utils.h\"\n> -#include \n> +# include \"thread-utils.h\"\n> +# ifndef _WIN32\n> +# include \n> +# else\n> +# include \n> +# endif\n> #endif\n> \n\nIt is unlikely that an #ifdef \"contamination\" of this extent will go \nthrough easily, but I have a suggestion that may make your patch both \neasier to read and more likely to be accepted into git.git: Try to wrap \nthe win32 calls into pthread-compatible function signatures. Then you can \nadd a compat/win32/pthread.h and not even touch core files of git.git at \nall.\n\nOh, and you definitely do not want to copy-paste err_win_to_posix(). You \ndefinitely want to reuse the existing instance.\n\nCiao,\nDscho\n"},{"id":"126704","messageId":"4AF0E842.2010201@workspacewhiz.com","threadId":"21472","inReplyTo":"alpine.DEB.1.00.0911040031210.4985@pacific.mpi-cbg.de","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Joshua Jensen","fromEmail":"jjensen@workspacewhiz.com","sentAt":"2009-11-04T02:34:42Z","receivedAt":"2009-11-04T02:34:42Z","isPatch":true,"sender":{"key":"jjensen@workspacewhiz.com","avatar":"https://avatars.githubusercontent.com/u/111687?v=4"},"body":"----- Original Message -----\nFrom: Johannes Schindelin\nDate: 11/3/2009 4:38 PM\n>> #ifdef THREADED_DELTA_SEARCH\n>> -#include \"thread-utils.h\"\n>> -#include\n>> +# include \"thread-utils.h\"\n>> +# ifndef _WIN32\n>> +# include\n>> +# else\n>> +# include\n>> +# endif\n>> #endif\n>>\n>> \n> It is unlikely that an #ifdef \"contamination\" of this extent will go\n> through easily, but I have a suggestion that may make your patch both\n> easier to read and more likely to be accepted into git.git: Try to wrap\n> the win32 calls into pthread-compatible function signatures. Then you can\n> add a compat/win32/pthread.h and not even touch core files of git.git at\n> all.\n> \nPardon my ignorance, but is there a reason to not use Pthreads for \nWin32? http://sourceware.org/pthreads-win32/\n\nJosh\n"},{"id":"126737","messageId":"16cee31f0911032344m3263730l607c02eb4e9adef5@mail.gmail.com","threadId":"21472","inReplyTo":"4AF0E842.2010201@workspacewhiz.com","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T07:44:54Z","receivedAt":"2009-11-04T07:44:54Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":">\n> Pardon my ignorance, but is there a reason to not use Pthreads for Win32?\n>  http://sourceware.org/pthreads-win32/\n>\n\nNot using pthreads on Windows makes Git:\n1. faster on that platform\n2. not depend on Pthreads for Win32\n\nIMHO that makes Git one step closer to become native on Windows, and\nis a sensible step.\n\nAndrew\n"},{"id":"126739","messageId":"4AF13819.7050306@viscovery.net","threadId":"21472","inReplyTo":"1257283802-29726-2-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-04T08:15:21Z","receivedAt":"2009-11-04T08:15:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andrzej K. Haczewski schrieb:\n> ---\n\nYou should sign-off your patches.\n\n> #ifdef THREADED_DELTA_SEARCH\n> -#include \"thread-utils.h\"\n> -#include \n> +# include \"thread-utils.h\"\n> +# ifndef _WIN32\n> +# include \n> +# else\n> +# include \n> +# endif\n> #endif\n\nCan't you just use the pthread package that is included in msysgit?\n\n> +#ifndef _WIN32\n> static void *threaded_find_deltas(void *arg)\n> +#else\n> +static DWORD WINAPI threaded_find_deltas(LPVOID arg)\n> +#endif\n> ...\n> +#ifndef _WIN32\n> \treturn NULL;\n> +#else\n> +\treturn 0;\n> +#endif\n> etc ...\n\nYou have far too many #ifdef in the generic code. There must be a better\nway to hide the implementation details of this emulation.\n\n> +#ifdef _WIN32\n> +\t/*\n> +\t * Windows require initialization of mutex (CRITICAL_SECTION)\n> +\t * and conditional variable.\n> +\t */\n> +\tpthread_mutex_init(&read_mutex);\n> +\tpthread_mutex_init(&cache_mutex);\n> +\tpthread_mutex_init(&progress_mutex);\n> +\twin32_cond_init(&progress_cond);\n> +#endif\n\n*If* we are going to use this minimal pthreads implementation, then I\nthink it will be OK to call pthread_*_init even on non-Windows.\n\n> +static __inline int win32_cond_init(win32_cond_t *cond)\n> +{\n> +\tcond->waiters = 0;\n> +\n> +\tInitializeCriticalSection(&cond->waiters_lock);\n> +\n> +\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n\nWouldn't an Event object be lighter-weight? (I'm only guessing.)\n\n> +\tif (NULL == cond->sema)\n> +\t\treturn -1;\n> +\treturn 0;\n> +}\n> +\n> +static __inline int win32_cond_destroy(win32_cond_t *cond)\n> +{\n> +\tCloseHandle(cond->sema);\n> +\tcond->sema = NULL;\n> +\n> +\tDeleteCriticalSection(&cond->waiters_lock);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static __inline int win32_cond_wait(win32_cond_t *cond, CRITICAL_SECTION *mutex)\n\nAnd the reason that this is not pthread_cond_wait, is...?\n\n> +{\n> +\tDWORD result;\n> +\tint ret = 0;\n> +\n> +\t/* we're waiting... */\n> +\tEnterCriticalSection(&cond->waiters_lock);\n> +\t++cond->waiters;\n> +\tLeaveCriticalSection(&cond->waiters_lock);\n> +\n> +\t/* unlock external mutex and wait for signal */\n> +\tLeaveCriticalSection(mutex);\n> +\tresult = WaitForSingleObject(cond->sema, INFINITE);\n\nReleasing the mutex and entering the wait state as well as leaving the\nwait state and reacquiring the mutex should be atomic. Neither are in this\nimplementation. You are not mentioning why you are implementing things\nlike this and why this would be acceptable.\n\n> +\n> +\tif (0 != result)\n> +\t\tret = -1;\n> +\n> +\t/* one waiter less */\n> +\tEnterCriticalSection(&cond->waiters_lock);\n> +\t--cond->waiters;\n> +\tLeaveCriticalSection(&cond->waiters_lock);\n> +\n> +\t/* lock external mutex again */\n> +\tEnterCriticalSection(mutex);\n\n> +/* almost copy-paste code of mingw.c */\n> +static int err_win_to_posix()\n> +{\n\nThere must be a better way than to just copy & paste this huge piece of code.\n\n-- Hannes\n"},{"id":"126740","messageId":"16cee31f0911040017x70c7dfeev6696c3ea8f7ffe56@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.DEB.1.00.0911040031210.4985@pacific.mpi-cbg.de","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T08:17:55Z","receivedAt":"2009-11-04T08:17:55Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Johannes Schindelin :\n> Could you please add the reasoning from the cover letter to this commit\n> message?  And add a sign-off?\n\nSure, will do so for next submission of that patch.\n\n> It is unlikely that an #ifdef \"contamination\" of this extent will go\n> through easily, but I have a suggestion that may make your patch both\n> easier to read and more likely to be accepted into git.git: Try to wrap\n> the win32 calls into pthread-compatible function signatures.  Then you can\n> add a compat/win32/pthread.h and not even touch core files of git.git at\n> all.\n\nFirst of all I didn't want to use wrappers because (if not inlined)\nthey introduce one additional call, that can be avoided with #defines\n(as you can see even pthread_init can be done with macro). Second\nreason is that I didn't want to create wrapping structures that would\nneed to be initialized / allocated / tracked. That patch translates\npthread calls to purely Win32 calls without anything in between.\n\nHere are my reasoning for some of these #ifdefs and what can be done\nand what can't (without using wrappers):\n\n1. Thread routine has very different signature:\nvoid *__cdecl func(void *); /* pthreads */\nuint32_t __stdcall func(void *); /* Windows API */\nFirst I thought it might be a problem to do (especially return value,\nwhich is different size for 64-bit architectures), but since Git\ndoesn't use return value, it can be done.\n\n2. Initialization of CRITICAL_SECTION and SEMAPHORE (used by condition\nvariables implementation). These need explicit initialization on\nWindows and can't be done statically with PTHREAD_MUTEX_INITIALIZER\nand PTHREAD_COND_INITIALIZER. There's no easy way around that (read:\nit needs wrappers).\n\n> Oh, and you definitely do not want to copy-paste err_win_to_posix().  You\n> definitely want to reuse the existing instance.\n\nYeah, that was lazy, mea culpa.\n\nI'll resubmit the patch with some fixes shortly,\nAndrew\n"},{"id":"126741","messageId":"4AF13A4F.5070300@viscovery.net","threadId":"21472","inReplyTo":"16cee31f0911032344m3263730l607c02eb4e9adef5@mail.gmail.com","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-04T08:24:47Z","receivedAt":"2009-11-04T08:24:47Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"[please don't cull Cc list on this ML]\n\nAndrzej K. Haczewski schrieb:\n>> Pardon my ignorance, but is there a reason to not use Pthreads for Win32?\n>> http://sourceware.org/pthreads-win32/\n>>\n> \n> Not using pthreads on Windows makes Git:\n> 1. faster on that platform\n\nI believe this only if you present hard numbers. My guess is that (for\nexample) packing objects with two threads is still faster with a slow\npthreads emulation than without threading at all.\n\n> 2. not depend on Pthreads for Win32\n\nWhy is this an advantage?\n\n> IMHO that makes Git one step closer to become native on Windows, and\n> is a sensible step.\n\nEmulating pthreads on Windows with all its facets is an extremely\ndifficult task. If exact POSIX conformance is needed, I would choose an\nexisting package over doing it myself at any time.\n\nGranted, we don't need the esoteric parts (cancelation points), which\nwould simplify the emulation a lot. But, as I pointed out in my other\nmail, even a pthread_cond_wait() is not that trivial to implement with the\nWindows API.\n\n-- Hannes\n"},{"id":"126742","messageId":"d2e97e800911040048h42ef7193v117773cc528b2361@mail.gmail.com","threadId":"21472","inReplyTo":"4AF13819.7050306@viscovery.net","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Michael Wookey","fromEmail":"michaelwookey@gmail.com","sentAt":"2009-11-04T08:48:35Z","receivedAt":"2009-11-04T08:48:35Z","isPatch":true,"sender":{"key":"michaelwookey@gmail.com","avatar":"https://avatars.githubusercontent.com/u/19476?v=4"},"body":"2009/11/4 Johannes Sixt :\n> Andrzej K. Haczewski schrieb:\n>\n>> +static __inline int win32_cond_init(win32_cond_t *cond)\n>> +{\n>> +     cond->waiters = 0;\n>> +\n>> +     InitializeCriticalSection(&cond->waiters_lock);\n>> +\n>> +     cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n>\n> Wouldn't an Event object be lighter-weight? (I'm only guessing.)\n\nBoth events and semaphores resolve to wait-able kernel objects; so\nneither is \"lighter-weight\" than the other.\n"},{"id":"126748","messageId":"1257331059-26344-1-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":"1257283802-29726-1-git-send-email-ahaczewski@gmail.com","subject":"[PATCH] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T10:37:39Z","receivedAt":"2009-11-04T10:37:39Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"Here is slightly modified patch with more comments where explanations were\nrequested (ie. non atomic release mutex and wait).\n\nThe implementation of conditional variable is based on ACE.\n\nThe patch needs testing from someone capable of compiling Git on Windows\nand running it with msysgit environment. I can confirm that it compiles\ncleanly on both Linux and Windows. I modified Makefile only for MSVC\npart, so if you'd like to compile it with mingw or cygwin, proper\ncorrections have to be made. I aim for native MSVC compilation, that's\nwhy I did it like that. That's also the reason I don't like\nhaving Pthreads for Win32 dependency - it's faster to use native\ncalls than depend on 3rd party wrapper library to do it for you\n(ie. pthreads for win32 does allocations to implement POSIX\nstandard, and full-conformance isn't required by Git, since Git uses\nonly small subset of pthreads).\n\nOne more motivation I had for the patch: as I was reading through\narchives I had a feeling that Git aims to be as lightweight\nas possible, hence removing additional dependencies (even for\nWindows platform) seems sensible to me.\n\nSigned-off-by: Andrzej K. Haczewski \n---\n Makefile | 4 +-\n builtin-pack-objects.c | 29 +++++++++-\n compat/mingw.c | 2 +-\n compat/win32/pthread.h | 143 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 13 ++++\n preload-index.c | 4 +-\n 6 files changed, 187 insertions(+), 8 deletions(-)\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex 521e8a5..450d8fe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -939,7 +939,7 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n@@ -947,7 +947,7 @@ ifdef MSVC\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..c96d293 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1592,7 +1592,7 @@ struct thread_params {\n \n static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n \n-static void *threaded_find_deltas(void *arg)\n+static THREAD_FUNC(threaded_find_deltas, arg)\n {\n \tstruct thread_params *me = arg;\n \n@@ -1620,7 +1620,7 @@ static void *threaded_find_deltas(void *arg)\n \t\tpthread_mutex_unlock(&me->mutex);\n \t}\n \t/* leave ->working 1 so that this doesn't get more work assigned */\n-\treturn NULL;\n+\tTHREAD_RETURN(NULL);\n }\n \n static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n@@ -2327,6 +2327,18 @@ 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+\n+#ifdef _WIN32\n+\t/*\n+\t * Windows requires initialization of mutex (CRITICAL_SECTION)\n+\t * and conditional variable.\n+\t */\n+\tpthread_mutex_init(&read_mutex);\n+\tpthread_mutex_init(&cache_mutex);\n+\tpthread_mutex_init(&progress_mutex);\n+\tpthread_cond_init(&progress_cond, NULL);\n+#endif\n+\n #endif\n \n \tprepare_packed_git();\n@@ -2345,7 +2357,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tstop_progress(&progress_state);\n \n \tif (non_empty && !nr_result)\n-\t\treturn 0;\n+\t\tgoto cleanup;\n \tif (nr_result)\n \t\tprepare_pack(window, depth);\n \twrite_pack_file();\n@@ -2353,5 +2365,16 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tfprintf(stderr, \"Total %\"PRIu32\" (delta %\"PRIu32\"),\"\n \t\t\t\" reused %\"PRIu32\" (delta %\"PRIu32\")\\n\",\n \t\t\twritten, written_delta, reused, reused_delta);\n+\n+cleanup:\n+#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)\n+\t/* cleanup Windows threads thingies */\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+#endif\n+\n \treturn 0;\n }\n+\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..8f82d3c\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,143 @@\n+/*\n+ * Header used to \"adapt\" pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+ \n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * don't include mingw.h for err_win_to_posix function - mingw.h doesn't \n+ * have include-guards\n+ */\n+extern int err_win_to_posix(DWORD winerr);\n+\n+/* Implement simple condition variable for Windows threads, based on ACE implementation */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+#define PTHREAD_COND_INITIALIZER { 0, { 0 }, NULL }\n+\n+static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (NULL == cond->sema) \n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n+static __inline int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+static __inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\tint ret = 0;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait */\n+\tif (0 != WaitForSingleObject(cond->sema, INFINITE))\n+\t\tret = -1;\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn ret;\n+}\n+\n+static __inline int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;\n+\telse\n+\t\treturn 0;\n+}\n+\n+#define pthread_t HANDLE\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define PTHREAD_MUTEX_INITIALIZER { 0 }\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+static __inline int pthread_create(pthread_t *t, const void *unused, DWORD (__stdcall *start_routine)(LPVOID), void *arg)\n+{\n+\t*t = CreateThread(NULL, 0, start_routine, arg, 0, NULL);\n+\n+\tif (NULL == *t) {\n+\t\terrno = err_win_to_posix(GetLastError());\n+\t\treturn -1;\n+\t} else {\n+\t\terrno = 0;\n+\t\treturn 0;\n+\t}\n+}\n+\n+static __inline int pthread_join(pthread_t t, void **unused)\n+{\n+\tDWORD result = WaitForSingleObject(t, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\terrno = 0;\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\terrno = EINVAL;\n+\t\t\treturn -1;\n+\t\tdefault:\n+\t\t\terrno = err_win_to_posix(GetLastError());\n+\t\t\treturn -1;\n+\t}\n+}\n+\n+#endif /* PTHREAD_H */\n+\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ef60803..202b90e 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -464,4 +464,17 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n */\n int unlink_or_warn(const char *path);\n \n+/*\n+ * Properly defines thread routine for Windows and POSIX\n+ */\n+#ifndef NO_PTHREADS\n+# ifndef _WIN32\n+# define THREAD_FUNC(f, a) void *f(void *a)\n+# define THREAD_RETURN(x) return (x)\n+# else\n+# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)\n+# define THREAD_RETURN(x) return (DWORD)(x);\n+# endif\n+#endif\n+\n #endif\ndiff --git a/preload-index.c b/preload-index.c\nindex 9289933..ace10fe 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -28,7 +28,7 @@ struct thread_data {\n \tint offset, nr;\n };\n \n-static void *preload_thread(void *_data)\n+static THREAD_FUNC(preload_thread, _data)\n {\n \tint nr;\n \tstruct thread_data *p = _data;\n@@ -59,7 +59,7 @@ static void *preload_thread(void *_data)\n \t\t\tcontinue;\n \t\tce_mark_uptodate(ce);\n \t} while (--nr > 0);\n-\treturn NULL;\n+\tTHREAD_RETURN(NULL);\n }\n \n static void preload_index(struct index_state *index, const char **pathspec)\n-- \n1.6.5.2\n"},{"id":"126750","messageId":"40aa078e0911040250k55fa1920g6eee5657c6e35345@mail.gmail.com","threadId":"21472","inReplyTo":"1257331059-26344-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-11-04T10:50:26Z","receivedAt":"2009-11-04T10:50:26Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Nov 4, 2009 at 11:37 AM, Andrzej K. Haczewski\n wrote:\n> +/*\n> + * Properly defines thread routine for Windows and POSIX\n> + */\n> +#ifndef NO_PTHREADS\n> +# ifndef _WIN32\n> +#  define THREAD_FUNC(f, a) void *f(void *a)\n> +#  define THREAD_RETURN(x) return (x)\n> +# else\n> +#  define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)\n> +#  define THREAD_RETURN(x) return (DWORD)(x);\n> +# endif\n> +#endif\n> +\n\nSeeing how THREAD_RETURN is only called with NULL-parameter, perhaps\nit would be better to do this?\n\n+/*\n+ * Properly defines thread routine for Windows and POSIX\n+ */\n+#ifndef NO_PTHREADS\n+# ifndef _WIN32\n+# define THREAD_FUNC(f, a) void *f(void *a)\n+# define THREAD_RETURN() return NULL;\n+# else\n+# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)\n+# define THREAD_RETURN() return 0;\n+# endif\n+#endif\n> +\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"126751","messageId":"16cee31f0911040253t57e9f8a4le70db2f5607fd645@mail.gmail.com","threadId":"21472","inReplyTo":"4AF13819.7050306@viscovery.net","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T10:53:30Z","receivedAt":"2009-11-04T10:53:30Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Johannes Sixt :\n> Can't you just use the pthread package that is included in msysgit?\n\nI don't like bloat, and msysgit is bloated. Sure, there are parts of\nmsysgit that are even heavier (bash, perl), but this will be removed\nwith further C'ification of scripts. I did what I thought could be\nsensible for my first patch. I'm newbie after all.\n\n>> +     /* we're waiting... */\n>> +     EnterCriticalSection(&cond->waiters_lock);\n>> +     ++cond->waiters;\n>> +     LeaveCriticalSection(&cond->waiters_lock);\n>> +\n>> +     /* unlock external mutex and wait for signal */\n>> +     LeaveCriticalSection(mutex);\n>> +     result = WaitForSingleObject(cond->sema, INFINITE);\n>\n> Releasing the mutex and entering the wait state as well as leaving the\n> wait state and reacquiring the mutex should be atomic. Neither are in this\n> implementation. You are not mentioning why you are implementing things\n> like this and why this would be acceptable.\n\nIt's safe to do it like this here because we're serializing waiters\ncount and when signaling we make sure we have waiters before we\nrelease semaphore. That implementation is based on ACE.\n\nAndrew\n"},{"id":"126753","messageId":"16cee31f0911040256s1c69e86j46ac573b4ef33017@mail.gmail.com","threadId":"21472","inReplyTo":"40aa078e0911040250k55fa1920g6eee5657c6e35345@mail.gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T10:56:00Z","receivedAt":"2009-11-04T10:56:00Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Erik Faye-Lund :\n>\n> Seeing how THREAD_RETURN is only called with NULL-parameter, perhaps\n> it would be better to do this?\n>\n> +/*\n> + * Properly defines thread routine for Windows and POSIX\n> + */\n> +#ifndef NO_PTHREADS\n> +# ifndef _WIN32\n> +#  define THREAD_FUNC(f, a) void *f(void *a)\n> +#  define THREAD_RETURN() return NULL;\n> +# else\n> +#  define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)\n> +#  define THREAD_RETURN() return 0;\n> +# endif\n> +#endif\n\nGood point. Should I resubmit the patch again?\n\nAndrew\n"},{"id":"126756","messageId":"alpine.DEB.1.00.0911041201380.2788@felix-maschine","threadId":"21472","inReplyTo":"4AF0E842.2010201@workspacewhiz.com","subject":"Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-11-04T11:02:20Z","receivedAt":"2009-11-04T11:02:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nI do not appreciate at all that you culled me from the Cc: list.\n\nOn Tue, 3 Nov 2009, Joshua Jensen wrote:\n\n> ----- Original Message -----\n> From: Johannes Schindelin\n> Date: 11/3/2009 4:38 PM\n> > > #ifdef THREADED_DELTA_SEARCH\n> > > -#include \"thread-utils.h\"\n> > > -#include\n> > > +# include \"thread-utils.h\"\n> > > +# ifndef _WIN32\n> > > +# include\n> > > +# else\n> > > +# include\n> > > +# endif\n> > > #endif\n> > >\n> > > \n> > It is unlikely that an #ifdef \"contamination\" of this extent will go \n> > through easily, but I have a suggestion that may make your patch both \n> > easier to read and more likely to be accepted into git.git: Try to \n> > wrap the win32 calls into pthread-compatible function signatures. \n> > Then you can add a compat/win32/pthread.h and not even touch core \n> > files of git.git at all.\n> > \n> Pardon my ignorance, but is there a reason to not use Pthreads for \n> Win32? http://sourceware.org/pthreads-win32/\n\nPthreads is a rather large dependency we do not really need.\n\nCiao,\nDscho\n"},{"id":"126757","messageId":"hcrnm0$dh6$1@ger.gmane.org","threadId":"21472","inReplyTo":"40aa078e0911040250k55fa1920g6eee5657c6e35345@mail.gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-11-04T11:14:08Z","receivedAt":"2009-11-04T11:14:08Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 11/04/2009 11:50 AM, Erik Faye-Lund wrote:\n> On Wed, Nov 4, 2009 at 11:37 AM, Andrzej K. Haczewski\n> wrote:\n>> +/*\n>> + * Properly defines thread routine for Windows and POSIX\n>> + */\n>> +#ifndef NO_PTHREADS\n>> +# ifndef _WIN32\n>> +# define THREAD_FUNC(f, a) void *f(void *a)\n>> +# define THREAD_RETURN(x) return (x)\n>> +# else\n>> +# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)\n>> +# define THREAD_RETURN(x) return (DWORD)(x);\n>> +# endif\n>> +#endif\n>> +\n>\n> Seeing how THREAD_RETURN is only called with NULL-parameter, perhaps\n> it would be better to do this?\n>\n> +/*\n> + * Properly defines thread routine for Windows and POSIX\n> + */\n> +#ifndef NO_PTHREADS\n> +# ifndef _WIN32\n> +# define THREAD_FUNC(f, a) void *f(void *a)\n> +# define THREAD_RETURN() return NULL;\n> +# else\n> +# define THREAD_FUNC(f, a) DWORD __stdcall f(LPVOID a)\n> +# define THREAD_RETURN() return 0;\n> +# endif\n> +#endif\n>> +\n\nEven better, \"return 0\" is good under either platform (0 converts to \nvoid *), and LPVOID is the same thing as void*, so you can just do\n\n#ifndef _WIN32\n# define THREAD_RET_TYPE DWORD __stdcall\n#else\n# define THREAD_RET_TYPE void *\n#endif\n\nPaolo\n"},{"id":"126759","messageId":"hcro74$fgb$1@ger.gmane.org","threadId":"21472","inReplyTo":"1257331059-26344-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-11-04T11:23:15Z","receivedAt":"2009-11-04T11:23:15Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"The condition variable implementation seems more complicated than \nnecessary. The mutex can be used to protect access to cond->waiters, so \nwaiters_lock is not necessary. On the other hand, it seems to me that \npthread_cond_signal should be the one that decrements the waiters count. \n Otherwise, a loop like\n\n\twhile (pthread_cond_signal (cond, mutex));\n\nwill fill the semaphore with signals and the waiters will get lots of \nspurious accesses.\n\nstatic __inline int pthread_cond_wait(pthread_cond_t *cond,\n CRITICAL_SECTION *mutex)\n{\n\tint ret = 0;\n\n\t/* the mutex protects access to waiters count */\n\t++cond->waiters;\n\n\t/*\n\t * Unlock external mutex and wait for signal.\n\t * NOTE: cond->waiters > 0 now. If pthread_cond_signal\n\t * is called after leaving mutex unlocked before we wait on\n\t * semaphore, it will add a signal to the semaphore,\n\t * and we'll happily go on with the wait. This would not\n\t * happen with an event, for example.\n\t */\n\tLeaveCriticalSection(mutex);\n\tif (0 != WaitForSingleObject(cond->sema, INFINITE))\n\t\tret = -1;\n\n\tEnterCriticalSection(mutex);\n\treturn ret;\n}\n\nstatic __inline int pthread_cond_signal(pthread_cond_t *cond)\n{\n\t/* the mutex protects access to waiters count */\n\tif (cond->waiters > 0) {\n\t\t--cond->waiters;\n\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;\n\t} else\n\t\treturn 0;\n}\n\nPaolo\n"},{"id":"126760","messageId":"4AF175E8.7020400@viscovery.net","threadId":"21472","inReplyTo":"1257331059-26344-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-04T12:39:04Z","receivedAt":"2009-11-04T12:39:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Please do not cull Cc list when you resend a patch, if possible.\n\nAfter staring some time on the code, I have convinced myself that the\npthread_cond_wait and pthread_cond_signal implementation will work *in our\nusage scenario* that has these preconditions:\n\n- There is no more than one thread waiting on any particular condition\nvariable instance.\n\n- pthread_cond_signal is called while the mutex is held.\n\n- We retest the condition after pthread_cond_wait returns.\n\nThese conditions should be attached in BIG BOLD letters to this\nimplementation; particularly, the last one.\n\nOn to your patch...\n\nThe subject is a bit misleading, IMHO. You are not porting the\n(p)threading code, but you are adding pthread_* function wrappers for Windows.\n\nYour patch adds whitespace-at-eol. Please use git show --check to see where.\n\nAndrzej K. Haczewski schrieb:\n> Here is slightly modified patch with more comments where explanations were\n> requested (ie. non atomic release mutex and wait).\n> \n> The implementation of conditional variable is based on ACE.\n> \n> The patch needs testing from someone capable of compiling Git on Windows\n> and running it with msysgit environment. I can confirm that it compiles\n> cleanly on both Linux and Windows. I modified Makefile only for MSVC\n> part, so if you'd like to compile it with mingw or cygwin, proper\n> corrections have to be made. I aim for native MSVC compilation, that's\n> why I did it like that. That's also the reason I don't like\n> having Pthreads for Win32 dependency - it's faster to use native\n> calls than depend on 3rd party wrapper library to do it for you\n> (ie. pthreads for win32 does allocations to implement POSIX\n> standard, and full-conformance isn't required by Git, since Git uses\n> only small subset of pthreads).\n> \n> One more motivation I had for the patch: as I was reading through\n> archives I had a feeling that Git aims to be as lightweight\n> as possible, hence removing additional dependencies (even for\n> Windows platform) seems sensible to me.\n> \n> Signed-off-by: Andrzej K. Haczewski \n\nPlease drop words from the commit message that do not make sense once this\ncommit is in git's history. Look at existing commit messages to get a\nfeeling for the style. Do write about \"why\" (motivation), \"how\" (design\nchoices) and \"how not\" (dead ends that you tried).\n\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 02f9246..c96d293 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -1592,7 +1592,7 @@ struct thread_params {\n> \n> static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n> \n> -static void *threaded_find_deltas(void *arg)\n> +static THREAD_FUNC(threaded_find_deltas, arg)\n> ...\n> -\treturn NULL;\n> +\tTHREAD_RETURN(NULL);\n\nSee Erik's and Paolo's comments.\n\n> @@ -2327,6 +2327,18 @@ 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> +\n> +#ifdef _WIN32\n> +\t/*\n> +\t * Windows requires initialization of mutex (CRITICAL_SECTION)\n> +\t * and conditional variable.\n> +\t */\n> +\tpthread_mutex_init(&read_mutex);\n> +\tpthread_mutex_init(&cache_mutex);\n> +\tpthread_mutex_init(&progress_mutex);\n> +\tpthread_cond_init(&progress_cond, NULL);\n> +#endif\n\nI think it would be OK to drop '= PTHREAD_{MUTEX,COND}_INITIALIZER' and\nuse *_init function calls without the #ifdef. Likewise for *_destroy.\n\n> +cleanup:\n> +#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)\n> +\t/* cleanup Windows threads thingies */\n> +\tpthread_cond_destroy(&progress_cond);\n> +\tpthread_mutex_destroy(&read_mutex);\n> +\tpthread_mutex_destroy(&cache_mutex);\n> +\tpthread_mutex_destroy(&progress_mutex);\n> +#endif\n> +\n> \treturn 0;\n> }\n> +\n\nDrop this empty line at EOF.\n\n> @@ -0,0 +1,143 @@\n> +/*\n> + * Header used to \"adapt\" pthread-based POSIX code to Windows API threads.\n\nI think \"adapt\" is the right word here. You don't need to put it in quotes. ;)\n\n> + *\n> + * Copyright (C) 2009 Andrzej K. Haczewski \n> + */\n> + \n> +#ifndef PTHREAD_H\n> +#define PTHREAD_H\n> +\n> +#ifndef WIN32_LEAN_AND_MEAN\n> +#define WIN32_LEAN_AND_MEAN\n> +#endif\n> +\n> +#include \n> +\n> +/*\n> + * don't include mingw.h for err_win_to_posix function - mingw.h doesn't \n> + * have include-guards\n\nSo what? Is there an #include loop? Can't you add include guards?\n\n> +static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n\nWhat's wrong with 'static inline int ...' (without the underscores)?\n\n> +{\n> +\tcond->waiters = 0;\n> +\n> +\tInitializeCriticalSection(&cond->waiters_lock);\n> +\n> +\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n> +\tif (NULL == cond->sema) \n> +\t\treturn -1;\n> +\treturn 0;\n\nIn case of failure, the pthread_* functions return the error number, not\n-1. Moreover, we write\n\n\tif (!cond->sema)\n\t\treturn err_win_to_posix(GetLastError());\nor\n\treturn cond->sema ? 0 : err_win_to_posix(GetLastError());\n\n> +static __inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n> +{\n> ...\n> +\t/* let's wait */\n> +\tif (0 != WaitForSingleObject(cond->sema, INFINITE))\n> +\t\tret = -1;\n\nMind the return value!\n\n> +static __inline int pthread_cond_signal(pthread_cond_t *cond)\n> +{\n> ...\n> +\tif (have_waiters)\n> +\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;\n\nReturn value again.\n\n> +static __inline int pthread_create(pthread_t *t, const void *unused, DWORD (__stdcall *start_routine)(LPVOID), void *arg)\n> +{\n> +\t*t = CreateThread(NULL, 0, start_routine, arg, 0, NULL);\n> +\n> +\tif (NULL == *t) {\n\n\tif (!*t)\n\n> +\t\terrno = err_win_to_posix(GetLastError());\n> +\t\treturn -1;\n\nReturn value again. errno is not set.\n\n> +\t} else {\n> +\t\terrno = 0;\n> +\t\treturn 0;\n> +\t}\n> +}\n> +\n> +static __inline int pthread_join(pthread_t t, void **unused)\n> +{\n> ...\n> +\t\t\terrno = err_win_to_posix(GetLastError());\n> +\t\t\treturn -1;\n\nAnd again.\n\n-- Hannes\n"},{"id":"126763","messageId":"16cee31f0911040547m69e5b9cbi30e20d2a7790bd6f@mail.gmail.com","threadId":"21472","inReplyTo":"4AF175E8.7020400@viscovery.net","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T13:47:09Z","receivedAt":"2009-11-04T13:47:09Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Johannes Sixt :\n> Please do not cull Cc list when you resend a patch, if possible.\n\nOk, will do. I was sending patch using git send-email and I just\nforgot to copy Cc there. Still trying to BTW is there a way to\n\"reformat-patch\" with new amended commit and then \"resend-email\"?\n\n> After staring some time on the code, I have convinced myself that the\n> pthread_cond_wait and pthread_cond_signal implementation will work *in our\n> usage scenario* that has these preconditions:\n\nBut it is not impossible with that implementation. I based this\nimplementation on ACE (Adaptive Communication Environment, large C++\nlibrary) implementation of the same concepts. All I removed from their\nimplementation is cond_broadcast, since it's not used by Git. I'm sure\nthat ACE does the best job when it comes to threading primitives.\n\nOn resubmit I'll give more credit to ACE.\n\n> - pthread_cond_signal is called while the mutex is held.\n\nAFAIK that is a requirement for condition variable to be signaled\nwhile holding the same mutex that other threads cond_wait on. I just\ndon't check that it is true, because Git is locking mutex.\n\n> - We retest the condition after pthread_cond_wait returns.\n>\n> These conditions should be attached in BIG BOLD letters to this\n> implementation; particularly, the last one.\n\nThat's also a known requirement for working with cond vars. Here's\nexcerpt from pthread_cond_wait man page:\nWhen using condition variables there is always a boolean predicate\ninvolving shared variables associated with each condition wait that is\ntrue if the thread should proceed. Spurious wakeups from the\npthread_cond_wait() or pthread_cond_timedwait() functions may occur.\nSince the return from pthread_cond_wait() or pthread_cond_timedwait()\ndoes not imply anything about the value of this predicate, the\npredicate should be re-evaluated upon such return.\n\n> The subject is a bit misleading, IMHO. You are not porting the\n> (p)threading code, but you are adding pthread_* function wrappers for Windows.\n>\n> Your patch adds whitespace-at-eol. Please use git show --check to see where.\n>\n> Please drop words from the commit message that do not make sense once this\n> commit is in git's history. Look at existing commit messages to get a\n> feeling for the style. Do write about \"why\" (motivation), \"how\" (design\n> choices) and \"how not\" (dead ends that you tried).\n>\n\nOk, thanks for pointing that out.\n\n> I think it would be OK to drop '= PTHREAD_{MUTEX,COND}_INITIALIZER' and\n> use *_init function calls without the #ifdef. Likewise for *_destroy.\n\nActually it won't save us many #ifdefs. There's one #ifdef for\ninitialization that could be saved, but then comes #ifdef for cleanup:\n#if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)\n\nWhat you propose will remove one #ifdef _WIN32 for initialization, but\nthe cleanup will look almost the same:\n#ifdef THREADED_DELTA_SEARCH\n\n>\n> -- Hannes\n>\n\nThanks for awesome review, I'll fix all those returns and whitespaces\nand resubmit.\n\n--\nAndrzej\n"},{"id":"126764","messageId":"alpine.DEB.1.00.0911041427140.2788@felix-maschine","threadId":"21472","inReplyTo":"1257331059-26344-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-11-04T14:04:09Z","receivedAt":"2009-11-04T14:04:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n\n\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 02f9246..c96d293 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -2327,6 +2327,18 @@ 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> +\n> +#ifdef _WIN32\n\nThis flies in the face of our endeavors to enhance readability by reducing \nthe number of #ifdef's, and at least guarding the #ifdef'ed parts behind \nmeaningful names rather than platform specifiers.\n\nSee for example THREADED_DELTA_SEARCH: it does not read \"HAS_PTHREADS\" or \nsome such.\n\nCiao,\nDscho\n"},{"id":"126765","messageId":"16cee31f0911040614p2d58c418m29e66ac825b12986@mail.gmail.com","threadId":"21472","inReplyTo":"4AF175E8.7020400@viscovery.net","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T14:14:49Z","receivedAt":"2009-11-04T14:14:49Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Johannes Sixt :\n\n>> +static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n>\n> What's wrong with 'static inline int ...' (without the underscores)?\n>\n\nForgot to answer. 'inline' is avaliable in C++ only (on MSVC at\nleast), while '__inline' is MS extension for C and C++.\n\n--\nAndrew\n"},{"id":"126766","messageId":"40aa078e0911040619m7ca6350bw659a95a17a56be2d@mail.gmail.com","threadId":"21472","inReplyTo":"16cee31f0911040614p2d58c418m29e66ac825b12986@mail.gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-11-04T14:19:39Z","receivedAt":"2009-11-04T14:19:39Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Nov 4, 2009 at 3:14 PM, Andrzej K. Haczewski\n wrote:\n> 2009/11/4 Johannes Sixt :\n>\n>>> +static __inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n>>\n>> What's wrong with 'static inline int ...' (without the underscores)?\n>>\n>\n> Forgot to answer. 'inline' is avaliable in C++ only (on MSVC at\n> least), while '__inline' is MS extension for C and C++.\n\nYou don't need it:\ncompat/msvc.h:9:#define inline __inline\n\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"126768","messageId":"4AF190F1.3020607@viscovery.net","threadId":"21472","inReplyTo":"16cee31f0911040547m69e5b9cbi30e20d2a7790bd6f@mail.gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-04T14:34:25Z","receivedAt":"2009-11-04T14:34:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andrzej K. Haczewski schrieb:\n> 2009/11/4 Johannes Sixt :\n>> After staring some time on the code, I have convinced myself that the\n>> pthread_cond_wait and pthread_cond_signal implementation will work *in our\n>> usage scenario* that has these preconditions:\n> \n> But it is not impossible with that implementation.\n\nRead again what I wrote: We are in agreement.\n\n> On resubmit I'll give more credit to ACE.\n\nPerhaps also a link to the source and, even better, to a discussion of the\nimplementation.\n\n>> - pthread_cond_signal is called while the mutex is held.\n> \n> AFAIK that is a requirement for condition variable to be signaled\n> while holding the same mutex that other threads cond_wait on. I just\n> don't check that it is true, because Git is locking mutex.\n\nIt is not a requirement, but, yes, we do hold the mutex.\n\n>> - We retest the condition after pthread_cond_wait returns.\n>>\n>> These conditions should be attached in BIG BOLD letters to this\n>> implementation; particularly, the last one.\n> \n> That's also a known requirement for working with cond vars.\n\nIndeed.\n\n>> I think it would be OK to drop '= PTHREAD_{MUTEX,COND}_INITIALIZER' and\n>> use *_init function calls without the #ifdef. Likewise for *_destroy.\n> \n> Actually it won't save us many #ifdefs. There's one #ifdef for\n> initialization that could be saved, but then comes #ifdef for cleanup:\n> #if defined(THREADED_DELTA_SEARCH) && defined(_WIN32)\n> \n> What you propose will remove one #ifdef _WIN32 for initialization, but\n> the cleanup will look almost the same:\n> #ifdef THREADED_DELTA_SEARCH\n\nYou are right. But #ifdef THREADED_DELTA_SEARCH is about a \"generic\"\nproperty of the code and is already used elsewhere in the file, whereas\n#ifdef WIN32 would be new and is is about platform differences.\n\nAnyway, we would have to see what Junio says about the new function calls,\nbecause he's usually quite anal when it comes to added code vs. static\ninitialization. ;)\n\n-- Hannes\n"},{"id":"126769","messageId":"16cee31f0911040650s3eba1067mb66a48bb50c97c28@mail.gmail.com","threadId":"21472","inReplyTo":"4AF190F1.3020607@viscovery.net","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T14:50:10Z","receivedAt":"2009-11-04T14:50:10Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Johannes Sixt :\n>\n> You are right. But #ifdef THREADED_DELTA_SEARCH is about a \"generic\"\n> property of the code and is already used elsewhere in the file, whereas\n> #ifdef WIN32 would be new and is is about platform differences.\n>\n> Anyway, we would have to see what Junio says about the new function calls,\n> because he's usually quite anal when it comes to added code vs. static\n> initialization. ;)\n\nI could do it with wrappers for pthread_mutex_lock and _unlock and\nlazy init there plus lazy init cond var in cond_wait and _signal, that\nway it could be done without any additional code in the first #ifdef.\nBut I don't see any simple solution for working around\ndeinitialization, that's why I'd leave non-static initialization. Let\nme put some touchups and resubmit for another round.\n\n--\nAndrzej\n"},{"id":"126772","messageId":"1257350100-29281-1-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":"1257331059-26344-1-git-send-email-ahaczewski@gmail.com","subject":"[PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T15:55:00Z","receivedAt":"2009-11-04T15:55:00Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"This patch implements native to Windows subset of pthreads API used by Git.\nIt allows to remove Pthreads for Win32 dependency for msysgit and cygwin.\n\nThe patch modifies Makefile only for MSVC (that's the environment I'm capable\nof testing on), so it requires further corrections to compile with MinGW\nor Cygwin.\n\nSigned-off-by: Andrzej K. Haczewski \n---\n Makefile | 8 ++-\n builtin-pack-objects.c | 41 +++++++++++++-\n compat/mingw.c | 2 +-\n compat/mingw.h | 5 ++\n compat/win32/pthread.h | 136 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 10 ++++\n preload-index.c | 4 +-\n 7 files changed, 198 insertions(+), 8 deletions(-)\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex 521e8a5..14c371c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -939,7 +939,8 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n+\tNO_STATIC_PTHREADS_INIT = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n@@ -947,7 +948,7 @@ ifdef MSVC\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\n@@ -1293,6 +1294,9 @@ ifdef THREADED_DELTA_SEARCH\n \tBASIC_CFLAGS += -DTHREADED_DELTA_SEARCH\n \tLIB_OBJS += thread-utils.o\n endif\n+ifdef NO_STATIC_PTHREADS_INIT\n+\tCOMPAT_CFLAGS += -DNO_STATIC_PTHREADS_INIT\n+endif\n ifdef DIR_HAS_BSD_GROUP_SEMANTICS\n \tCOMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS\n endif\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..f297d82 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1592,7 +1592,34 @@ struct thread_params {\n \n static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n \n-static void *threaded_find_deltas(void *arg)\n+/*\n+ * For platforms that do support static pthreads initialization\n+ * make these noops.\n+ */\n+#ifndef NO_STATIC_PTHREADS_INIT\n+# define init_threaded_delta_search()\n+# define cleanup_threaded_delta_search()\n+#else\n+static void init_threaded_delta_search()\n+{\n+\tpthread_mutex_init(&read_mutex);\n+\tpthread_mutex_init(&cache_mutex);\n+\tpthread_mutex_init(&progress_mutex);\n+\tpthread_cond_init(&progress_cond, NULL);\n+}\n+\n+static void cleanup_threaded_delta_search()\n+{\n+\t/* cleanup Windows threads thingies */\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+\n+}\n+#endif\n+\n+static THREAD_RET_TYPE threaded_find_deltas(void *arg)\n {\n \tstruct thread_params *me = arg;\n \n@@ -1620,7 +1647,7 @@ static void *threaded_find_deltas(void *arg)\n \t\tpthread_mutex_unlock(&me->mutex);\n \t}\n \t/* leave ->working 1 so that this doesn't get more work assigned */\n-\treturn NULL;\n+\treturn 0;\n }\n \n static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n@@ -2327,6 +2354,8 @@ 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+\n+\tinit_threaded_delta_search();\n #endif\n \n \tprepare_packed_git();\n@@ -2345,7 +2374,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tstop_progress(&progress_state);\n \n \tif (non_empty && !nr_result)\n-\t\treturn 0;\n+\t\tgoto cleanup;\n \tif (nr_result)\n \t\tprepare_pack(window, depth);\n \twrite_pack_file();\n@@ -2353,5 +2382,11 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tfprintf(stderr, \"Total %\"PRIu32\" (delta %\"PRIu32\"),\"\n \t\t\t\" reused %\"PRIu32\" (delta %\"PRIu32\")\\n\",\n \t\t\twritten, written_delta, reused, reused_delta);\n+\n+cleanup:\n+#ifdef THREADED_DELTA_SEARCH\n+\tcleanup_threaded_delta_search();\n+#endif\n+\n \treturn 0;\n }\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6907345..7e25fb5 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -294,3 +294,8 @@ struct mingw_dirent\n #define readdir(x) mingw_readdir(x)\n struct dirent *mingw_readdir(DIR *dir);\n #endif // !NO_MINGW_REPLACE_READDIR\n+\n+/*\n+ * Used by Pthread API implementation for Windows\n+ */\n+extern int err_win_to_posix(DWORD winerr);\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..a71b90f\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,136 @@\n+/*\n+ * Header used to adapt pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * Implement simple condition variable for Windows threads, based on ACE\n+ * implementation.\n+ *\n+ * See original implementation: http://bit.ly/1vkDjo\n+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html\n+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html\n+ */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+#define PTHREAD_COND_INITIALIZER { 0, { 0 }, NULL }\n+\n+static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (!cond->sema)\n+\t\treturn 0; /* POSIX do not allow pthread_cond_init to fail */\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait */\n+\tWaitForSingleObject(cond->sema, INFINITE))\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n+\t\t\t0 : err_win_to_posix(GetLastError();\n+\telse\n+\t\treturn 0;\n+}\n+\n+#define pthread_t HANDLE\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define PTHREAD_MUTEX_INITIALIZER { 0 }\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+static inline int pthread_create(pthread_t *thread, const void *unused,\n+\t\tDWORD (__stdcall *start_routine)(LPVOID), void *arg)\n+{\n+\t*thread = CreateThread(NULL, 0, start_routine, arg, 0, NULL);\n+\n+\tif (!*thread)\n+\t\treturn err_win_to_posix(GetLastError());\n+\telse\n+\t\treturn 0;\n+}\n+\n+static inline int pthread_join(pthread_t thread, void **unused)\n+{\n+\tDWORD result = WaitForSingleObject(t, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\treturn EINVAL;\n+\t\tdefault:\n+\t\t\treturn err_win_to_posix(GetLastError());\n+\t}\n+}\n+\n+#endif /* PTHREAD_H */\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ef60803..4311117 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -464,4 +464,14 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n */\n int unlink_or_warn(const char *path);\n \n+/*\n+ * Define type of thread function return type to distinguish\n+ * Windows and POSIX.\n+ */\n+#ifndef _WIN32\n+# define THREAD_RET_TYPE void *\n+#else\n+# define THREAD_RET_TYPE DWORD __stdcall\n+#endif\n+\n #endif\ndiff --git a/preload-index.c b/preload-index.c\nindex 9289933..41b11a3 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -28,7 +28,7 @@ struct thread_data {\n \tint offset, nr;\n };\n \n-static void *preload_thread(void *_data)\n+static THREAD_RET_TYPE preload_thread(void* _data)\n {\n \tint nr;\n \tstruct thread_data *p = _data;\n@@ -59,7 +59,7 @@ static void *preload_thread(void *_data)\n \t\t\tcontinue;\n \t\tce_mark_uptodate(ce);\n \t} while (--nr > 0);\n-\treturn NULL;\n+\treturn 0;\n }\n \n static void preload_index(struct index_state *index, const char **pathspec)\n-- \n1.6.5.2\n"},{"id":"126789","messageId":"alpine.LFD.2.00.0911041247250.10340@xanadu.home","threadId":"21472","inReplyTo":"1257350100-29281-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-04T18:10:22Z","receivedAt":"2009-11-04T18:10:22Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n\n> +\tNO_STATIC_PTHREADS_INIT = YesPlease\n\nLet's not go that route please. If Windows can't get away without \nruntime initializations then let's use them on all platforms. There is \nno gain in exploding the code path combinations here wrt testing \ncoverage.\n\n> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)\n\nWhy can't you just cast the function pointer in your pthread_create \nwrapper instead? No one cares about the returned value anyway.\n\n> @@ -2327,6 +2354,8 @@ 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> +\n> +\tinit_threaded_delta_search();\n\nWhat about doing this at the beginning of ll_find_deltas() instead?\nAnd similarly for cleanup_threaded_delta_search(): call it right before \nleaving ll_find_deltas(). This way thread issues would remain more \nlocalized. In fact I'd move the whole thing above in ll_find_deltas() \nas well (separately from this patch though).\n\n\nNicolas\n"},{"id":"126824","messageId":"alpine.LNX.2.00.0911041406101.14365@iabervon.org","threadId":"21472","inReplyTo":"16cee31f0911040650s3eba1067mb66a48bb50c97c28@mail.gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-11-04T20:43:01Z","receivedAt":"2009-11-04T20:43:01Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n\n> 2009/11/4 Johannes Sixt :\n> >\n> > You are right. But #ifdef THREADED_DELTA_SEARCH is about a \"generic\"\n> > property of the code and is already used elsewhere in the file, whereas\n> > #ifdef WIN32 would be new and is is about platform differences.\n> >\n> > Anyway, we would have to see what Junio says about the new function calls,\n> > because he's usually quite anal when it comes to added code vs. static\n> > initialization. ;)\n> \n> I could do it with wrappers for pthread_mutex_lock and _unlock and\n> lazy init there plus lazy init cond var in cond_wait and _signal, that\n> way it could be done without any additional code in the first #ifdef.\n> But I don't see any simple solution for working around\n> deinitialization, that's why I'd leave non-static initialization. Let\n> me put some touchups and resubmit for another round.\n\nIs it actually necessary to deinitialize? Since the variables are static \nand therefore can't leak, and would presumably not need to be \nreinitialized differently if they were used again, I think they should be \nable to just stay. If Windows is unhappy about processes still having \nlocks initialized at exit, I suppose we could go through and destroy all \nour mutexes and conds at cleanup time. Pthreads does have the appropriate \nfunctions, and it would be correct to use them, although unnecessary.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"126826","messageId":"16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911041247250.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T21:16:50Z","receivedAt":"2009-11-04T21:16:50Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/4 Nicolas Pitre :\n> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n>\n>> +     NO_STATIC_PTHREADS_INIT = YesPlease\n>\n> Let's not go that route please.  If Windows can't get away without\n> runtime initializations then let's use them on all platforms.  There is\n> no gain in exploding the code path combinations here wrt testing\n> coverage.\n>\n\nI don't like that approach either, but I was frighten of Junio being\nanal about static inits ;).\n\nLet's make it clear: has anyone have any objections that I add\nexplicit initialization of mutexes and condition variables for POSIX\nalso?\n\n>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)\n>\n> Why can't you just cast the function pointer in your pthread_create\n> wrapper instead?  No one cares about the returned value anyway.\n\nBecause of calling convention - I'd have to cast cdecl function as\nstdcall function, which would change the function call clean up (in\ncdecl caller is responsible for unwinding stack, stdcall callee; the\neffect - double stack unwinding).\n\n>> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n>>  #ifdef THREADED_DELTA_SEARCH\n>>       if (!delta_search_threads)      /* --threads=0 means autodetect */\n>>               delta_search_threads = online_cpus();\n>> +\n>> +     init_threaded_delta_search();\n>\n> What about doing this at the beginning of ll_find_deltas() instead?\n> And similarly for cleanup_threaded_delta_search(): call it right before\n> leaving ll_find_deltas().  This way thread issues would remain more\n> localized.  In fact I'd move the whole thing above in ll_find_deltas()\n> as well (separately from this patch though).\n\nSounds sensible, but I'd wait for the NO_STATIC_PTHREADS_INIT verdict.\n\n--\nAndrzej\n"},{"id":"126827","messageId":"alpine.LFD.2.00.0911041607250.10340@xanadu.home","threadId":"21472","inReplyTo":"alpine.LNX.2.00.0911041406101.14365@iabervon.org","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-04T21:17:15Z","receivedAt":"2009-11-04T21:17:15Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 4 Nov 2009, Daniel Barkalow wrote:\n\n> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n> \n> > 2009/11/4 Johannes Sixt :\n> > >\n> > > You are right. But #ifdef THREADED_DELTA_SEARCH is about a \"generic\"\n> > > property of the code and is already used elsewhere in the file, whereas\n> > > #ifdef WIN32 would be new and is is about platform differences.\n> > >\n> > > Anyway, we would have to see what Junio says about the new function calls,\n> > > because he's usually quite anal when it comes to added code vs. static\n> > > initialization. ;)\n> > \n> > I could do it with wrappers for pthread_mutex_lock and _unlock and\n> > lazy init there plus lazy init cond var in cond_wait and _signal, that\n> > way it could be done without any additional code in the first #ifdef.\n> > But I don't see any simple solution for working around\n> > deinitialization, that's why I'd leave non-static initialization. Let\n> > me put some touchups and resubmit for another round.\n> \n> Is it actually necessary to deinitialize? Since the variables are static \n> and therefore can't leak, and would presumably not need to be \n> reinitialized differently if they were used again, I think they should be \n> able to just stay. If Windows is unhappy about processes still having \n> locks initialized at exit, I suppose we could go through and destroy all \n> our mutexes and conds at cleanup time. Pthreads does have the appropriate \n> functions, and it would be correct to use them, although unnecessary.\n\nLazy initialization would probably turn up to be more expensive \n(checking a flag on each usage) than unconditionally initializing them \nonce. Remember that those are used at least once per object meaning a \nlot.\n\nAnd I much prefer having runtime initialization for both Unix and \nWindows than having separate paths on each platform potentially hiding \ndifferent bugs. And given that on Unix you can statically initialize \nthose, then a runtime initialization is certainly not going to be _that_ \ncostly.\n\nAnd while at it, we might just deinitialize them as soon as we're done \nwith them saving resources (on Unix that might turn up to be a no op \nanyway) which is why I suggested doing this within ll_find_deltas() \ndirectly.\n\n\nNicolas\n"},{"id":"126832","messageId":"alpine.LFD.2.00.0911041623570.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com","subject":"[PATCH] pack-objects: move thread autodetection closer to relevant code","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-04T21:32:46Z","receivedAt":"2009-11-04T21:32:46Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Let's keep thread stuff close together if possible. And in this case, \nthis even reduces the #ifdef noise, and allows for skipping the \nautodetection altogether if delta search is not needed (like with a pure \nclone).\n\nSigned-off-by: Nicolas Pitre \n---\n\n> 2009/11/4 Nicolas Pitre :\n> >> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n> >>  #ifdef THREADED_DELTA_SEARCH\n> >>       if (!delta_search_threads)      /* --threads=0 means autodetect */\n> >>               delta_search_threads = online_cpus();\n> >> +\n> >> +     init_threaded_delta_search();\n> >\n> > What about doing this at the beginning of ll_find_deltas() instead?\n> > And similarly for cleanup_threaded_delta_search(): call it right before\n> > leaving ll_find_deltas().  This way thread issues would remain more\n> > localized.  In fact I'd move the whole thing above in ll_find_deltas()\n> > as well (separately from this patch though).\n\nSo here it is.\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..4c91e94 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1629,6 +1629,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tstruct thread_params *p;\n \tint i, ret, active_threads = 0;\n \n+\tif (!delta_search_threads)\t/* --threads=0 means autodetect */\n+\t\tdelta_search_threads = online_cpus();\n \tif (delta_search_threads <= 1) {\n \t\tfind_deltas(list, &list_size, window, depth, processed);\n \t\treturn;\n@@ -2324,11 +2326,6 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \tif (keep_unreachable && unpack_unreachable)\n \t\tdie(\"--keep-unreachable and --unpack-unreachable are incompatible.\");\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)\n"},{"id":"126834","messageId":"40aa078e0911041341s1adbbf31t6961207ba9c7905b@mail.gmail.com","threadId":"21472","inReplyTo":"16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-11-04T21:41:42Z","receivedAt":"2009-11-04T21:41:42Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Nov 4, 2009 at 10:16 PM, Andrzej K. Haczewski\n wrote:\n>>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)\n>>\n>> Why can't you just cast the function pointer in your pthread_create\n>> wrapper instead?  No one cares about the returned value anyway.\n>\n> Because of calling convention - I'd have to cast cdecl function as\n> stdcall function, which would change the function call clean up (in\n> cdecl caller is responsible for unwinding stack, stdcall callee; the\n> effect - double stack unwinding).\n>\n\nCouldn't the windows version of pthread_create have a wrapper\nfunction, that corrected the calling convention, much like the\nfunction run_thread that start_async in run-command.c has?\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"126836","messageId":"alpine.LFD.2.00.0911041634030.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-04T21:52:14Z","receivedAt":"2009-11-04T21:52:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n\n> 2009/11/4 Nicolas Pitre :\n> > On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n> >\n> >> +     NO_STATIC_PTHREADS_INIT = YesPlease\n> >\n> > Let's not go that route please.  If Windows can't get away without\n> > runtime initializations then let's use them on all platforms.  There is\n> > no gain in exploding the code path combinations here wrt testing\n> > coverage.\n> >\n> \n> I don't like that approach either, but I was frighten of Junio being\n> anal about static inits ;).\n\nI think the alternative is much worse than loosing those static inits.\n\n> Let's make it clear: has anyone have any objections that I add\n> explicit initialization of mutexes and condition variables for POSIX\n> also?\n\nPlease do it and if anyone finds a problem with it then we'll start from \nthere.\n\n\nNicolas\n"},{"id":"126840","messageId":"alpine.LNX.2.00.0911041640060.14365@iabervon.org","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911041607250.10340@xanadu.home","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-11-04T22:22:38Z","receivedAt":"2009-11-04T22:22:38Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 4 Nov 2009, Nicolas Pitre wrote:\n\n> On Wed, 4 Nov 2009, Daniel Barkalow wrote:\n> \n> > On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n> > \n> > > 2009/11/4 Johannes Sixt :\n> > > >\n> > > > You are right. But #ifdef THREADED_DELTA_SEARCH is about a \"generic\"\n> > > > property of the code and is already used elsewhere in the file, whereas\n> > > > #ifdef WIN32 would be new and is is about platform differences.\n> > > >\n> > > > Anyway, we would have to see what Junio says about the new function calls,\n> > > > because he's usually quite anal when it comes to added code vs. static\n> > > > initialization. ;)\n> > > \n> > > I could do it with wrappers for pthread_mutex_lock and _unlock and\n> > > lazy init there plus lazy init cond var in cond_wait and _signal, that\n> > > way it could be done without any additional code in the first #ifdef.\n> > > But I don't see any simple solution for working around\n> > > deinitialization, that's why I'd leave non-static initialization. Let\n> > > me put some touchups and resubmit for another round.\n> > \n> > Is it actually necessary to deinitialize? Since the variables are static \n> > and therefore can't leak, and would presumably not need to be \n> > reinitialized differently if they were used again, I think they should be \n> > able to just stay. If Windows is unhappy about processes still having \n> > locks initialized at exit, I suppose we could go through and destroy all \n> > our mutexes and conds at cleanup time. Pthreads does have the appropriate \n> > functions, and it would be correct to use them, although unnecessary.\n> \n> Lazy initialization would probably turn up to be more expensive \n> (checking a flag on each usage) than unconditionally initializing them \n> once. Remember that those are used at least once per object meaning a \n> lot.\n\nMeh, checking a flag on the same cache line as the lock you're about to \ntake can't be a big incremental cost, especially if it's actually checking \nwhether some sort of cookie is non-zero before doing something with it.\n\n> And I much prefer having runtime initialization for both Unix and \n> Windows than having separate paths on each platform potentially hiding \n> different bugs. And given that on Unix you can statically initialize \n> those, then a runtime initialization is certainly not going to be _that_ \n> costly.\n\nYeah, definitely best to have them match, whichever way we go.\n\nI don't think it matters terribly much either way which we use, so long as \nits consistent. It'd be nice if the static initializers worked, just \nbecause people seem to write code with them, but we could just not do that \nin the future.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"126844","messageId":"4AF20534.2030004@gmail.com","threadId":"21472","inReplyTo":"40aa078e0911041341s1adbbf31t6961207ba9c7905b@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T22:50:28Z","receivedAt":"2009-11-04T22:50:28Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"Erik Faye-Lund pisze:\n> Couldn't the windows version of pthread_create have a wrapper\n> function, that corrected the calling convention, much like the\n> function run_thread that start_async in run-command.c has?\n\nCan't be done without allocations. I'd have to pass to that wrapping\nthread function an address of original function *and* an original\nargument, and there's no way to pack that as one void*.\n\n--\nAndrzej\n"},{"id":"126851","messageId":"4AF21283.3080407@gmail.com","threadId":"21472","inReplyTo":"16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com","subject":"[PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T23:47:15Z","receivedAt":"2009-11-04T23:47:15Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"This patch implements native to Windows subset of pthreads API used by Git.\nIt allows to remove Pthreads for Win32 dependency for msysgit and cygwin.\n\nThe patch modifies Makefile only for MSVC (that's the environment I'm\ncapable of testing on), so it requires further corrections to compile\nwith MinGW or Cygwin.\n\nSigned-off-by: Andrzej K. Haczewski \n---\n Makefile | 4 +-\n builtin-pack-objects.c | 34 ++++++++++--\n compat/mingw.c | 2 +-\n compat/mingw.h | 5 ++\n compat/win32/pthread.h | 132 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 10 ++++\n preload-index.c | 4 +-\n 7 files changed, 180 insertions(+), 11 deletions(-)\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex 94d44b0..0146ac7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -975,7 +975,7 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n@@ -983,7 +983,7 @@ ifdef MSVC\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..e897b16 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \n #ifdef THREADED_DELTA_SEARCH\n \n-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n #define read_unlock()\t\tpthread_mutex_unlock(&read_mutex)\n \n-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t cache_mutex;\n #define cache_lock()\t\tpthread_mutex_lock(&cache_mutex)\n #define cache_unlock()\t\tpthread_mutex_unlock(&cache_mutex)\n \n-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t progress_mutex;\n #define progress_lock()\t\tpthread_mutex_lock(&progress_mutex)\n #define progress_unlock()\tpthread_mutex_unlock(&progress_mutex)\n \n@@ -1590,9 +1590,28 @@ struct thread_params {\n \tunsigned *processed;\n };\n \n-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n+static pthread_cond_t progress_cond;\n \n-static void *threaded_find_deltas(void *arg)\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_threaded_search()\n+{\n+\tpthread_mutex_init(&read_mutex);\n+\tpthread_mutex_init(&cache_mutex);\n+\tpthread_mutex_init(&progress_mutex);\n+\tpthread_cond_init(&progress_cond, NULL);\n+}\n+\n+static void cleanup_threaded_search()\n+{\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+}\n+\n+static THREAD_RET_TYPE threaded_find_deltas(void *arg)\n {\n \tstruct thread_params *me = arg;\n \n@@ -1620,7 +1639,7 @@ static void *threaded_find_deltas(void *arg)\n \t\tpthread_mutex_unlock(&me->mutex);\n \t}\n \t/* leave ->working 1 so that this doesn't get more work assigned */\n-\treturn NULL;\n+\treturn 0;\n }\n \n static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n@@ -1638,6 +1657,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\t\tdelta_search_threads);\n \tp = xcalloc(delta_search_threads, sizeof(*p));\n \n+\tinit_threaded_search();\n+\n \t/* Partition the work amongst work threads. */\n \tfor (i = 0; i < delta_search_threads; i++) {\n \t\tunsigned sub_size = list_size / (delta_search_threads - i);\n@@ -1745,6 +1766,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\tactive_threads--;\n \t\t}\n \t}\n+\tcleanup_threaded_search();\n \tfree(p);\n }\n \ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6907345..7e25fb5 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -294,3 +294,8 @@ struct mingw_dirent\n #define readdir(x) mingw_readdir(x)\n struct dirent *mingw_readdir(DIR *dir);\n #endif // !NO_MINGW_REPLACE_READDIR\n+\n+/*\n+ * Used by Pthread API implementation for Windows\n+ */\n+extern int err_win_to_posix(DWORD winerr);\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..0e43714\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,132 @@\n+/*\n+ * Header used to adapt pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * Implement simple condition variable for Windows threads, based on ACE\n+ * implementation.\n+ *\n+ * See original implementation: http://bit.ly/1vkDjo\n+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html\n+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html\n+ */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (!cond->sema)\n+\t\treturn 0; /* POSIX do not allow pthread_cond_init to fail */\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait */\n+\tWaitForSingleObject(cond->sema, INFINITE))\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n+\t\t\t0 : err_win_to_posix(GetLastError();\n+\telse\n+\t\treturn 0;\n+}\n+\n+#define pthread_t HANDLE\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+static inline int pthread_create(pthread_t *thread, const void *unused,\n+\t\tDWORD (__stdcall *start_routine)(LPVOID), void *arg)\n+{\n+\t*thread = CreateThread(NULL, 0, start_routine, arg, 0, NULL);\n+\n+\tif (!*thread)\n+\t\treturn err_win_to_posix(GetLastError());\n+\telse\n+\t\treturn 0;\n+}\n+\n+static inline int pthread_join(pthread_t thread, void **unused)\n+{\n+\tDWORD result = WaitForSingleObject(t, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\treturn EINVAL;\n+\t\tdefault:\n+\t\t\treturn err_win_to_posix(GetLastError());\n+\t}\n+}\n+\n+#endif /* PTHREAD_H */\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ef60803..4311117 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -464,4 +464,14 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n */\n int unlink_or_warn(const char *path);\n \n+/*\n+ * Define type of thread function return type to distinguish\n+ * Windows and POSIX.\n+ */\n+#ifndef _WIN32\n+# define THREAD_RET_TYPE void *\n+#else\n+# define THREAD_RET_TYPE DWORD __stdcall\n+#endif\n+\n #endif\ndiff --git a/preload-index.c b/preload-index.c\nindex 9289933..41b11a3 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -28,7 +28,7 @@ struct thread_data {\n \tint offset, nr;\n };\n \n-static void *preload_thread(void *_data)\n+static THREAD_RET_TYPE preload_thread(void* _data)\n {\n \tint nr;\n \tstruct thread_data *p = _data;\n@@ -59,7 +59,7 @@ static void *preload_thread(void *_data)\n \t\t\tcontinue;\n \t\tce_mark_uptodate(ce);\n \t} while (--nr > 0);\n-\treturn NULL;\n+\treturn 0;\n }\n \n static void preload_index(struct index_state *index, const char **pathspec)\n-- \n1.6.5.2\n"},{"id":"126854","messageId":"4AF214D5.6050202@gmail.com","threadId":"21472","inReplyTo":"4AF21283.3080407@gmail.com","subject":"[PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-04T23:57:09Z","receivedAt":"2009-11-04T23:57:09Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"This patch implements native to Windows subset of pthreads API used by Git.\nIt allows to remove Pthreads for Win32 dependency for msysgit and cygwin.\n\nThe patch modifies Makefile only for MSVC (that's the environment I'm\ncapable of testing on), so it requires further corrections to compile\nwith MinGW or Cygwin.\n\nSigned-off-by: Andrzej K. Haczewski \n---\nIgnore previous patch, there was a compile error for POSIX platform\n(pthread_mutex_init takes 2 arguments, MSVC doesn't complain if\n1 argument is supplied to a macro call, but on POSIX it's function\ncall).\n\nI haven't integrated Nicolas patch.\n\n Makefile | 4 +-\n builtin-pack-objects.c | 34 ++++++++++--\n compat/mingw.c | 2 +-\n compat/mingw.h | 5 ++\n compat/win32/pthread.h | 132 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-compat-util.h | 10 ++++\n preload-index.c | 4 +-\n 7 files changed, 180 insertions(+), 11 deletions(-)\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex 94d44b0..0146ac7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -975,7 +975,7 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n@@ -983,7 +983,7 @@ ifdef MSVC\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n \tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..e897b16 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \n #ifdef THREADED_DELTA_SEARCH\n \n-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n #define read_unlock()\t\tpthread_mutex_unlock(&read_mutex)\n \n-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t cache_mutex;\n #define cache_lock()\t\tpthread_mutex_lock(&cache_mutex)\n #define cache_unlock()\t\tpthread_mutex_unlock(&cache_mutex)\n \n-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t progress_mutex;\n #define progress_lock()\t\tpthread_mutex_lock(&progress_mutex)\n #define progress_unlock()\tpthread_mutex_unlock(&progress_mutex)\n \n@@ -1590,9 +1590,28 @@ struct thread_params {\n \tunsigned *processed;\n };\n \n-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n+static pthread_cond_t progress_cond;\n \n-static void *threaded_find_deltas(void *arg)\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_threaded_search()\n+{\n+\tpthread_mutex_init(&read_mutex, NULL);\n+\tpthread_mutex_init(&cache_mutex, NULL);\n+\tpthread_mutex_init(&progress_mutex, NULL);\n+\tpthread_cond_init(&progress_cond, NULL);\n+}\n+\n+static void cleanup_threaded_search()\n+{\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+}\n+\n+static THREAD_RET_TYPE threaded_find_deltas(void *arg)\n {\n \tstruct thread_params *me = arg;\n \n@@ -1620,7 +1639,7 @@ static void *threaded_find_deltas(void *arg)\n \t\tpthread_mutex_unlock(&me->mutex);\n \t}\n \t/* leave ->working 1 so that this doesn't get more work assigned */\n-\treturn NULL;\n+\treturn 0;\n }\n \n static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n@@ -1638,6 +1657,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\t\tdelta_search_threads);\n \tp = xcalloc(delta_search_threads, sizeof(*p));\n \n+\tinit_threaded_search();\n+\n \t/* Partition the work amongst work threads. */\n \tfor (i = 0; i < delta_search_threads; i++) {\n \t\tunsigned sub_size = list_size / (delta_search_threads - i);\n@@ -1745,6 +1766,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\tactive_threads--;\n \t\t}\n \t}\n+\tcleanup_threaded_search();\n \tfree(p);\n }\n \ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6907345..7e25fb5 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -294,3 +294,8 @@ struct mingw_dirent\n #define readdir(x) mingw_readdir(x)\n struct dirent *mingw_readdir(DIR *dir);\n #endif // !NO_MINGW_REPLACE_READDIR\n+\n+/*\n+ * Used by Pthread API implementation for Windows\n+ */\n+extern int err_win_to_posix(DWORD winerr);\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..0e43714\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,132 @@\n+/*\n+ * Header used to adapt pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * Implement simple condition variable for Windows threads, based on ACE\n+ * implementation.\n+ *\n+ * See original implementation: http://bit.ly/1vkDjo\n+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html\n+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html\n+ */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (!cond->sema)\n+\t\treturn 0; /* POSIX do not allow pthread_cond_init to fail */\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait */\n+\tWaitForSingleObject(cond->sema, INFINITE))\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n+\t\t\t0 : err_win_to_posix(GetLastError();\n+\telse\n+\t\treturn 0;\n+}\n+\n+#define pthread_t HANDLE\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+static inline int pthread_create(pthread_t *thread, const void *unused,\n+\t\tDWORD (__stdcall *start_routine)(LPVOID), void *arg)\n+{\n+\t*thread = CreateThread(NULL, 0, start_routine, arg, 0, NULL);\n+\n+\tif (!*thread)\n+\t\treturn err_win_to_posix(GetLastError());\n+\telse\n+\t\treturn 0;\n+}\n+\n+static inline int pthread_join(pthread_t thread, void **unused)\n+{\n+\tDWORD result = WaitForSingleObject(t, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\treturn EINVAL;\n+\t\tdefault:\n+\t\t\treturn err_win_to_posix(GetLastError());\n+\t}\n+}\n+\n+#endif /* PTHREAD_H */\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex ef60803..4311117 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -464,4 +464,14 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n */\n int unlink_or_warn(const char *path);\n \n+/*\n+ * Define type of thread function return type to distinguish\n+ * Windows and POSIX.\n+ */\n+#ifndef _WIN32\n+# define THREAD_RET_TYPE void *\n+#else\n+# define THREAD_RET_TYPE DWORD __stdcall\n+#endif\n+\n #endif\ndiff --git a/preload-index.c b/preload-index.c\nindex 9289933..41b11a3 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -28,7 +28,7 @@ struct thread_data {\n \tint offset, nr;\n };\n \n-static void *preload_thread(void *_data)\n+static THREAD_RET_TYPE preload_thread(void* _data)\n {\n \tint nr;\n \tstruct thread_data *p = _data;\n@@ -59,7 +59,7 @@ static void *preload_thread(void *_data)\n \t\t\tcontinue;\n \t\tce_mark_uptodate(ce);\n \t} while (--nr > 0);\n-\treturn NULL;\n+\treturn 0;\n }\n \n static void preload_index(struct index_state *index, const char **pathspec)\n-- 1.6.5.2 \n"},{"id":"126855","messageId":"7viqdpg7vg.fsf@alter.siamese.dyndns.org","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911041247250.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-04T23:58:27Z","receivedAt":"2009-11-04T23:58:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n>\n>> +\tNO_STATIC_PTHREADS_INIT = YesPlease\n>\n> Let's not go that route please. If Windows can't get away without \n> runtime initializations then let's use them on all platforms. There is \n> no gain in exploding the code path combinations here wrt testing \n> coverage.\n\nHear hear.\n"},{"id":"126859","messageId":"alpine.LFD.2.00.0911041915120.10340@xanadu.home","threadId":"21472","inReplyTo":"4AF214D5.6050202@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T00:22:01Z","receivedAt":"2009-11-05T00:22:01Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> @@ -1638,6 +1657,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n> \t\t\t\tdelta_search_threads);\n> \tp = xcalloc(delta_search_threads, sizeof(*p));\n> \n> +\tinit_threaded_search();\n\nCareful. At the beginning of the function you'll find:\n\n if (delta_search_threads <= 1) {\n find_deltas(list, &list_size, window, depth, processed);\n return;\n }\n\nThat is, if we have thread support compiled in but we're told to use \nonly one thread, then the bulk of the work splitting is bypassed \nentirely. Inside find_deltas() there will still be pthread_mutex_lock() \nand pthread_mutex_unlock() calls even if no threads are spawned.\n\n\nNicolas\n"},{"id":"126860","messageId":"alpine.LFD.2.00.0911041922230.10340@xanadu.home","threadId":"21472","inReplyTo":"alpine.LNX.2.00.0911041640060.14365@iabervon.org","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T00:27:11Z","receivedAt":"2009-11-05T00:27:11Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 4 Nov 2009, Daniel Barkalow wrote:\n\n> On Wed, 4 Nov 2009, Nicolas Pitre wrote:\n> \n> > On Wed, 4 Nov 2009, Daniel Barkalow wrote:\n> > \n> > > On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n> > > \n> > > > 2009/11/4 Johannes Sixt :\n> > > > >\n> > > > > You are right. But #ifdef THREADED_DELTA_SEARCH is about a \"generic\"\n> > > > > property of the code and is already used elsewhere in the file, whereas\n> > > > > #ifdef WIN32 would be new and is is about platform differences.\n> > > > >\n> > > > > Anyway, we would have to see what Junio says about the new function calls,\n> > > > > because he's usually quite anal when it comes to added code vs. static\n> > > > > initialization. ;)\n> > > > \n> > > > I could do it with wrappers for pthread_mutex_lock and _unlock and\n> > > > lazy init there plus lazy init cond var in cond_wait and _signal, that\n> > > > way it could be done without any additional code in the first #ifdef.\n> > > > But I don't see any simple solution for working around\n> > > > deinitialization, that's why I'd leave non-static initialization. Let\n> > > > me put some touchups and resubmit for another round.\n> > > \n> > > Is it actually necessary to deinitialize? Since the variables are static \n> > > and therefore can't leak, and would presumably not need to be \n> > > reinitialized differently if they were used again, I think they should be \n> > > able to just stay. If Windows is unhappy about processes still having \n> > > locks initialized at exit, I suppose we could go through and destroy all \n> > > our mutexes and conds at cleanup time. Pthreads does have the appropriate \n> > > functions, and it would be correct to use them, although unnecessary.\n> > \n> > Lazy initialization would probably turn up to be more expensive \n> > (checking a flag on each usage) than unconditionally initializing them \n> > once. Remember that those are used at least once per object meaning a \n> > lot.\n> \n> Meh, checking a flag on the same cache line as the lock you're about to \n> take can't be a big incremental cost, especially if it's actually checking \n> whether some sort of cookie is non-zero before doing something with it.\n\nThis is still a bigger cost than not checking such flag at all. \nEspecially if the check will be false on every call but the first one \nout of millions. I agree this is not significant, but neither is a \nruntime initialization vs a static one.\n\n> I don't think it matters terribly much either way which we use, so long as \n> its consistent. It'd be nice if the static initializers worked, just \n> because people seem to write code with them, but we could just not do that \n> in the future.\n\nMaybe the static initializer can be turned into a global constructor on \nWindows?\n\n\nNicolas\n"},{"id":"126869","messageId":"alpine.LFD.2.00.0911042039200.10340@xanadu.home","threadId":"21472","inReplyTo":"4AF214D5.6050202@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T02:10:46Z","receivedAt":"2009-11-05T02:10:46Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> +static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n> +{\n> +\tcond->waiters = 0;\n> +\n> +\tInitializeCriticalSection(&cond->waiters_lock);\n> +\n> +\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n> +\tif (!cond->sema)\n> +\t\treturn 0; /* POSIX do not allow pthread_cond_init to fail */\n> +\treturn 0;\n> +}\n\nPlease use die(\"CreateSemaphore() failed\") in the failure case instead \nof returning success.\n\nHowever, my pthread_cond_init man page says:\n\n[[[\nRETURN VALUE\n If successful, the pthread_cond_destroy() and pthread_cond_init() func-\n tions shall return zero; otherwise, an error number shall be returned\n to indicate the error.\n\n The [EBUSY] and [EINVAL] error checks, if implemented, shall act as if\n they were performed immediately at the beginning of processing for the\n function and caused an error return prior to modifying the state of the\n condition variable specified by cond.\n\nERRORS\n The pthread_cond_destroy() function may fail if:\n\n EBUSY The implementation has detected an attempt to destroy the object\n referenced by cond while it is referenced (for example, while\n being used in a pthread_cond_wait() or pthread_cond_timedwait())\n by another thread.\n\n EINVAL The value specified by cond is invalid.\n\n The pthread_cond_init() function shall fail if:\n\n EAGAIN The system lacked the necessary resources (other than memory) to\n initialize another condition variable.\n\n ENOMEM Insufficient memory exists to initialize the condition variable.\n\n The pthread_cond_init() function may fail if:\n\n EBUSY The implementation has detected an attempt to reinitialize the\n object referenced by cond, a previously initialized, but not yet\n destroyed, condition variable.\n\n EINVAL The value specified by attr is invalid.\n]]]\n\nI'm not advocating that you implement detailed error codes as we don't \nreally care about specific errors. This is just to disagree with the \n\"POSIX do not allow pthread_cond_init to fail\" assertion. In any case, \nusing die() to keep it simple is certainly better than blindly returning \n0 on failure. However you could simply return ENOMEM and use the die() \nin init_threaded_search() instead.\n\n\nNicolas\n"},{"id":"126871","messageId":"alpine.LFD.2.00.0911042111270.10340@xanadu.home","threadId":"21472","inReplyTo":"4AF20534.2030004@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T02:47:20Z","receivedAt":"2009-11-05T02:47:20Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n\n> Erik Faye-Lund pisze:\n> > Couldn't the windows version of pthread_create have a wrapper\n> > function, that corrected the calling convention, much like the\n> > function run_thread that start_async in run-command.c has?\n> \n> Can't be done without allocations. I'd have to pass to that wrapping\n> thread function an address of original function *and* an original\n> argument, and there's no way to pack that as one void*.\n\nWhat about:\n\ntypedef struct {\n\tHANDLE handle;\n\tvoid *(*start_routine)(void *);\n\tvoid *arg;\n} pthread_t;\n\nDWORD __stdcall windows_thread_start(LPVOID _self)\n{\n\tpthread_t *self = _self;\n\tvoid *ret = self->start_routine(self->arg);\n\treturn (DWORD)ret;\n}\n\nstatic inline int pthread_create(pthread_t *thread, const void *unused,\n void *(*start_routine)(void *), void *arg)\n{\n thread->handle = CreateThread(NULL, 0, windows_thread_start, \n thread, 0, NULL);\n [...]\n}\n\n?\n\nSure this will use 8 to 16 more bytes per thread, but we're dealing with \na rather small number of threads anyway (more threads than the number of \nCPU cores is useless) making this extra memory usage rather \ninsignificant compared to the many megabytes of RAM the rest of the code \nis using. The advantage is full compatibility with the native pthread \ninterface git is using at the source level while still being much \nlighter than a full blown pthread implementation.\n\nAnd thread creation is a relatively rare event compared to e.g. mutex \nlock/unlock, so the indirection shouldn't be noticeable. For the same \nreason, I also think that you could make pthread_create() and \npthread_join() into a C file instead of being inlined which would reduce \nthe code footprint at every call site, and allow for only one instance \nof windows_thread_start() which could then be made static.\n\n\nNicolas\n"},{"id":"126882","messageId":"4AF27FBD.10203@viscovery.net","threadId":"21472","inReplyTo":"4AF214D5.6050202@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-05T07:33:17Z","receivedAt":"2009-11-05T07:33:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andrzej K. Haczewski schrieb:\n> This patch implements native to Windows subset of pthreads API used by Git.\n> It allows to remove Pthreads for Win32 dependency for msysgit and cygwin.\n> \n> The patch modifies Makefile only for MSVC (that's the environment I'm\n> capable of testing on), so it requires further corrections to compile\n> with MinGW or Cygwin.\n\nLooks quite good already.\n\nIn the next round, please squash this in.\n\ndiff --git a/Makefile b/Makefile\nindex bae1b40..6648d11 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -414,6 +414,7 @@ LIB_H += cache-tree.h\n LIB_H += commit.h\n LIB_H += compat/cygwin.h\n LIB_H += compat/mingw.h\n+LIB_H += compat/win32/pthread.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n"},{"id":"126892","messageId":"16cee31f0911050045t7a7301cdm39114997edfbfa60@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911042039200.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T08:45:15Z","receivedAt":"2009-11-05T08:45:15Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/5 Nicolas Pitre :\n> Please use die(\"CreateSemaphore() failed\") in the failure case instead\n> of returning success.\n\nSure, will do.\n\n> However, my pthread_cond_init man page says:\n\nAnd that is weird, because mine man page says:\n[[[\npthread_cond_init, pthread_cond_signal, pthread_cond_broadcast, and\npthread_cond_wait never return an error code.\n]]]\n\n--\nAndrzej\n"},{"id":"126893","messageId":"16cee31f0911050051m4cd29827nca9c8238b21461a0@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911041915120.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T08:51:22Z","receivedAt":"2009-11-05T08:51:22Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/5 Nicolas Pitre :\n> Careful.  At the beginning of the function you'll find:\n>\n>        if (delta_search_threads <= 1) {\n>                find_deltas(list, &list_size, window, depth, processed);\n>                return;\n>        }\n>\n> That is, if we have thread support compiled in but we're told to use\n> only one thread, then the bulk of the work splitting is bypassed\n> entirely.  Inside find_deltas() there will still be pthread_mutex_lock()\n> and pthread_mutex_unlock() calls even if no threads are spawned.\n\nAh, I wasn't aware of that. Actually why would find_deltas lock if no\nthreads are used? Maybe, for non-threaded call to find_deltas, locking\ncould be factored out?\n\n--\nAndrzej\n"},{"id":"126894","messageId":"16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911042111270.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T09:00:03Z","receivedAt":"2009-11-05T09:00:03Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/5 Nicolas Pitre :\n> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n>\n> What about:\n>\n> typedef struct {\n>        HANDLE handle;\n>        void *(*start_routine)(void *);\n>        void *arg;\n> } pthread_t;\n>\n> DWORD __stdcall windows_thread_start(LPVOID _self)\n> {\n>        pthread_t *self = _self;\n>        void *ret = self->start_routine(self->arg);\n>        return (DWORD)ret;\n> }\n>\n> static inline int pthread_create(pthread_t *thread, const void *unused,\n>                                 void *(*start_routine)(void *), void *arg)\n> {\n>        thread->handle = CreateThread(NULL, 0, windows_thread_start,\n>                                      thread, 0, NULL);\n>        [...]\n> }\n\nThe problem I see is not with pthread_init, but pthread_join. Here's\nhow it looks:\n\nint pthread_join(pthread_t thread, void **value_ptr);\n\nIf pthread_t would be a struct, then we can't call pthread_join like\nthat... At least that's what I though yesterday, but maybe it can be\ndone like this:\n\nint win32_pthread_join(pthread_t *thread, void **value_ptr)\n{\n [...]\n}\n\n#define pthread_join(a, b) win32_pthread_join(&(a), (b))\n\nThat way we don't need allocations to simulate pthread init/join API\n\n> And thread creation is a relatively rare event compared to e.g. mutex\n> lock/unlock, so the indirection shouldn't be noticeable.  For the same\n> reason, I also think that you could make pthread_create() and\n> pthread_join() into a C file instead of being inlined which would reduce\n> the code footprint at every call site, and allow for only one instance\n> of windows_thread_start() which could then be made static.\n\nYeah, I'll factor that out to separate file.\n\n--\nAndrzej\n"},{"id":"126896","messageId":"40aa078e0911050141t3c85e14k91692ac5c542eb77@mail.gmail.com","threadId":"21472","inReplyTo":"16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-11-05T09:41:51Z","receivedAt":"2009-11-05T09:41:51Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Nov 5, 2009 at 10:00 AM, Andrzej K. Haczewski\n wrote:\n>\n> That way we don't need allocations to simulate pthread init/join API\n\nYay! By the way, I love the work you're doing here. Getting threaded\ndelta-searching on Windows is something I'm looking forward to :)\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"126900","messageId":"1257416325-5605-1-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":"16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com","subject":"[PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T10:18:45Z","receivedAt":"2009-11-05T10:18:45Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"This patch implements native to Windows subset of pthreads API used by Git.\nIt allows to remove Pthreads for Win32 dependency for msysgit and Cygwin.\n\nThe patch modifies Makefile only for MSVC (that's the environment I'm\ncapable of testing on), so it requires further corrections to compile\nwith MinGW or Cygwin.\n\nSigned-off-by: Andrzej K. Haczewski \n---\n Makefile | 7 ++-\n builtin-pack-objects.c | 31 ++++++++++--\n compat/mingw.c | 2 +-\n compat/mingw.h | 5 ++\n compat/win32/pthread.c | 39 +++++++++++++++\n compat/win32/pthread.h | 125 ++++++++++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 201 insertions(+), 8 deletions(-)\n create mode 100644 compat/win32/pthread.c\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex bc039ac..30089a8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -453,6 +453,7 @@ LIB_H += commit.h\n LIB_H += compat/bswap.h\n LIB_H += compat/cygwin.h\n LIB_H += compat/mingw.h\n+LIB_H += compat/win32/pthread.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n@@ -971,15 +972,15 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n \tAR = compat/vcbuild/scripts/lib.pl\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n-\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..00594fd 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \n #ifdef THREADED_DELTA_SEARCH\n \n-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n #define read_unlock()\t\tpthread_mutex_unlock(&read_mutex)\n \n-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t cache_mutex;\n #define cache_lock()\t\tpthread_mutex_lock(&cache_mutex)\n #define cache_unlock()\t\tpthread_mutex_unlock(&cache_mutex)\n \n-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t progress_mutex;\n #define progress_lock()\t\tpthread_mutex_lock(&progress_mutex)\n #define progress_unlock()\tpthread_mutex_unlock(&progress_mutex)\n \n@@ -1590,7 +1590,26 @@ struct thread_params {\n \tunsigned *processed;\n };\n \n-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n+static pthread_cond_t progress_cond;\n+\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_threaded_search()\n+{\n+\tpthread_mutex_init(&read_mutex, NULL);\n+\tpthread_mutex_init(&cache_mutex, NULL);\n+\tpthread_mutex_init(&progress_mutex, NULL);\n+\tpthread_cond_init(&progress_cond, NULL);\n+}\n+\n+static void cleanup_threaded_search()\n+{\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+}\n \n static void *threaded_find_deltas(void *arg)\n {\n@@ -1629,8 +1648,11 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tstruct thread_params *p;\n \tint i, ret, active_threads = 0;\n \n+\tinit_threaded_search();\n+\n \tif (delta_search_threads <= 1) {\n \t\tfind_deltas(list, &list_size, window, depth, processed);\n+\t\tcleanup_threaded_search();\n \t\treturn;\n \t}\n \tif (progress > pack_to_stdout)\n@@ -1745,6 +1767,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\tactive_threads--;\n \t\t}\n \t}\n+\tcleanup_threaded_search();\n \tfree(p);\n }\n \ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6907345..7e25fb5 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -294,3 +294,8 @@ struct mingw_dirent\n #define readdir(x) mingw_readdir(x)\n struct dirent *mingw_readdir(DIR *dir);\n #endif // !NO_MINGW_REPLACE_READDIR\n+\n+/*\n+ * Used by Pthread API implementation for Windows\n+ */\n+extern int err_win_to_posix(DWORD winerr);\ndiff --git a/compat/win32/pthread.c b/compat/win32/pthread.c\nnew file mode 100644\nindex 0000000..a7ab7af\n--- /dev/null\n+++ b/compat/win32/pthread.c\n@@ -0,0 +1,39 @@\n+/*\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#include \"pthread.h\"\n+\n+static DWORD __stdcall win32_start_routine(LPVOID arg)\n+{\n+\tpthread_t *thread = arg;\n+\tthread->value = thread->start_routine(thread->arg);\n+\treturn 0;\n+}\n+\n+int pthread_create(pthread_t *thread, const void *unused,\n+\t\t void *(*start_routine)(void*), void *arg)\n+{\n+\tthread->arg = arg;\n+\tthread->handle = CreateThread(NULL, 0, win32_start_routine, thread, 0, NULL);\n+\n+\tif (!thread->handle)\n+\t\treturn err_win_to_posix(GetLastError());\n+\telse\n+\t\treturn 0;\n+}\n+\n+int win32_pthread_join(pthread_t *thread, void **value_ptr)\n+{\n+\tDWORD result = WaitForSingleObject(thread->handle, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\tif (value_ptr)\n+\t\t\t\t*value_ptr = thread.value;\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\treturn EINVAL;\n+\t\tdefault:\n+\t\t\treturn err_win_to_posix(GetLastError());\n+\t}\n+}\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..e50eb6b\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,125 @@\n+/*\n+ * Header used to adapt pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * Implement simple condition variable for Windows threads, based on ACE\n+ * implementation.\n+ *\n+ * See original implementation: http://bit.ly/1vkDjo\n+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html\n+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html\n+ */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (!cond->sema)\n+\t\tdie(\"\");\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait */\n+\tWaitForSingleObject(cond->sema, INFINITE))\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn 0;\n+}\n+\n+static inline int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n+\t\t\t0 : err_win_to_posix(GetLastError();\n+\telse\n+\t\treturn 0;\n+}\n+\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+typedef struct {\n+\tHANDLE handle;\n+\tvoid *(*start_routine)(void*);\n+\tvoid *arg;\n+\tvoid *value;\n+} pthread_t;\n+\n+extern int pthread_create(pthread_t *thread, const void *unused,\n+\t\t\t void *(*start_routine)(void*), void *arg);\n+\n+/*\n+ * To avoid the need of allocating struct, we use small macro wrapper to pass\n+ * pointer to win32_pthread_join instead of using typedef struct {} *pthread_t\n+ */\n+#define pthread_join(a, b) win32_pthread_join(&(a), (b))\n+\n+extern int win32_pthread_join(pthread_t *thread, void **value_ptr);\n+\n+#endif /* PTHREAD_H */\n-- \n1.6.5.2\n"},{"id":"126903","messageId":"4AF2C4B9.10402@viscovery.net","threadId":"21472","inReplyTo":"1257416325-5605-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-05T12:27:37Z","receivedAt":"2009-11-05T12:27:37Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andrzej K. Haczewski schrieb:\n> diff --git a/Makefile b/Makefile\n> index bc039ac..30089a8 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -453,6 +453,7 @@ LIB_H += commit.h\n> LIB_H += compat/bswap.h\n> LIB_H += compat/cygwin.h\n> LIB_H += compat/mingw.h\n> +LIB_H += compat/win32/pthread.h\n> LIB_H += csum-file.h\n> LIB_H += decorate.h\n> LIB_H += delta.h\n> @@ -971,15 +972,15 @@ ifdef MSVC\n> \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n> \tNO_REGEX = YesPlease\n> \tNO_CURL = YesPlease\n> -\tNO_PTHREADS = YesPlease\n> +\tTHREADED_DELTA_SEARCH = YesPlease\n> \tBLK_SHA1 = YesPlease\n> \n> \tCC = compat/vcbuild/scripts/clink.pl\n> \tAR = compat/vcbuild/scripts/lib.pl\n> \tCFLAGS =\n> \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n> -\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n> -\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n> +\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o\n> +\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n> \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n> \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n> \tlib =\n\nWhat compiles compat/win32/pthread.c?\n\nPlease don't forget to add compat/win32/*.o to the clean target.\n\n> +int pthread_create(pthread_t *thread, const void *unused,\n> +\t\t void *(*start_routine)(void*), void *arg)\n> +{\n> +\tthread->arg = arg;\n> +\tthread->handle = CreateThread(NULL, 0, win32_start_routine, thread, 0, NULL);\n\nElsewhere we use _beginthreadex(). What's the difference?\n\n> +static inline int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n> +{\n> +\tcond->waiters = 0;\n> +\n> +\tInitializeCriticalSection(&cond->waiters_lock);\n> +\n> +\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n> +\tif (!cond->sema)\n> +\t\tdie(\"\");\n> +\treturn 0;\n> +}\n> +\n> +static inline int pthread_cond_destroy(pthread_cond_t *cond)\n> +{\n> +\tCloseHandle(cond->sema);\n> +\tcond->sema = NULL;\n> +\n> +\tDeleteCriticalSection(&cond->waiters_lock);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static inline int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n> +{\n> +\t/* serialize access to waiters count */\n> +\tEnterCriticalSection(&cond->waiters_lock);\n> +\t++cond->waiters;\n> +\tLeaveCriticalSection(&cond->waiters_lock);\n> +\n> +\t/*\n> +\t * Unlock external mutex and wait for signal.\n> +\t * NOTE: we've held mutex locked long enough to increment\n> +\t * waiters count above, so there's no problem with\n> +\t * leaving mutex unlocked before we wait on semaphore.\n> +\t */\n> +\tLeaveCriticalSection(mutex);\n> +\n> +\t/* let's wait */\n> +\tWaitForSingleObject(cond->sema, INFINITE))\n> +\n> +\t/* we're done waiting, so make sure we decrease waiters count */\n> +\tEnterCriticalSection(&cond->waiters_lock);\n> +\t--cond->waiters;\n> +\tLeaveCriticalSection(&cond->waiters_lock);\n> +\n> +\t/* lock external mutex again */\n> +\tEnterCriticalSection(mutex);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static inline int pthread_cond_signal(pthread_cond_t *cond)\n> +{\n> +\tint have_waiters;\n> +\n> +\t/* serialize access to waiters count */\n> +\tEnterCriticalSection(&cond->waiters_lock);\n> +\thave_waiters = cond->waiters > 0;\n> +\tLeaveCriticalSection(&cond->waiters_lock);\n> +\n> +\t/*\n> +\t * Signal only when there are waiters\n> +\t */\n> +\tif (have_waiters)\n> +\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n> +\t\t\t0 : err_win_to_posix(GetLastError();\n> +\telse\n> +\t\treturn 0;\n> +}\n\nThe pthread_cond_* functions are quite voluminous, but not performance\ncritical. Could you please move them to pthread.c as well?\n\n-- Hannes\n"},{"id":"126904","messageId":"16cee31f0911050453w67ef7186l72e22f32a8d8092d@mail.gmail.com","threadId":"21472","inReplyTo":"4AF2C4B9.10402@viscovery.net","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T12:53:09Z","receivedAt":"2009-11-05T12:53:09Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/5 Johannes Sixt :\n> Elsewhere we use _beginthreadex(). What's the difference?\n\nOh my, I've just run through MSDN documentation and I'm wrongly using\nCreateThread. I should have used _beginthread from the start, because\nbesides calling CreateThread it initializes local thread storage for\nglobal C-runtime variables (errno etc.), which CreateThread does not\ndo. That could lead to very unpleasant consequences. I'll redo thread\ncreation to use _beginthreadex(). Thanks for that question!\n\n> The pthread_cond_* functions are quite voluminous, but not performance\n> critical. Could you please move them to pthread.c as well?\n\nOk, will do.\n\n--\nAndrzej\n"},{"id":"126906","messageId":"20091105134858.GG27126@dpotapov.dyndns.org","threadId":"21472","inReplyTo":"16cee31f0911040547m69e5b9cbi30e20d2a7790bd6f@mail.gmail.com","subject":"Re: [PATCH] MSVC: port pthread code to native Windows threads","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2009-11-05T13:48:58Z","receivedAt":"2009-11-05T13:48:58Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Wed, Nov 04, 2009 at 02:47:09PM +0100, Andrzej K. Haczewski wrote:\n> 2009/11/4 Johannes Sixt :\n> \n> > - pthread_cond_signal is called while the mutex is held.\n> \n> AFAIK that is a requirement for condition variable to be signaled\n> while holding the same mutex that other threads cond_wait on. I just\n> don't check that it is true, because Git is locking mutex.\n\nThere is no such requirement in POSIX:\n\n The pthread_cond_broadcast() or pthread_cond_signal() functions may\n be called by a thread whether or not it currently owns the mutex that\n threads calling pthread_cond_wait() or pthread_cond_timedwait() have\n associated with the condition variable during their waits; however,\n if predictable scheduling behavior is required, then that mutex shall\n be locked by the thread calling pthread_cond_broadcast() or\n pthread_cond_signal().\n\nhttp://www.opengroup.org/onlinepubs/009695399/functions/pthread_cond_signal.html\n\n\n\nDmitry\n"},{"id":"126907","messageId":"1257439548-9258-1-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":"1257283802-29726-1-git-send-email-ahaczewski@gmail.com","subject":"[PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T16:45:48Z","receivedAt":"2009-11-05T16:45:48Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"This patch implements native to Windows subset of pthreads API used by Git.\nIt allows to remove Pthreads for Win32 dependency for MSVC, msysgit and\nCygwin.\n\nThe patch modifies Makefile only for MSVC (that's the environment I'm\ncapable of testing on), so it requires further corrections to compile\nwith MinGW or Cygwin.\n\nSigned-off-by: Andrzej K. Haczewski \n---\nHere is another round of that patch with all comments considered. There is\nno workaround to make static initialization of mutexes and condition\nvariables work for Windows, that's why there's explicit initialization\nadded.\n\nI hope I added to Cc all interested in this patch. Excuse me if I omitted\nsomeone.\n\n Makefile | 7 ++-\n builtin-pack-objects.c | 31 +++++++++++--\n compat/mingw.c | 2 +-\n compat/mingw.h | 5 ++\n compat/win32/pthread.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++\n compat/win32/pthread.h | 69 ++++++++++++++++++++++++++++\n 6 files changed, 222 insertions(+), 8 deletions(-)\n create mode 100644 compat/win32/pthread.c\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex bc039ac..30089a8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -453,6 +453,7 @@ LIB_H += commit.h\n LIB_H += compat/bswap.h\n LIB_H += compat/cygwin.h\n LIB_H += compat/mingw.h\n+LIB_H += compat/win32/pthread.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n@@ -971,15 +972,15 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n \tAR = compat/vcbuild/scripts/lib.pl\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n-\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..00594fd 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \n #ifdef THREADED_DELTA_SEARCH\n \n-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n #define read_unlock()\t\tpthread_mutex_unlock(&read_mutex)\n \n-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t cache_mutex;\n #define cache_lock()\t\tpthread_mutex_lock(&cache_mutex)\n #define cache_unlock()\t\tpthread_mutex_unlock(&cache_mutex)\n \n-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t progress_mutex;\n #define progress_lock()\t\tpthread_mutex_lock(&progress_mutex)\n #define progress_unlock()\tpthread_mutex_unlock(&progress_mutex)\n \n@@ -1590,7 +1590,26 @@ struct thread_params {\n \tunsigned *processed;\n };\n \n-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n+static pthread_cond_t progress_cond;\n+\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_threaded_search()\n+{\n+\tpthread_mutex_init(&read_mutex, NULL);\n+\tpthread_mutex_init(&cache_mutex, NULL);\n+\tpthread_mutex_init(&progress_mutex, NULL);\n+\tpthread_cond_init(&progress_cond, NULL);\n+}\n+\n+static void cleanup_threaded_search()\n+{\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+}\n \n static void *threaded_find_deltas(void *arg)\n {\n@@ -1629,8 +1648,11 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tstruct thread_params *p;\n \tint i, ret, active_threads = 0;\n \n+\tinit_threaded_search();\n+\n \tif (delta_search_threads <= 1) {\n \t\tfind_deltas(list, &list_size, window, depth, processed);\n+\t\tcleanup_threaded_search();\n \t\treturn;\n \t}\n \tif (progress > pack_to_stdout)\n@@ -1745,6 +1767,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\tactive_threads--;\n \t\t}\n \t}\n+\tcleanup_threaded_search();\n \tfree(p);\n }\n \ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6907345..7e25fb5 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -294,3 +294,8 @@ struct mingw_dirent\n #define readdir(x) mingw_readdir(x)\n struct dirent *mingw_readdir(DIR *dir);\n #endif // !NO_MINGW_REPLACE_READDIR\n+\n+/*\n+ * Used by Pthread API implementation for Windows\n+ */\n+extern int err_win_to_posix(DWORD winerr);\ndiff --git a/compat/win32/pthread.c b/compat/win32/pthread.c\nnew file mode 100644\nindex 0000000..e4d21bd\n--- /dev/null\n+++ b/compat/win32/pthread.c\n@@ -0,0 +1,116 @@\n+/*\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#include \"../../git-compat-util.h\"\n+#include \"pthread.h\"\n+\n+#include \n+#include \n+\n+static unsigned __stdcall win32_start_routine(void *arg)\n+{\n+\tpthread_t *thread = arg;\n+\tthread->value = thread->start_routine(thread->arg);\n+\treturn 0;\n+}\n+\n+int pthread_create(pthread_t *thread, const void *unused,\n+\t\t void *(*start_routine)(void*), void *arg)\n+{\n+\tthread->arg = arg;\n+\tthread->start_routine = start_routine;\n+\tthread->value = NULL;\n+\tthread->handle =\n+\t\t_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);\n+\n+\tif (!thread->handle)\n+\t\treturn errno;\n+\telse\n+\t\treturn 0;\n+}\n+\n+int win32_pthread_join(pthread_t *thread, void **value_ptr)\n+{\n+\tDWORD result = WaitForSingleObject((HANDLE)thread->handle, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\tif (value_ptr)\n+\t\t\t\t*value_ptr = thread->value;\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\treturn EINVAL;\n+\t\tdefault:\n+\t\t\treturn err_win_to_posix(GetLastError());\n+\t}\n+}\n+\n+int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (!cond->sema)\n+\t\tdie(\"CreateSemaphore() failed\");\n+\treturn 0;\n+}\n+\n+int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait - ignore return value */\n+\tWaitForSingleObject(cond->sema, INFINITE);\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn 0;\n+}\n+\n+int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n+\t\t\t0 : err_win_to_posix(GetLastError());\n+\telse\n+\t\treturn 0;\n+}\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..a7594cc\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,69 @@\n+/*\n+ * Header used to adapt pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * Defines that adapt Windows API threads to pthreads API\n+ */\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+/*\n+ * Implement simple condition variable for Windows threads, based on ACE\n+ * implementation.\n+ *\n+ * See original implementation: http://bit.ly/1vkDjo\n+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html\n+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html\n+ */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+extern int pthread_cond_init(pthread_cond_t *cond, const void *unused);\n+\n+extern int pthread_cond_destroy(pthread_cond_t *cond);\n+\n+extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);\n+\n+extern int pthread_cond_signal(pthread_cond_t *cond);\n+\n+/*\n+ * Simple thread creation implementation using pthread API\n+ */\n+typedef struct {\n+\tuintptr_t handle;\n+\tvoid *(*start_routine)(void*);\n+\tvoid *arg;\n+\tvoid *value;\n+} pthread_t;\n+\n+extern int pthread_create(pthread_t *thread, const void *unused,\n+\t\t\t void *(*start_routine)(void*), void *arg);\n+\n+/*\n+ * To avoid the need of allocating struct, we use small macro wrapper to pass\n+ * pointer to win32_pthread_join instead of using typedef struct {} *pthread_t\n+ */\n+#define pthread_join(a, b) win32_pthread_join(&(a), (b))\n+\n+extern int win32_pthread_join(pthread_t *thread, void **value_ptr);\n+\n+#endif /* PTHREAD_H */\n-- \n1.6.5.2\n"},{"id":"126908","messageId":"4AF30BFD.4070101@viscovery.net","threadId":"21472","inReplyTo":"1257439548-9258-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-05T17:31:41Z","receivedAt":"2009-11-05T17:31:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Thanks.\n\nI pushed this to\n\n git://repo.or.cz/git/mingw/j6t.git pthreads-for-windows\n\ntogether with Nico's patch and another patch to enable pthreads on\nMinGW unconditionally. Feedback welcome!\n\nAndrzej K. Haczewski (1):\n MSVC: Windows-native implementation for subset of Pthreads API\n\nJohannes Sixt (1):\n MinGW: enable pthreads\n\nNicolas Pitre (1):\n pack-objects: move thread autodetection closer to relevant code\n\nThis is my patch:\n\n--- >8 ---\nFrom: Johannes Sixt \nSubject: [PATCH] MinGW: enable pthreads\n\nIf the MinGW build was built as part of the msysgit build environment,\nthen threading was already enabled because the pthreads package from\nGNU-Win32 is available in msysgit.\n\nThe previous patch added a minimal pthreads implementation for Windows.\nTherefore, we can now enable code that uses pthreads unconditionally.\n\nSigned-off-by: Johannes Sixt \n---\n Makefile | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex db7ffb0..4b8603a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -986,9 +986,11 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tUNRELIABLE_FSTAT = UnfortunatelyYes\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n-\tCOMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o\n+\tCOMPAT_OBJS += compat/mingw.o compat/fnmatch/fnmatch.o compat/winansi.o \\\n+\t\tcompat/win32/pthread.o\n \tEXTLIBS += -lws2_32\n \tX = .exe\n ifneq (,$(wildcard ../THIS_IS_MSYSGIT))\n@@ -998,10 +1000,8 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT))\n \tEXTLIBS += /mingw/lib/libz.a\n \tNO_R_TO_GCC_LINKER = YesPlease\n \tINTERNAL_QSORT = YesPlease\n-\tTHREADED_DELTA_SEARCH = YesPlease\n else\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n endif\n endif\n endif\n-- \n1.6.5.2.1198.ge698c\n"},{"id":"126913","messageId":"alpine.LFD.2.00.0911051417090.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911050045t7a7301cdm39114997edfbfa60@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T19:17:59Z","receivedAt":"2009-11-05T19:17:59Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> > However, my pthread_cond_init man page says:\n> \n> And that is weird, because mine man page says:\n> [[[\n> pthread_cond_init, pthread_cond_signal, pthread_cond_broadcast, and\n> pthread_cond_wait never return an error code.\n> ]]]\n\nMaybe that's for a particular implementation.\n\n\nNicolas\n"},{"id":"126914","messageId":"alpine.LFD.2.00.0911051418190.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911050051m4cd29827nca9c8238b21461a0@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T19:22:00Z","receivedAt":"2009-11-05T19:22:00Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> 2009/11/5 Nicolas Pitre :\n> > Careful.  At the beginning of the function you'll find:\n> >\n> >        if (delta_search_threads <= 1) {\n> >                find_deltas(list, &list_size, window, depth, processed);\n> >                return;\n> >        }\n> >\n> > That is, if we have thread support compiled in but we're told to use\n> > only one thread, then the bulk of the work splitting is bypassed\n> > entirely.  Inside find_deltas() there will still be pthread_mutex_lock()\n> > and pthread_mutex_unlock() calls even if no threads are spawned.\n> \n> Ah, I wasn't aware of that. Actually why would find_deltas lock if no\n> threads are used? Maybe, for non-threaded call to find_deltas, locking\n> could be factored out?\n\nIt is already factored out when thread support is not enabled.\n\nWhen thread support is enabled but there is only one thread, there was \nno point duplicating the code just to have a path without any mutexes, \nespecially on Linux where no performance difference could be measured.\n\n\nNicolas\n"},{"id":"126915","messageId":"alpine.LFD.2.00.0911051422590.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911050100v76316dacye7edd8718a893f01@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T19:25:27Z","receivedAt":"2009-11-05T19:25:27Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> 2009/11/5 Nicolas Pitre :\n> > On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:\n> >\n> > What about:\n> >\n> > typedef struct {\n> >        HANDLE handle;\n> >        void *(*start_routine)(void *);\n> >        void *arg;\n> > } pthread_t;\n> >\n> > DWORD __stdcall windows_thread_start(LPVOID _self)\n> > {\n> >        pthread_t *self = _self;\n> >        void *ret = self->start_routine(self->arg);\n> >        return (DWORD)ret;\n> > }\n> >\n> > static inline int pthread_create(pthread_t *thread, const void *unused,\n> >                                 void *(*start_routine)(void *), void *arg)\n> > {\n> >        thread->handle = CreateThread(NULL, 0, windows_thread_start,\n> >                                      thread, 0, NULL);\n> >        [...]\n> > }\n> \n> The problem I see is not with pthread_init, but pthread_join. Here's\n> how it looks:\n> \n> int pthread_join(pthread_t thread, void **value_ptr);\n> \n> If pthread_t would be a struct, then we can't call pthread_join like\n> that...\n\nWhy not? At least gcc is quite happy with such a construct. It \nprobably makes a copy of the stack before passing it though.\n\n> At least that's what I though yesterday, but maybe it can be done like \n> this:\n> \n> int win32_pthread_join(pthread_t *thread, void **value_ptr)\n> {\n> [...]\n> }\n> \n> #define pthread_join(a, b) win32_pthread_join(&(a), (b))\n> \n> That way we don't need allocations to simulate pthread init/join API\n\nRight.\n\n\nNicolas\n"},{"id":"126917","messageId":"alpine.LFD.2.00.0911051434090.10340@xanadu.home","threadId":"21472","inReplyTo":"1257439548-9258-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T19:39:37Z","receivedAt":"2009-11-05T19:39:37Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> +static unsigned __stdcall win32_start_routine(void *arg)\n> +{\n> +\tpthread_t *thread = arg;\n> +\tthread->value = thread->start_routine(thread->arg);\n> +\treturn 0;\n> +}\n\nI suppose you could reuse thread->arg for both the argument and the \nreturned value to save a word.\n\n> +int win32_pthread_join(pthread_t *thread, void **value_ptr)\n> +{\n> +\tDWORD result = WaitForSingleObject((HANDLE)thread->handle, INFINITE);\n\nWhy are you casting thread->handle here? Why not simply declaring it as \na HANDLE?\n\nOtherwise this looks pretty good now.\n\n\nNicolas\n"},{"id":"126919","messageId":"16cee31f0911051209jcc490e2pabeab9a1b6e3b11d@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911051434090.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T20:09:47Z","receivedAt":"2009-11-05T20:09:47Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/5 Nicolas Pitre :\n> On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n>\n>> +static unsigned __stdcall win32_start_routine(void *arg)\n>> +{\n>> +     pthread_t *thread = arg;\n>> +     thread->value = thread->start_routine(thread->arg);\n>> +     return 0;\n>> +}\n>\n> I suppose you could reuse thread->arg for both the argument and the\n> returned value to save a word.\n\nYou're right! J6t committed already, what can I do now?\n\n> Why are you casting thread->handle here?  Why not simply declaring it as\n> a HANDLE?\n\nJust to silence MSVC warnings. WaitForSingleObject requires HANDLE,\n_beginthreadex() returns uintptr_t. It's just a matter of where would\nI put cast ;).\n\n--\nAndrzej\n"},{"id":"126926","messageId":"alpine.LFD.2.00.0911051527570.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911051209jcc490e2pabeab9a1b6e3b11d@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T20:36:51Z","receivedAt":"2009-11-05T20:36:51Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> 2009/11/5 Nicolas Pitre :\n> > On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n> >\n> >> +static unsigned __stdcall win32_start_routine(void *arg)\n> >> +{\n> >> +     pthread_t *thread = arg;\n> >> +     thread->value = thread->start_routine(thread->arg);\n> >> +     return 0;\n> >> +}\n> >\n> > I suppose you could reuse thread->arg for both the argument and the\n> > returned value to save a word.\n> \n> You're right! J6t committed already, what can I do now?\n\nJust post a replacement patch.\n\n> > Why are you casting thread->handle here?  Why not simply declaring it as\n> > a HANDLE?\n> \n> Just to silence MSVC warnings. WaitForSingleObject requires HANDLE,\n> _beginthreadex() returns uintptr_t. It's just a matter of where would\n> I put cast ;).\n\nWonderful. One could wonder why Windows can't have coherent \ninterfaces...\n\nWell, given that all existing usages in the tree (run-command.c and \ncompat/mingw.c) already cast the _beginthreadex() return value instead, \nthen that might be a good idea to follow the same model.\n\n\nNicolas\n"},{"id":"126925","messageId":"16cee31f0911051238v71ec4b57s415efb7bb48999d1@mail.gmail.com","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911051422590.10340@xanadu.home","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-05T20:38:41Z","receivedAt":"2009-11-05T20:38:41Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"2009/11/5 Nicolas Pitre :\n> Why not?  At least gcc is quite happy with such a construct.  It\n> probably makes a copy of the stack before passing it though.\n\nErr... my mind is rotted with all that ugly java, c#, python and ruby,\neven c++. I should start taking some medications I suppose... what was\nI thinking is that C can't copy-construct a struct. Damn, too much sun\n(of java fame)...\n\nThat way I rediscovered simple struct construct...man I missed C so much :)\n\nAnyway, the solution with passing pthread_t as pointer saves some\nstack, so it's probably not that bad.\n\n--\nAndrzej\n"},{"id":"126927","messageId":"alpine.LFD.2.00.0911051708240.10340@xanadu.home","threadId":"21472","inReplyTo":"16cee31f0911051238v71ec4b57s415efb7bb48999d1@mail.gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-05T22:15:13Z","receivedAt":"2009-11-05T22:15:13Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Nov 2009, Andrzej K. Haczewski wrote:\n\n> 2009/11/5 Nicolas Pitre :\n> > Why not?  At least gcc is quite happy with such a construct.  It\n> > probably makes a copy of the stack before passing it though.\n\n\"a copy of the struct\" I meant here.\n\n> Err... my mind is rotted with all that ugly java, c#, python and ruby,\n> even c++. I should start taking some medications I suppose... what was\n> I thinking is that C can't copy-construct a struct. Damn, too much sun\n> (of java fame)...\n> \n> That way I rediscovered simple struct construct...man I missed C so much :)\n\nWelcome back ! ;-)\n\n> Anyway, the solution with passing pthread_t as pointer saves some\n> stack, so it's probably not that bad.\n\nYep, just what I said. Normally if you pass a structure to a function, \nit will be copied beforehand so modifications by the callee won't be \nseen by the caller. But in this case we don't care, hence passing the \noriginal structure address is more efficient.\n\n\nNicolas\n"},{"id":"126955","messageId":"7v7hu43yrc.fsf@alter.siamese.dyndns.org","threadId":"21472","inReplyTo":"alpine.LFD.2.00.0911041623570.10340@xanadu.home","subject":"Re: [PATCH] pack-objects: move thread autodetection closer to relevant code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-06T07:20:39Z","receivedAt":"2009-11-06T07:20:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> Let's keep thread stuff close together if possible. And in this case,\n> this even reduces the #ifdef noise, and allows for skipping the\n> autodetection altogether if delta search is not needed (like with a pure\n> clone).\n\nNice. The ll_find_delta() function itself will disappear and becomes the\nsingle-threaded find_deltas() when THREADED_DELTA_SEARCH is not defined,\nand the variable in question is used only inside the function anyway, so\nthis works beautifully.\n\nVery nice.\n"},{"id":"126958","messageId":"1257495059-12394-1-git-send-email-ahaczewski@gmail.com","threadId":"21472","inReplyTo":"1257283802-29726-1-git-send-email-ahaczewski@gmail.com","subject":"[PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Andrzej K. Haczewski","fromEmail":"ahaczewski@gmail.com","sentAt":"2009-11-06T08:10:59Z","receivedAt":"2009-11-06T08:10:59Z","isPatch":true,"sender":{"key":"ahaczewski@gmail.com","avatar":"https://avatars.githubusercontent.com/u/125249?v=4"},"body":"This patch implements native to Windows subset of pthreads API used by Git.\nIt allows to remove Pthreads for Win32 dependency for MSVC, msysgit and\nCygwin.\n\nThe patch modifies Makefile only for MSVC (that's the environment I'm\ncapable of testing on), so it requires further corrections to compile\nwith MinGW or Cygwin.\n\nSigned-off-by: Andrzej K. Haczewski \n---\nOne more round of that patch with Nicolas' comments considered, and\ndisclamer about the implementation added.\n\nJohannes, can you replace previous commit with that patch?\n\n Makefile | 7 ++-\n builtin-pack-objects.c | 31 +++++++++++--\n compat/mingw.c | 2 +-\n compat/mingw.h | 5 ++\n compat/win32/pthread.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++\n compat/win32/pthread.h | 68 +++++++++++++++++++++++++++\n 6 files changed, 225 insertions(+), 8 deletions(-)\n create mode 100644 compat/win32/pthread.c\n create mode 100644 compat/win32/pthread.h\n\ndiff --git a/Makefile b/Makefile\nindex bc039ac..30089a8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -453,6 +453,7 @@ LIB_H += commit.h\n LIB_H += compat/bswap.h\n LIB_H += compat/cygwin.h\n LIB_H += compat/mingw.h\n+LIB_H += compat/win32/pthread.h\n LIB_H += csum-file.h\n LIB_H += decorate.h\n LIB_H += delta.h\n@@ -971,15 +972,15 @@ ifdef MSVC\n \tOBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo\n \tNO_REGEX = YesPlease\n \tNO_CURL = YesPlease\n-\tNO_PTHREADS = YesPlease\n+\tTHREADED_DELTA_SEARCH = YesPlease\n \tBLK_SHA1 = YesPlease\n \n \tCC = compat/vcbuild/scripts/clink.pl\n \tAR = compat/vcbuild/scripts/lib.pl\n \tCFLAGS =\n \tBASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE\n-\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o\n-\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -DSTRIP_EXTENSION=\\\".exe\\\"\n+\tCOMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o\n+\tCOMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\\\".exe\\\"\n \tBASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib\n \tEXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib\n \tlib =\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 02f9246..00594fd 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1255,15 +1255,15 @@ static int delta_cacheable(unsigned long src_size, unsigned long trg_size,\n \n #ifdef THREADED_DELTA_SEARCH\n \n-static pthread_mutex_t read_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t read_mutex;\n #define read_lock()\t\tpthread_mutex_lock(&read_mutex)\n #define read_unlock()\t\tpthread_mutex_unlock(&read_mutex)\n \n-static pthread_mutex_t cache_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t cache_mutex;\n #define cache_lock()\t\tpthread_mutex_lock(&cache_mutex)\n #define cache_unlock()\t\tpthread_mutex_unlock(&cache_mutex)\n \n-static pthread_mutex_t progress_mutex = PTHREAD_MUTEX_INITIALIZER;\n+static pthread_mutex_t progress_mutex;\n #define progress_lock()\t\tpthread_mutex_lock(&progress_mutex)\n #define progress_unlock()\tpthread_mutex_unlock(&progress_mutex)\n \n@@ -1590,7 +1590,26 @@ struct thread_params {\n \tunsigned *processed;\n };\n \n-static pthread_cond_t progress_cond = PTHREAD_COND_INITIALIZER;\n+static pthread_cond_t progress_cond;\n+\n+/*\n+ * Mutex and conditional variable can't be statically-initialized on Windows.\n+ */\n+static void init_threaded_search()\n+{\n+\tpthread_mutex_init(&read_mutex, NULL);\n+\tpthread_mutex_init(&cache_mutex, NULL);\n+\tpthread_mutex_init(&progress_mutex, NULL);\n+\tpthread_cond_init(&progress_cond, NULL);\n+}\n+\n+static void cleanup_threaded_search()\n+{\n+\tpthread_cond_destroy(&progress_cond);\n+\tpthread_mutex_destroy(&read_mutex);\n+\tpthread_mutex_destroy(&cache_mutex);\n+\tpthread_mutex_destroy(&progress_mutex);\n+}\n \n static void *threaded_find_deltas(void *arg)\n {\n@@ -1629,8 +1648,11 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \tstruct thread_params *p;\n \tint i, ret, active_threads = 0;\n \n+\tinit_threaded_search();\n+\n \tif (delta_search_threads <= 1) {\n \t\tfind_deltas(list, &list_size, window, depth, processed);\n+\t\tcleanup_threaded_search();\n \t\treturn;\n \t}\n \tif (progress > pack_to_stdout)\n@@ -1745,6 +1767,7 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,\n \t\t\tactive_threads--;\n \t\t}\n \t}\n+\tcleanup_threaded_search();\n \tfree(p);\n }\n \ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b5b5b2..f2e9f02 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -5,7 +5,7 @@\n \n #include \n \n-static int err_win_to_posix(DWORD winerr)\n+int err_win_to_posix(DWORD winerr)\n {\n \tint error = ENOSYS;\n \tswitch(winerr) {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6907345..7e25fb5 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -294,3 +294,8 @@ struct mingw_dirent\n #define readdir(x) mingw_readdir(x)\n struct dirent *mingw_readdir(DIR *dir);\n #endif // !NO_MINGW_REPLACE_READDIR\n+\n+/*\n+ * Used by Pthread API implementation for Windows\n+ */\n+extern int err_win_to_posix(DWORD winerr);\ndiff --git a/compat/win32/pthread.c b/compat/win32/pthread.c\nnew file mode 100644\nindex 0000000..652d7b4\n--- /dev/null\n+++ b/compat/win32/pthread.c\n@@ -0,0 +1,120 @@\n+/*\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ *\n+ * DISCLAMER: The implementation is Git-specific, it is subset of original\n+ * Pthreads API, without lots of other features that Git doesn't use.\n+ * Git also makes sure that the passed arguments are valid, so there's\n+ * no need for double-checking.\n+ */\n+\n+#include \"../../git-compat-util.h\"\n+#include \"pthread.h\"\n+\n+#include \n+#include \n+\n+static unsigned __stdcall win32_start_routine(void *arg)\n+{\n+\tpthread_t *thread = arg;\n+\tthread->arg = thread->start_routine(thread->arg);\n+\treturn 0;\n+}\n+\n+int pthread_create(pthread_t *thread, const void *unused,\n+\t\t void *(*start_routine)(void*), void *arg)\n+{\n+\tthread->arg = arg;\n+\tthread->start_routine = start_routine;\n+\tthread->handle = (HANDLE)\n+\t\t_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);\n+\n+\tif (!thread->handle)\n+\t\treturn errno;\n+\telse\n+\t\treturn 0;\n+}\n+\n+int win32_pthread_join(pthread_t *thread, void **value_ptr)\n+{\n+\tDWORD result = WaitForSingleObject(thread->handle, INFINITE);\n+\tswitch (result) {\n+\t\tcase WAIT_OBJECT_0:\n+\t\t\tif (value_ptr)\n+\t\t\t\t*value_ptr = thread->arg;\n+\t\t\treturn 0;\n+\t\tcase WAIT_ABANDONED:\n+\t\t\treturn EINVAL;\n+\t\tdefault:\n+\t\t\treturn err_win_to_posix(GetLastError());\n+\t}\n+}\n+\n+int pthread_cond_init(pthread_cond_t *cond, const void *unused)\n+{\n+\tcond->waiters = 0;\n+\n+\tInitializeCriticalSection(&cond->waiters_lock);\n+\n+\tcond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);\n+\tif (!cond->sema)\n+\t\tdie(\"CreateSemaphore() failed\");\n+\treturn 0;\n+}\n+\n+int pthread_cond_destroy(pthread_cond_t *cond)\n+{\n+\tCloseHandle(cond->sema);\n+\tcond->sema = NULL;\n+\n+\tDeleteCriticalSection(&cond->waiters_lock);\n+\n+\treturn 0;\n+}\n+\n+int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)\n+{\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t++cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Unlock external mutex and wait for signal.\n+\t * NOTE: we've held mutex locked long enough to increment\n+\t * waiters count above, so there's no problem with\n+\t * leaving mutex unlocked before we wait on semaphore.\n+\t */\n+\tLeaveCriticalSection(mutex);\n+\n+\t/* let's wait - ignore return value */\n+\tWaitForSingleObject(cond->sema, INFINITE);\n+\n+\t/* we're done waiting, so make sure we decrease waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\t--cond->waiters;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/* lock external mutex again */\n+\tEnterCriticalSection(mutex);\n+\n+\treturn 0;\n+}\n+\n+int pthread_cond_signal(pthread_cond_t *cond)\n+{\n+\tint have_waiters;\n+\n+\t/* serialize access to waiters count */\n+\tEnterCriticalSection(&cond->waiters_lock);\n+\thave_waiters = cond->waiters > 0;\n+\tLeaveCriticalSection(&cond->waiters_lock);\n+\n+\t/*\n+\t * Signal only when there are waiters\n+\t */\n+\tif (have_waiters)\n+\t\treturn ReleaseSemaphore(cond->sema, 1, NULL) ?\n+\t\t\t0 : err_win_to_posix(GetLastError());\n+\telse\n+\t\treturn 0;\n+}\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nnew file mode 100644\nindex 0000000..63293ad\n--- /dev/null\n+++ b/compat/win32/pthread.h\n@@ -0,0 +1,68 @@\n+/*\n+ * Header used to adapt pthread-based POSIX code to Windows API threads.\n+ *\n+ * Copyright (C) 2009 Andrzej K. Haczewski \n+ */\n+\n+#ifndef PTHREAD_H\n+#define PTHREAD_H\n+\n+#ifndef WIN32_LEAN_AND_MEAN\n+#define WIN32_LEAN_AND_MEAN\n+#endif\n+\n+#include \n+\n+/*\n+ * Defines that adapt Windows API threads to pthreads API\n+ */\n+#define pthread_mutex_t CRITICAL_SECTION\n+\n+#define pthread_mutex_init(a,b) InitializeCriticalSection((a))\n+#define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n+#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_unlock LeaveCriticalSection\n+\n+/*\n+ * Implement simple condition variable for Windows threads, based on ACE\n+ * implementation.\n+ *\n+ * See original implementation: http://bit.ly/1vkDjo\n+ * ACE homepage: http://www.cse.wustl.edu/~schmidt/ACE.html\n+ * See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html\n+ */\n+typedef struct {\n+\tLONG waiters;\n+\tCRITICAL_SECTION waiters_lock;\n+\tHANDLE sema;\n+} pthread_cond_t;\n+\n+extern int pthread_cond_init(pthread_cond_t *cond, const void *unused);\n+\n+extern int pthread_cond_destroy(pthread_cond_t *cond);\n+\n+extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);\n+\n+extern int pthread_cond_signal(pthread_cond_t *cond);\n+\n+/*\n+ * Simple thread creation implementation using pthread API\n+ */\n+typedef struct {\n+\tHANDLE handle;\n+\tvoid *(*start_routine)(void*);\n+\tvoid *arg;\n+} pthread_t;\n+\n+extern int pthread_create(pthread_t *thread, const void *unused,\n+\t\t\t void *(*start_routine)(void*), void *arg);\n+\n+/*\n+ * To avoid the need of copying a struct, we use small macro wrapper to pass\n+ * pointer to win32_pthread_join instead.\n+ */\n+#define pthread_join(a, b) win32_pthread_join(&(a), (b))\n+\n+extern int win32_pthread_join(pthread_t *thread, void **value_ptr);\n+\n+#endif /* PTHREAD_H */\n-- \n1.6.5.2\n"},{"id":"126961","messageId":"4AF3DD7D.4040102@viscovery.net","threadId":"21472","inReplyTo":"1257495059-12394-1-git-send-email-ahaczewski@gmail.com","subject":"Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-11-06T08:25:33Z","receivedAt":"2009-11-06T08:25:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Andrzej K. Haczewski schrieb:\n> One more round of that patch with Nicolas' comments considered, and\n> disclamer about the implementation added.\n> \n> Johannes, can you replace previous commit with that patch?\n\nThanks; the result is in\n\n git://repo.or.cz/git/mingw/j6t.git pthreads-for-windows\n\n-- Hannes\n"}]}