{"thread":{"id":"44378","subject":"[PATCH] compat: Allow static initializer for pthreads on Windows","startedAt":"2016-10-26T21:57:44Z","lastAt":"2016-10-28T20:53:03Z","messageCount":28,"participants":["Stefan Beller","Johannes Sixt","Junio C Hamano","Jeff King","Jacob Keller","Johannes Schindelin","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"304921","messageId":"20161026215732.16411-1-sbeller@google.com","threadId":"44378","inReplyTo":null,"subject":"[PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-26T21:57:32Z","receivedAt":"2016-10-26T21:57:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In Windows it is not possible to have a static initialized mutex as of\nnow, but that seems to be painful for the upcoming refactoring of the\nattribute subsystem, as we have no good place to put the initialization\nof the attr global lock.\n\nThe trick is to get a named mutex as CreateMutex[1] will return the\nexisting named mutex if it exists in a thread safe way, or return\na newly created mutex with that name.\n\nInside the critical section of the single named mutex, we need to double\ncheck if the mutex was already initialized because the outer check is\nnot sufficient.\n(e.g. 2 threads enter the first condition `(!a)` at the same time, but\nonly one of them will acquire the named mutex first and proceeds to\ninitialize the given mutex a. The second thread shall not re-initialize\nthe given mutex `a`, which is why we have the inner condition on `(!a)`.\n\nDue to the use of memory barriers inside the critical section the mutex\n`a` gets updated to other threads, such that any further invocation\nwill skip the initialization check code altogether on the first condition.\n\n[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682411(v=vs.85).aspx\n\nSigned-off-by: Stefan Beller \n---\n\n Flying blind here, i.e. not compiled, not tested. For a system I do not\n have deep knowledge of. The only help was the online documentation.\n\n compat/win32/pthread.h | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/win32/pthread.h b/compat/win32/pthread.h\nindex 1c16408..a900513 100644\n--- a/compat/win32/pthread.h\n+++ b/compat/win32/pthread.h\n@@ -21,9 +21,25 @@\n static inline int return_0(int i) {\n \treturn 0;\n }\n+\n+#define PTHREAD_MUTEX_INITIALIZER NULL\n #define pthread_mutex_init(a,b) return_0((InitializeCriticalSection((a)), 0))\n #define pthread_mutex_destroy(a) DeleteCriticalSection((a))\n-#define pthread_mutex_lock EnterCriticalSection\n+#define pthread_mutex_lock(a) \\\n+{ \\\n+\tif (!a) { \\\n+\t\tHANDLE p = CreateMutex(NULL, FALSE, \"Git-Global-Windows-Mutex\"); \\\n+\t\tEnterCriticalSection(p); \\\n+\t\tMemoryBarrier(); \\\n+\t\tif (!a)\n+\t\t\tpthread_mutex_init(a); \\\n+\t\tMemoryBarrier(); \\\n+\t\tReleaseMutex(p); \\\n+\t} \\\n+\tEnterCriticalSection(a); \\\n+}\n+\n+\n #define pthread_mutex_unlock LeaveCriticalSection\n \n typedef int pthread_mutexattr_t;\n-- \n2.10.1.508.g6572022\n\n"},{"id":"304922","messageId":"CAGZ79kZryb-5jGif04BtK1V9tgFj-tnahqUk+1Lb7XeecU7cMQ@mail.gmail.com","threadId":"44378","inReplyTo":"20161026215732.16411-1-sbeller@google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-26T22:14:21Z","receivedAt":"2016-10-26T22:14:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Oct 26, 2016 at 2:57 PM, Stefan Beller wrote:\n> In Windows it is not possible to have a static initialized mutex as of\n> now, but that seems to be painful for the upcoming refactoring of the\n> attribute subsystem, as we have no good place to put the initialization\n> of the attr global lock.\n>\n> The trick is to get a named mutex as CreateMutex[1] will return the\n> existing named mutex if it exists in a thread safe way, or return\n> a newly created mutex with that name.\n>\n> Inside the critical section of the single named mutex, we need to double\n> check if the mutex was already initialized because the outer check is\n> not sufficient.\n> (e.g. 2 threads enter the first condition `(!a)` at the same time, but\n> only one of them will acquire the named mutex first and proceeds to\n> initialize the given mutex a. The second thread shall not re-initialize\n> the given mutex `a`, which is why we have the inner condition on `(!a)`.\n>\n> Due to the use of memory barriers inside the critical section the mutex\n> `a` gets updated to other threads, such that any further invocation\n> will skip the initialization check code altogether on the first condition.\n>\n> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682411(v=vs.85).aspx\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n> Flying blind here, i.e. not compiled, not tested. For a system I do not\n> have deep knowledge of. The only help was the online documentation.\n\nThis is of course a Double Check Locking Pattern, that Johannes warned about\na couple of days ago. However according to\nhttps://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html\nwe can make it work by adding enough memory barriers, (See section\n\"Making it work with explicit memory barriers\").\n"},{"id":"304947","messageId":"93be5d21-6cb6-ee2b-9f4f-c2fe7c690d6c@kdbg.org","threadId":"44378","inReplyTo":"20161026215732.16411-1-sbeller@google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-27T05:49:27Z","receivedAt":"2016-10-27T05:49:36Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 26.10.2016 um 23:57 schrieb Stefan Beller:\n> In Windows it is not possible to have a static initialized mutex as of\n> now, but that seems to be painful for the upcoming refactoring of the\n> attribute subsystem, as we have no good place to put the initialization\n> of the attr global lock.\n\nPlease rewrite the attribute system such that it can have a dynamic \ninitialization. If you find a global initialization in main() too gross \n(I would agree) then setup_git_directory() might be the right place.\n\n-- Hannes\n\n"},{"id":"304949","messageId":"xmqqlgxa8h3a.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"93be5d21-6cb6-ee2b-9f4f-c2fe7c690d6c@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-27T06:02:33Z","receivedAt":"2016-10-27T06:03:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 26.10.2016 um 23:57 schrieb Stefan Beller:\n>> In Windows it is not possible to have a static initialized mutex as of\n>> now, but that seems to be painful for the upcoming refactoring of the\n>> attribute subsystem, as we have no good place to put the initialization\n>> of the attr global lock.\n>\n> Please rewrite the attribute system such that it can have a dynamic\n> initialization. If you find a global initialization in main() too\n> gross (I would agree) then setup_git_directory() might be the right\n> place.\n\nAs many codepaths may not even need access to the attributes, I\ndoubt that would be a very productive direction to go.\n\n"},{"id":"304950","messageId":"67e38b43-0264-12f2-cca8-4b718ed7dc9d@kdbg.org","threadId":"44378","inReplyTo":"xmqqlgxa8h3a.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-27T06:10:24Z","receivedAt":"2016-10-27T06:10:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.10.2016 um 08:02 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>\n>> Am 26.10.2016 um 23:57 schrieb Stefan Beller:\n>>> In Windows it is not possible to have a static initialized mutex as of\n>>> now, but that seems to be painful for the upcoming refactoring of the\n>>> attribute subsystem, as we have no good place to put the initialization\n>>> of the attr global lock.\n>>\n>> Please rewrite the attribute system such that it can have a dynamic\n>> initialization. If you find a global initialization in main() too\n>> gross (I would agree) then setup_git_directory() might be the right\n>> place.\n>\n> As many codepaths may not even need access to the attributes, I\n> doubt that would be a very productive direction to go.\n\nSo, what is productive then? Pessimizing one (not exactly minor) platform?\n\n-- Hannes\n\n"},{"id":"304951","messageId":"xmqqh97y8g74.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"67e38b43-0264-12f2-cca8-4b718ed7dc9d@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-27T06:21:51Z","receivedAt":"2016-10-27T06:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> As many codepaths may not even need access to the attributes, I\n>> doubt that would be a very productive direction to go.\n>\n> So, what is productive then? Pessimizing one (not exactly minor) platform?\n\nLazy on-demand initialization as needed, perhaps? The on-demand\ninitialization mechanism may become no-op on some platforms that can\ndo static initialization.\n"},{"id":"304953","messageId":"b5084894-0f1e-2a32-79fb-74ce977b3a92@kdbg.org","threadId":"44378","inReplyTo":"xmqqh97y8g74.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-27T06:29:34Z","receivedAt":"2016-10-27T06:29:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.10.2016 um 08:21 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>\n>>> As many codepaths may not even need access to the attributes, I\n>>> doubt that would be a very productive direction to go.\n>>\n>> So, what is productive then? Pessimizing one (not exactly minor) platform?\n>\n> Lazy on-demand initialization as needed, perhaps? The on-demand\n> initialization mechanism may become no-op on some platforms that can\n> do static initialization.\n\nThis is the pessimization that I am talking about. I would not mind at \nall if it were only for the attribute subsystem, but the proposed patch \nwould pessimize *all* uses of pthread_mutex_lock.\n\n-- Hannes\n\n"},{"id":"304954","messageId":"xmqqd1im8foi.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"xmqqh97y8g74.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-27T06:33:01Z","receivedAt":"2016-10-27T06:33:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Sixt writes:\n>\n>>> As many codepaths may not even need access to the attributes, I\n>>> doubt that would be a very productive direction to go.\n>>\n>> So, what is productive then? Pessimizing one (not exactly minor) platform?\n>\n> Lazy on-demand initialization as needed, perhaps? The on-demand\n> initialization mechanism may become no-op on some platforms that can\n> do static initialization.\n\nAh, I think I misunderstood your \"please rewrite\". Did you mean to\nadd \"void attr_start(void)\" helper function to attr.c that does\nseries of pthread_mutex_init() calls as needed? That function can\nbe called from main() of platforms that cannot statically initialize\nmutices, while on other platforms it can be a no-op as long as the\nvariables are statically initialized? If so, that would not pessimize\nany platform, I would think.\n\n\n \n"},{"id":"304970","messageId":"CAGZ79kbP3pgPHgv-x1Q-Q1QwmXc=gOyxWhXh2SngO8WSZc3PFA@mail.gmail.com","threadId":"44378","inReplyTo":"xmqqd1im8foi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-27T17:01:02Z","receivedAt":"2016-10-27T17:01:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> Johannes Sixt writes:\n> This is the pessimization that I am talking about. I would not mind at all if\n> it were only for the attribute subsystem, but the proposed patch would\n> pessimize *all* uses of pthread_mutex_lock.\n\nIt would only pessimize *uninitialized* mutexes? For initialized mutexes\nthe added burden is super cheap (one additional condition).\n\nThe positive aspect of this way the patch proposes would be that any\nfuture contributor not knowing the details of how to do mutexes right\non Windows, would not totally break the whole system, i.e. this seems\nto be more maintainable in the future as it reduces the friction between\npthreads mutexes and the way we can do things in Git in a platform\nindependent way\n\nOn Wed, Oct 26, 2016 at 11:33 PM, Junio C Hamano wrote:\n>>\n>> Lazy on-demand initialization as needed, perhaps? The on-demand\n>> initialization mechanism may become no-op on some platforms that can\n>> do static initialization.\n>\n> Ah, I think I misunderstood your \"please rewrite\". Did you mean to\n> add \"void attr_start(void)\" helper function to attr.c that does\n> series of pthread_mutex_init() calls as needed?\n\nWell one init for now.\n\n> That function can\n> be called from main() of platforms that cannot statically initialize\n> mutices,\n\nBy main you mean the main() in common-main.c or cmd_main in git.c ?\n\nThose both look like the wrong place. Of course it would work adding it\nthere, but it smells like a maintenance nightmare.\n\nAnd then we would modify the attr_start command depending on the\nplatform, i.e.\n\n#ifdef WIN32\nvoid attr_start(void)\n{\n pthread_mutex_init(..);\n}\n#else\nvoid attr_start(void)\n{\n /* nothing as it is statically init'd */\n}\n#endif\n\n> while on other platforms it can be a no-op as long as the\n> variables are statically initialized? If so, that would not pessimize\n> any platform, I would think.\n\nI would think this pessimizes ALL platforms from a maintenance perspective\n(but what do I know about maintaining stuff as an eager young contributor ;)\n\nSo I am willing to go that route, though I do not quite understand where exactly\nyou'd expect me to put the initializer as all places I can think of\nare not the right\nplace.\n\nThanks,\nStefan\n"},{"id":"304973","messageId":"20161027171121.tsg2gy5gov5apihq@sigill.intra.peff.net","threadId":"44378","inReplyTo":"CAGZ79kbP3pgPHgv-x1Q-Q1QwmXc=gOyxWhXh2SngO8WSZc3PFA@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-27T17:11:21Z","receivedAt":"2016-10-27T17:11:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 27, 2016 at 10:01:02AM -0700, Stefan Beller wrote:\n\n> > That function can\n> > be called from main() of platforms that cannot statically initialize\n> > mutices,\n> \n> By main you mean the main() in common-main.c or cmd_main in git.c ?\n\nAny setup or initialization for library functions needs to go in main()\nof common-main.c. Otherwise, only builtins get it, and external\nprograms are left to segfault (or whatever).\n\n> Those both look like the wrong place. Of course it would work adding it\n> there, but it smells like a maintenance nightmare.\n\nI agree it's ugly, but I suspect it would work OK in practice.\nWe don't have that many mutexes and initializing them is cheap.\n\nLanguages like C++ have non-constant initializers, and the compiler\ntakes care of running the startup code before main() begins. There's no\nportable way to do that in C, but our cmd_main() is roughly the same\nthing.\n\nI still prefer the lazy initialization if it can work without incurring\nmeasurable overhead. That's the normal way to do things in C; the only\ncomplication here is the thread safety.\n\n-Peff\n"},{"id":"304976","messageId":"34c88c40-2088-fd74-5d26-56c0599b7eb9@kdbg.org","threadId":"44378","inReplyTo":"CAGZ79kbP3pgPHgv-x1Q-Q1QwmXc=gOyxWhXh2SngO8WSZc3PFA@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-27T18:22:34Z","receivedAt":"2016-10-27T18:22:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.10.2016 um 19:01 schrieb Stefan Beller:\n>> Johannes Sixt writes:\n>> This is the pessimization that I am talking about. I would not mind at all if\n>> it were only for the attribute subsystem, but the proposed patch would\n>> pessimize *all* uses of pthread_mutex_lock.\n>\n> It would only pessimize *uninitialized* mutexes? For initialized mutexes\n> the added burden is super cheap (one additional condition).\n\nIt is not possible to mark a mutex uninitialized on Windows without an \nextra piece of data. A solution would become quite complicated quite \nquickly, and at the cost of additional operations that are in the same \nballpark as an uncontended mutex. I'm not enthused.\n\n> The positive aspect of this way the patch proposes would be that any\n> future contributor not knowing the details of how to do mutexes right\n> on Windows, would not totally break the whole system, i.e. this seems\n> to be more maintainable in the future as it reduces the friction between\n> pthreads mutexes and the way we can do things in Git in a platform\n> independent way\n\nThis is a non-argument. Coders have to know their tools.\n\n-- Hannes\n\n"},{"id":"304980","messageId":"xmqqr3716301.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"34c88c40-2088-fd74-5d26-56c0599b7eb9@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-27T18:49:50Z","receivedAt":"2016-10-27T18:49:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 27.10.2016 um 19:01 schrieb Stefan Beller:\n> ...\n> It is not possible to mark a mutex uninitialized on Windows without an\n> extra piece of data. A solution would become quite complicated quite\n> quickly, and at the cost of additional operations that are in the same\n> ballpark as an uncontended mutex. I'm not enthused.\n>\n>> The positive aspect of this way the patch proposes would be that any\n>> future contributor not knowing the details of how to do mutexes right\n>> on Windows, would not totally break the whole system, i.e. this seems\n>> to be more maintainable in the future as it reduces the friction between\n>> pthreads mutexes and the way we can do things in Git in a platform\n>> independent way\n>\n> This is a non-argument. Coders have to know their tools.\n\nThe codebase should strive to give coders a coherent abstraction\nthat can be implemented efficiently on platforms, so that coders do\nnot have to care too deeply about quirks that exist on individual\nplatforms.\n\nIt is OK to argue that the particular solution Stefan lifted from\nsomewhere (perhaps msdn article he cited???) does not qualify as\nsuch an abstraction. But I do not agree with your \"Coders have to\nknow\" as a blanket statement.\n"},{"id":"304981","messageId":"CAGZ79kY_fZ_pDtVnwJoDkR6PjTNoqDMN5OC70Z8SH_J0Wvkq-w@mail.gmail.com","threadId":"44378","inReplyTo":"xmqqr3716301.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-27T19:08:12Z","receivedAt":"2016-10-27T19:08:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Oct 27, 2016 at 11:49 AM, Junio C Hamano wrote:\n> Johannes Sixt writes:\n>\n>> Am 27.10.2016 um 19:01 schrieb Stefan Beller:\n>> ...\n>> It is not possible to mark a mutex uninitialized on Windows without an\n>> extra piece of data. A solution would become quite complicated quite\n>> quickly, and at the cost of additional operations that are in the same\n>> ballpark as an uncontended mutex. I'm not enthused.\n>>\n>>> The positive aspect of this way the patch proposes would be that any\n>>> future contributor not knowing the details of how to do mutexes right\n>>> on Windows, would not totally break the whole system, i.e. this seems\n>>> to be more maintainable in the future as it reduces the friction between\n>>> pthreads mutexes and the way we can do things in Git in a platform\n>>> independent way\n>>\n>> This is a non-argument. Coders have to know their tools.\n\nWindows is not my tool.\n\n>\n> The codebase should strive to give coders a coherent abstraction\n> that can be implemented efficiently on platforms, so that coders do\n> not have to care too deeply about quirks that exist on individual\n> platforms.\n\nCurrently working as a coder I care about \"submodules, that work on\nlinux.\" I do not care about Windows in the big picture. I am however\nwilling to go an extra step to not break Windows. However that requires\na little bit of effort from both me and you:\n* I need to be aware of what I cannot do with \"not-my-tools\". (So please\n somebody tell me, also in the future when I break obscure platforms. Mind\n that I don't equate obscure with not widely used or in any other way negative.\n It's just that people working on linux find some quirks on Windows\nnot \"obvious\")\n* the workaround should not be too time consuming in the bigger picture,\n which is why I propose to make the API a bit clearer by emulating posix\n mutexes harder. (From a Windows POV this might sound like making it\n more obscure because posix mutexes itself are obscure.)\n\n>\n> It is OK to argue that the particular solution Stefan lifted from\n> somewhere (perhaps msdn article he cited???)\n\nA stack overflow article that I found with my search engine of choice, because\nI could not believe that Windows cannot have statically initialized mutexes.\n\n> does not qualify as\n> such an abstraction.\n\nThe implementation under discussion (well we did not discuss the\nimplementation a\nwhole lot yet) may even contain an error as the first memory barrier\nneeds to be in front\nof the first condition.\n\n> But I do not agree with your \"Coders have to\n> know\" as a blanket statement.\n\nWell I do to some extent, I just disagree what my tools are.\n"},{"id":"304985","messageId":"a2e5acd6-485d-0387-7a85-6042dee702f7@kdbg.org","threadId":"44378","inReplyTo":"CAGZ79kY_fZ_pDtVnwJoDkR6PjTNoqDMN5OC70Z8SH_J0Wvkq-w@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-27T20:05:26Z","receivedAt":"2016-10-27T20:06:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.10.2016 um 21:08 schrieb Stefan Beller:\n> On Thu, Oct 27, 2016 at 11:49 AM, Junio C Hamano wrote:\n>> Johannes Sixt writes:\n>>\n>>> Am 27.10.2016 um 19:01 schrieb Stefan Beller:\n>>> ...\n>>> It is not possible to mark a mutex uninitialized on Windows without an\n>>> extra piece of data. A solution would become quite complicated quite\n>>> quickly, and at the cost of additional operations that are in the same\n>>> ballpark as an uncontended mutex. I'm not enthused.\n>>>\n>>>> The positive aspect of this way the patch proposes would be that any\n>>>> future contributor not knowing the details of how to do mutexes right\n>>>> on Windows, would not totally break the whole system, i.e. this seems\n>>>> to be more maintainable in the future as it reduces the friction between\n>>>> pthreads mutexes and the way we can do things in Git in a platform\n>>>> independent way\n>>>\n>>> This is a non-argument. Coders have to know their tools.\n>\n> Windows is not my tool.\n\nThe tool I meant is pthreads. For example, you can't have a \npthread_mutex_t variable and not initialize it with either \nPTHREAD_MUTEX_INITIALIZER or pthread_mutex_init.\n\n>> The codebase should strive to give coders a coherent abstraction\n>> that can be implemented efficiently on platforms, so that coders do\n>> not have to care too deeply about quirks that exist on individual\n>> platforms.\n>\n> Currently working as a coder I care about \"submodules, that work on\n> linux.\" I do not care about Windows in the big picture.\n\nI don't know what you meant to say with this sentence, but taken \nliterally, are you on the right ship here, I have to ask?\n\n> I am however\n> willing to go an extra step to not break Windows.\n\n\"Not break Windows\" is equivalent to \"make it work on Windows\", mind \nyou. We can't have a new feature only on Linux when there is no reason \nnot to have it on Windows as well. Sorry, \"Git is for Unix only\" is long \nover.\n\n> However that requires\n> a little bit of effort from both me and you:\n> * I need to be aware of what I cannot do with \"not-my-tools\". (So please\n> somebody tell me, also in the future when I break obscure platforms. Mind\n> that I don't equate obscure with not widely used or in any other way negative.\n> It's just that people working on linux find some quirks on Windows\n> not \"obvious\")\n\nThat goes without saying.\n\n> * the workaround should not be too time consuming in the bigger picture,\n\nThat, however, is wishful thinking. If you want to have a feature \ndearly, you have to make it work, and that may take its time. I'm not \nsaying that *you* have to make it work (there are platform experts \naround to lend a hand), but just an extra step to \"not break\" an \nimportant platform is not enough.\n\n> which is why I propose to make the API a bit clearer by emulating posix\n> mutexes harder. (From a Windows POV this might sound like making it\n> more obscure because posix mutexes itself are obscure.)\n\nSo, when you think that POSIX mutexes are obscure, why don't we settle \non the simpler Windows critical sections? Emulating them with pthreads \nshould be child's play.\n\n> The implementation under discussion (well we did not discuss the\n> implementation a\n> whole lot yet) ...\n\nThere's not a whole lot to discuss: it must be rewritten from scratch \n(it's not just the memory barriers, it is everything else, too). But \ntime is much better spent on an attr_start() solution.\n\n-- Hannes\n\n"},{"id":"305000","messageId":"CA+P7+xpckfaeHmoEGQBdLD-=Kf7gQ-jOxGFKrKmiFH1SBN7GjA@mail.gmail.com","threadId":"44378","inReplyTo":"a2e5acd6-485d-0387-7a85-6042dee702f7@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-10-27T21:49:41Z","receivedAt":"2016-10-27T21:50:45Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Oct 27, 2016 at 1:05 PM, Johannes Sixt wrote:\n>> The implementation under discussion (well we did not discuss the\n>> implementation a\n>> whole lot yet) ...\n>\n>\n> There's not a whole lot to discuss: it must be rewritten from scratch (it's\n> not just the memory barriers, it is everything else, too). But time is much\n> better spent on an attr_start() solution.\n>\n> -- Hannes\n>\n\nOk, so I've been reading this thread. I don't understand your\nobjections to emulating in this way.. Could you clearly spell out why\nyou believe this solution isn't acceptable? So far all I've understood\nwas \"it's not critical sections\" and \"it penalizes Windows too much\"\nbut... If Windows cannot statically initialize a pthread mutex, then\nwe *have* to dynamically initialize it somewhere. This solution adds a\nsingle check before each lock and is safe due to use of memory\nbarriers. Yes, this will cost a tiny bit extra overhead for each use\nof \"pthread_mutex_lock\" but I fail to see how that is a huge\npenalty...\n\nSo I'm trying to understand if that really is your only concern,\nbecause I don't actually buy that it is a huge overhead to pay.\n\nI agree with Stefan that there isn't really a great place to put a\ndynamic initialization.\n\nTo be clear I am trying to understand the objections since so far all\nI have read is \"this is bad, and I object\" without clearly explaining\nreasoning.\n\nRegards,\nJake\n"},{"id":"305003","messageId":"CAGZ79kbpzAcaRWyJQe_XS6QCve_U9=mOUx9C=si=o_3wzmPcRQ@mail.gmail.com","threadId":"44378","inReplyTo":"CA+P7+xpckfaeHmoEGQBdLD-=Kf7gQ-jOxGFKrKmiFH1SBN7GjA@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-27T21:59:11Z","receivedAt":"2016-10-27T21:59:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> there isn't really a great place to put a dynamic initialization.\n\nAnswering this question specifically (Where to put it?),\nI am about to send out a patch that puts it in compat/mingw.c:2232:\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 3fbfda5..9881c3d 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2232,6 +2232,9 @@ void mingw_startup(void)\n /* initialize critical section for waitpid pinfo_t list */\n InitializeCriticalSection(&pinfo_cs);\n\n+ /* initialize critical sections in the attr code */\n+ start_attr();\n+\n /* set up default file mode and file modes for stdin/out/err */\n _fmode = _O_BINARY;\n _setmode(_fileno(stdin), _O_BINARY);\n\nIf I understood Johannes correctly this is the place to put it.\nJunio seems to be ok with static mutex init for non windows platforms,\nso I put it into the mingw specifc startup code.\n\nThanks,\nStefan\n"},{"id":"305041","messageId":"2ddca5e3-3c4d-b555-4309-a180ceed581e@kdbg.org","threadId":"44378","inReplyTo":"CA+P7+xpckfaeHmoEGQBdLD-=Kf7gQ-jOxGFKrKmiFH1SBN7GjA@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-28T05:55:16Z","receivedAt":"2016-10-28T05:55:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.10.2016 um 23:49 schrieb Jacob Keller:\n> Ok, so I've been reading this thread. I don't understand your\n> objections to emulating in this way.. Could you clearly spell out why\n> you believe this solution isn't acceptable? So far all I've understood\n> was \"it's not critical sections\" and \"it penalizes Windows too much\"\n> but... If Windows cannot statically initialize a pthread mutex, then\n> we *have* to dynamically initialize it somewhere. This solution adds a\n> single check before each lock and is safe due to use of memory\n> barriers. Yes, this will cost a tiny bit extra overhead for each use\n> of \"pthread_mutex_lock\" but I fail to see how that is a huge\n> penalty...\n\nOne point is that the DCLP idiom must be implemented correctly. There \nare solutions, of course, and when the initialization is over, we have a \nminiscule overhead at each pthread_mutex_lock call.\n\nThe main point is that the initialization has to solve a chicken-and-egg \nproblem: After we have found an uninitialized critical section, we have \nto have mutual exclusion for the initialization. We need another \ncritical section for this, but we cannot have one that is initialized. \nFor this reason, the solution uses a different kind of mutual exclusion \nprimitive, which is more akin to POSIX semaphores and works across \nprocesses. In the patch proposed by Stefan, a *session-wide* mutex is \nused. That means that all concurrent git invocations in a user's session \nsynchronize their initialization of critical section objects.\n\nThat's just ridiculous. It's like waiting for a ... no, *the* ... battle \nship just to get our bouncers in their position. We are talking \nmilliseconds here, not nanoseconds.\n\n-- Hannes\n\n"},{"id":"305043","messageId":"CA+P7+xrvHfTd77uTM+g1_QyWFvQuak2x-wdej7hD99v-37AOLg@mail.gmail.com","threadId":"44378","inReplyTo":"2ddca5e3-3c4d-b555-4309-a180ceed581e@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-10-28T06:11:37Z","receivedAt":"2016-10-28T06:12:02Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Oct 27, 2016 at 10:55 PM, Johannes Sixt wrote:\n> One point is that the DCLP idiom must be implemented correctly. There are\n> solutions, of course, and when the initialization is over, we have a\n> miniscule overhead at each pthread_mutex_lock call.\n>\n\nRight, this I understood, but appeared to be solved.\n\n> The main point is that the initialization has to solve a chicken-and-egg\n> problem: After we have found an uninitialized critical section, we have to\n> have mutual exclusion for the initialization. We need another critical\n> section for this, but we cannot have one that is initialized. For this\n> reason, the solution uses a different kind of mutual exclusion primitive,\n> which is more akin to POSIX semaphores and works across processes. In the\n> patch proposed by Stefan, a *session-wide* mutex is used. That means that\n> all concurrent git invocations in a user's session synchronize their\n> initialization of critical section objects.\n>\n\nThank you for explaining this. Now I understand why this would be\nconsidered a big issue, and potentially worth considering the\nalternatives like attr_start. This was missing since I don't think any\nof the rest of us knew (correct me if I am wrong) that the\nsynchronization would be global. For many cases it's probably not that\nbad, but we do have a suitable explanation, and I think living with\n\"attr_start()\" in the win32 initialization path which is something\nStefan suggested in a previous email would make the most sense then.\n\n> That's just ridiculous. It's like waiting for a ... no, *the* ... battle\n> ship just to get our bouncers in their position. We are talking milliseconds\n> here, not nanoseconds.\n>\n\nRight. Thanks for filling in the missing details.\n\nRegards,\nJake\n\n> -- Hannes\n>\n"},{"id":"305056","messageId":"alpine.DEB.2.20.1610281356310.3264@virtualbox","threadId":"44378","inReplyTo":"CA+P7+xpckfaeHmoEGQBdLD-=Kf7gQ-jOxGFKrKmiFH1SBN7GjA@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-28T11:58:44Z","receivedAt":"2016-10-28T11:59:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jake,\n\nOn Thu, 27 Oct 2016, Jacob Keller wrote:\n\n> I agree with Stefan that there isn't really a great place to put a\n> dynamic initialization.\n\nUmmm. Wait. What???\n\nhttps://github.com/git/git/blob/v2.10.1/common-main.c#L25-L41\n\nCiao,\nJohannes\n"},{"id":"305068","messageId":"B5FD05E58E36480894F1BDBBC9589EE1@PhilipOakley","threadId":"44378","inReplyTo":"2ddca5e3-3c4d-b555-4309-a180ceed581e@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2016-10-28T13:01:45Z","receivedAt":"2016-10-28T16:32:10Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Sixt\" \n>\n> One point is that the DCLP idiom must be implemented correctly. There are \n> solutions, of course, and when the initialization is over, we have a \n> miniscule overhead at each pthread_mutex_lock call.\n>\n\nI had to look up DCLP ( = Double Checked Locking Patterns), and found a good \nwrite up on the issues..\n\nhttp://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf \"C++ and the \nPerils of Double-Checked Locking\", which include 'C' issues, and \nmulti-thread, multi-processor issues. Not an easy issue when fighting \noptimisers..\n\n--\n\nPhilip \n\n"},{"id":"305079","messageId":"CA+P7+xoyF8EG079eC-dfTSj+YrbxhPWx356-jZ90gDs6SwyppA@mail.gmail.com","threadId":"44378","inReplyTo":"alpine.DEB.2.20.1610281356310.3264@virtualbox","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-10-28T18:48:36Z","receivedAt":"2016-10-28T18:49:16Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Oct 28, 2016 at 4:58 AM, Johannes Schindelin\n wrote:\n> Hi Jake,\n>\n> On Thu, 27 Oct 2016, Jacob Keller wrote:\n>\n>> I agree with Stefan that there isn't really a great place to put a\n>> dynamic initialization.\n>\n> Ummm. Wait. What???\n>\n> https://github.com/git/git/blob/v2.10.1/common-main.c#L25-L41\n>\n> Ciao,\n> Johannes\n\nI think Stefan has since provided a better suggestion of isolating the\ndynamic initialization into the MINGW startup section. However, you\nare right either place works, though I think platforms which support\nstatic initialization should use it.\n\nThanks,\nJake\n"},{"id":"305080","messageId":"CA+P7+xoNSrhaQ50GZ4RzLregznw+0yf2=ttNuYf1VUw+YabdFQ@mail.gmail.com","threadId":"44378","inReplyTo":"B5FD05E58E36480894F1BDBBC9589EE1@PhilipOakley","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-10-28T18:49:25Z","receivedAt":"2016-10-28T18:49:53Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Oct 28, 2016 at 6:01 AM, Philip Oakley wrote:\n> From: \"Johannes Sixt\" \n>>\n>>\n>> One point is that the DCLP idiom must be implemented correctly. There are\n>> solutions, of course, and when the initialization is over, we have a\n>> miniscule overhead at each pthread_mutex_lock call.\n>>\n>\n> I had to look up DCLP ( = Double Checked Locking Patterns), and found a good\n> write up on the issues..\n>\n> http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf \"C++ and the\n> Perils of Double-Checked Locking\", which include 'C' issues, and\n> multi-thread, multi-processor issues. Not an easy issue when fighting\n> optimisers..\n>\n> --\n>\n> Philip\n\n\nYep, this is why we have memory barriers. Ofcourse languages like C\ndon't really allow you to express them in the language and we restore\nto various platform specific methods.\n\nThanks,\nJake\n"},{"id":"305121","messageId":"38b70094-5550-8512-7735-a6739f435803@kdbg.org","threadId":"44378","inReplyTo":"CA+P7+xoyF8EG079eC-dfTSj+YrbxhPWx356-jZ90gDs6SwyppA@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-28T20:26:01Z","receivedAt":"2016-10-28T20:26:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.10.2016 um 20:48 schrieb Jacob Keller:\n> On Fri, Oct 28, 2016 at 4:58 AM, Johannes Schindelin\n> wrote:\n>> Hi Jake,\n>>\n>> On Thu, 27 Oct 2016, Jacob Keller wrote:\n>>\n>>> I agree with Stefan that there isn't really a great place to put a\n>>> dynamic initialization.\n>>\n>> Ummm. Wait. What???\n>>\n>> https://github.com/git/git/blob/v2.10.1/common-main.c#L25-L41\n>\n> I think Stefan has since provided a better suggestion of isolating the\n> dynamic initialization into the MINGW startup section. However, you\n> are right either place works, though I think platforms which support\n> static initialization should use it.\n\nIt's a pity that this new patch introduces the first use of \nPTHREAD_MUTEX_INITILIZER. (I do not count the one in compat/nedmalloc, \nit's in platform specific code.) We have to introduce a dummy definition \non Windows to even compile the code.\n\nNow we have a double-edged sword: Other coders who are not aware of this \nthread might think it works. But it does not.\n\nAnother problem with the proposed patch is that there is no declaration \nfor attr_start() before the call in compat/mingw.c. We would have to \nmove the declaration of attr_start() to cache.h (for example), because \n#including attr.h in compat/mingw.c is plainly wrong. However, it would \nnot be a major offense to #include attr.h in common-main.c. But when we \ndo that, we can certainly spare the few cycles to call pthread_mutex_init.\n\n-- Hannes\n\n"},{"id":"305122","messageId":"xmqqr370z07v.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"38b70094-5550-8512-7735-a6739f435803@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-28T20:29:24Z","receivedAt":"2016-10-28T20:29:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Another problem with the proposed patch is that there is no\n> declaration for attr_start() before the call in compat/mingw.c. We\n> would have to move the declaration of attr_start() to cache.h (for\n> example), because #including attr.h in compat/mingw.c is plainly\n> wrong. However, it would not be a major offense to #include attr.h in\n> common-main.c. But when we do that, we can certainly spare the few\n> cycles to call pthread_mutex_init.\n\nThat sounds like a good argument to have it in common-main.c. \n\nWould it mean that the code that defines the mutex needs to have\n#ifdef that defines a no-op attr_start() and defines the mutex with\nPTHREAD_MUTEX_INITILIZER with #else that just defines the mutex\nwithout initializatin, with the real attr_start(), though?\n"},{"id":"305123","messageId":"CAGZ79kZVivquVDH9S63K8p3bOis+e=b3cKVVrg13zPe_BrL_Sw@mail.gmail.com","threadId":"44378","inReplyTo":"xmqqr370z07v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-28T20:36:05Z","receivedAt":"2016-10-28T20:36:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Oct 28, 2016 at 1:29 PM, Junio C Hamano wrote:\n> Johannes Sixt writes:\n>\n>> Another problem with the proposed patch is that there is no\n>> declaration for attr_start() before the call in compat/mingw.c. We\n>> would have to move the declaration of attr_start() to cache.h (for\n>> example), because #including attr.h in compat/mingw.c is plainly\n>> wrong. However, it would not be a major offense to #include attr.h in\n>> common-main.c. But when we do that, we can certainly spare the few\n>> cycles to call pthread_mutex_init.\n>\n> That sounds like a good argument to have it in common-main.c.\n\nIf we're going that route, I would get rid of PTHREAD_MUTEX_INITILIZER\nand call a pthread_mutex_init platform independently.\n\n>\n> Would it mean that the code that defines the mutex needs to have\n> #ifdef that defines a no-op attr_start() and defines the mutex with\n> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex\n> without initializatin, with the real attr_start(), though?\n"},{"id":"305124","messageId":"xmqqmvhoyzth.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"CAGZ79kZVivquVDH9S63K8p3bOis+e=b3cKVVrg13zPe_BrL_Sw@mail.gmail.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-28T20:38:02Z","receivedAt":"2016-10-28T20:38:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Fri, Oct 28, 2016 at 1:29 PM, Junio C Hamano wrote:\n>> Johannes Sixt writes:\n>>\n>>> Another problem with the proposed patch is that there is no\n>>> declaration for attr_start() before the call in compat/mingw.c. We\n>>> would have to move the declaration of attr_start() to cache.h (for\n>>> example), because #including attr.h in compat/mingw.c is plainly\n>>> wrong. However, it would not be a major offense to #include attr.h in\n>>> common-main.c. But when we do that, we can certainly spare the few\n>>> cycles to call pthread_mutex_init.\n>>\n>> That sounds like a good argument to have it in common-main.c.\n>\n> If we're going that route, I would get rid of PTHREAD_MUTEX_INITILIZER\n> and call a pthread_mutex_init platform independently.\n\nYup, but earlier j6t was trying hard not to penalize any single\nplatform, and that would certainly penalize the ones with static\ninitialization, so I was hoping to hear if there is a clever\nworkaround to avoid that.\n\n>> Would it mean that the code that defines the mutex needs to have\n>> #ifdef that defines a no-op attr_start() and defines the mutex with\n>> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex\n>> without initializatin, with the real attr_start(), though?\n"},{"id":"305126","messageId":"ab4c8148-2124-f08d-a770-e4f4e49a1c15@kdbg.org","threadId":"44378","inReplyTo":"xmqqr370z07v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-10-28T20:49:04Z","receivedAt":"2016-10-28T20:49:43Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 28.10.2016 um 22:29 schrieb Junio C Hamano:\n> Johannes Sixt writes:\n>\n>> Another problem with the proposed patch is that there is no\n>> declaration for attr_start() before the call in compat/mingw.c. We\n>> would have to move the declaration of attr_start() to cache.h (for\n>> example), because #including attr.h in compat/mingw.c is plainly\n>> wrong. However, it would not be a major offense to #include attr.h in\n>> common-main.c. But when we do that, we can certainly spare the few\n>> cycles to call pthread_mutex_init.\n>\n> That sounds like a good argument to have it in common-main.c.\n>\n> Would it mean that the code that defines the mutex needs to have\n> #ifdef that defines a no-op attr_start() and defines the mutex with\n> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex\n> without initializatin, with the real attr_start(), though?\n\nNo. My intent was to call pthread_mutex_init for all platforms. We \nalready call open(\"/dev/null\") unconditionally on every program startup. \nThe few cycles spent in pthread_mutex_init should be the least of our \nworries.\n\n-- Hannes\n\n"},{"id":"305127","messageId":"xmqqbmy4yz4o.fsf@gitster.mtv.corp.google.com","threadId":"44378","inReplyTo":"ab4c8148-2124-f08d-a770-e4f4e49a1c15@kdbg.org","subject":"Re: [PATCH] compat: Allow static initializer for pthreads on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-28T20:52:55Z","receivedAt":"2016-10-28T20:53:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 28.10.2016 um 22:29 schrieb Junio C Hamano:\n>> Johannes Sixt writes:\n>>\n>>> Another problem with the proposed patch is that there is no\n>>> declaration for attr_start() before the call in compat/mingw.c. We\n>>> would have to move the declaration of attr_start() to cache.h (for\n>>> example), because #including attr.h in compat/mingw.c is plainly\n>>> wrong. However, it would not be a major offense to #include attr.h in\n>>> common-main.c. But when we do that, we can certainly spare the few\n>>> cycles to call pthread_mutex_init.\n>>\n>> That sounds like a good argument to have it in common-main.c.\n>>\n>> Would it mean that the code that defines the mutex needs to have\n>> #ifdef that defines a no-op attr_start() and defines the mutex with\n>> PTHREAD_MUTEX_INITILIZER with #else that just defines the mutex\n>> without initializatin, with the real attr_start(), though?\n>\n> No. My intent was to call pthread_mutex_init for all platforms.\n\nAh, OK, so there was no magic plan. That's OK.\n\n"}]}