{"thread":{"id":"23525","subject":"[PATCH] Fix checkout of large files to network shares under Windows XP","startedAt":"2010-04-19T12:45:55Z","lastAt":"2010-10-17T10:54:40Z","messageCount":15,"participants":["Sebastian Schuberth","Junio C Hamano","René Scharfe","Albert Dvornik","Johannes Sixt","Johannes Schindelin","Dmitry Potapov"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"139894","messageId":"4BCC5083.30801@gmail.com","threadId":"23525","inReplyTo":null,"subject":"[PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2010-04-19T12:45:55Z","receivedAt":"2010-04-19T12:45:55Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"This fixes msysGit issue 409, see\nhttp://code.google.com/p/msysgit/issues/detail?id=409\n\nSigned-off-by: Sebastian Schuberth <sschuberth@gmail.com>\n---\n compat/mingw.c |   24 ++++++++++++++++++++++++\n compat/mingw.h |    3 +++\n 2 files changed, 27 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 7ec615c..672d074 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -293,6 +293,30 @@ int mingw_open (const char *filename, int oflags, ...)\n \treturn fd;\n }\n \n+#undef write\n+ssize_t mingw_write(int fd, const void *buf, size_t count)\n+{\n+\tssize_t written = 0;\n+\tsize_t total = 0, size = count;\n+\n+\twhile (total < count && size > 0) {\n+\t\twritten = write(fd, buf, size);\n+\t\tif (written < 0 && errno == EINVAL) {\n+\t\t\t// There seems to be a bug in the Windows XP network stack that\n+\t\t\t// causes writes with sizes > 64 MB to fail, so we halve the size\n+\t\t\t// until we succeed or ultimately fail.\n+\t\t\tsize /= 2;\n+\t\t} else {\n+\t\t\tbuf += written;\n+\t\t\ttotal += written;\n+\t\t\tif (total + size > count)\n+\t\t\t\tsize = count - total;\n+\t\t}\n+\t}\n+\n+\treturn written < 0 ? written : total;\n+}\n+\n #undef fopen\n FILE *mingw_fopen (const char *filename, const char *otype)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 756f3ab..751bb4c 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -178,6 +178,9 @@ int mingw_rmdir(const char *path);\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n+ssize_t mingw_write(int fd, const void *buf, size_t count);\n+#define write mingw_write\n+\n FILE *mingw_fopen (const char *filename, const char *otype);\n #define fopen mingw_fopen\n \n-- \n1.7.0.2.msysgit.0.898.gbf4f.dirty\n"},{"id":"139926","messageId":"7vaaszqinh.fsf@alter.siamese.dyndns.org","threadId":"23525","inReplyTo":"4BCC5083.30801@gmail.com","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-19T20:41:54Z","receivedAt":"2010-04-19T20:41:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dscho, I saw that you are planning to queue this to your tree; would it\nmake your job easier if I queue this myself now, or if I refrain from\nqueueing this myself and instead wait for you to tell me to pull or apply?\n\nI can go either way.\n"},{"id":"139927","messageId":"4BCCC05E.4030206@lsrfire.ath.cx","threadId":"23525","inReplyTo":"4BCC5083.30801@gmail.com","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-04-19T20:43:10Z","receivedAt":"2010-04-19T20:43:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.04.2010 14:45, schrieb Sebastian Schuberth:\n> This fixes msysGit issue 409, see\n> http://code.google.com/p/msysgit/issues/detail?id=409\n> \n> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>\n> ---\n>  compat/mingw.c |   24 ++++++++++++++++++++++++\n>  compat/mingw.h |    3 +++\n>  2 files changed, 27 insertions(+), 0 deletions(-)\n> \n> diff --git a/compat/mingw.c b/compat/mingw.c\n> index 7ec615c..672d074 100644\n> --- a/compat/mingw.c\n> +++ b/compat/mingw.c\n> @@ -293,6 +293,30 @@ int mingw_open (const char *filename, int oflags, ...)\n>  \treturn fd;\n>  }\n>  \n> +#undef write\n> +ssize_t mingw_write(int fd, const void *buf, size_t count)\n> +{\n> +\tssize_t written = 0;\n> +\tsize_t total = 0, size = count;\n> +\n> +\twhile (total < count && size > 0) {\n> +\t\twritten = write(fd, buf, size);\n> +\t\tif (written < 0 && errno == EINVAL) {\n> +\t\t\t// There seems to be a bug in the Windows XP network stack that\n> +\t\t\t// causes writes with sizes > 64 MB to fail, so we halve the size\n> +\t\t\t// until we succeed or ultimately fail.\n\nC style comments (/*...*/) are preferred over C++ style comments (//...)\nfor git.\n\nIs there a known-good size, or at least a mostly-working one?  Would it\nmake sense to start with that size instead of halving and trying until\nthat size is reached?\n\n> +\t\t\tsize /= 2;\n> +\t\t} else {\n> +\t\t\tbuf += written;\n> +\t\t\ttotal += written;\n\nWhat about other errors?  You need to break out of the loop instead of\nadding -1 to buf and total, right?\n\n> +\t\t\tif (total + size > count)\n> +\t\t\t\tsize = count - total;\n> +\t\t}\n> +\t}\n\nShouldn't the loop be left in the successful case, too?  write(2) is\nallowed to write less than requested, so the caller already needs to\ndeal with that case anyway.\n\n> +\n> +\treturn written < 0 ? written : total;\n> +}\n> +\n>  #undef fopen\n>  FILE *mingw_fopen (const char *filename, const char *otype)\n>  {\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index 756f3ab..751bb4c 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -178,6 +178,9 @@ int mingw_rmdir(const char *path);\n>  int mingw_open (const char *filename, int oflags, ...);\n>  #define open mingw_open\n>  \n> +ssize_t mingw_write(int fd, const void *buf, size_t count);\n> +#define write mingw_write\n> +\n>  FILE *mingw_fopen (const char *filename, const char *otype);\n>  #define fopen mingw_fopen\n>  \n"},{"id":"139939","messageId":"z2l9f74dc2c1004191546n5562759an54342e3efdaa9539@mail.gmail.com","threadId":"23525","inReplyTo":"4BCCC05E.4030206@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Albert Dvornik","fromEmail":"dvornik+git@gmail.com","sentAt":"2010-04-19T22:46:52Z","receivedAt":"2010-04-19T22:46:52Z","isPatch":true,"sender":{"key":"dvornik+git@gmail.com","avatar":null},"body":"On Mon, Apr 19, 2010 at 4:43 PM, René Scharfe\n<rene.scharfe@lsrfire.ath.cx> wrote:\n  [...]\n>> +                     if (total + size > count)\n>> +                             size = count - total;\n>> +             }\n>> +     }\n>\n> Shouldn't the loop be left in the successful case, too?  write(2) is\n> allowed to write less than requested, so the caller already needs to\n> deal with that case anyway.\n\nThat's what I thought initially, since the code would be cleaner, but\nI don't like the fact that you could actually end up making a lot more\nfailed write() calls that way, since you restart the size search on\neach call to mingw_write().\n\nFor example, suppose you were calling mingw_write() with a count that\nwas exactly 11.5 times bigger than whatever maximum size write() was\nwilling to accept.  If you only did one write() per mingw_write(),\nletting the caller restart, this will result in 47 failed writes and\n16 successes.  Letting mingw_write() do the restart (as in the\nexisting code) will end up with 4 failed writes and 16 successes.\nNow, I assume (wait, this is Windows-- I'd *like to hope*) that a\nfailed write() is a lot cheaper than a successful one, but this still\nrubs me the wrong way.\n\nOf course, if we know (or can guess) the maximum size write() will\ntake, that would be best.\n\n--bert\n"},{"id":"139956","messageId":"4BCD6351.1050706@viscovery.net","threadId":"23525","inReplyTo":"4BCCC05E.4030206@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-04-20T08:18:25Z","receivedAt":"2010-04-20T08:18:25Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 4/19/2010 22:43, schrieb René Scharfe:\n> Am 19.04.2010 14:45, schrieb Sebastian Schuberth:\n>> +#undef write\n>> +ssize_t mingw_write(int fd, const void *buf, size_t count)\n>> +{\n>> +\tssize_t written = 0;\n>> +\tsize_t total = 0, size = count;\n>> +\n>> +\twhile (total < count && size > 0) {\n>> +\t\twritten = write(fd, buf, size);\n>> +\t\tif (written < 0 && errno == EINVAL) {\n>> +\t\t\t// There seems to be a bug in the Windows XP network stack that\n>> +\t\t\t// causes writes with sizes > 64 MB to fail, so we halve the size\n>> +\t\t\t// until we succeed or ultimately fail.\n> \n> C style comments (/*...*/) are preferred over C++ style comments (//...)\n> for git.\n> \n> Is there a known-good size, or at least a mostly-working one?  Would it\n> make sense to start with that size instead of halving and trying until\n> that size is reached?\n> \n>> +\t\t\tsize /= 2;\n>> +\t\t} else {\n>> +\t\t\tbuf += written;\n>> +\t\t\ttotal += written;\n> \n> What about other errors?  You need to break out of the loop instead of\n> adding -1 to buf and total, right?\n\nThanks for a thorough review. I had the gut feeling that something's wrong\nwith the code due to its structure, but didn't stare at the code long\nenough to notice this.\n\nI suggest to have this structure\n\n\twrite\n\tif success or failure is not EINVAL\n\t\treturn\n\n\tdo\n\t\treduce size\n\t\tif larger than known (presumed?) maximum\n\t\t\treduce to that maximum\n\t\twrite\n\twhile not success and failure is EINVAL\n\n\twhile not failure and exactly reduced size written\n\t\twrite more\n\nI don't think that we will observe any short writes *after* the size was\nreduced, which Albert is concerned about. Somebody who observes the\nfailure that this works around could instrument the function to see\nwhether short writes are really a problem.\n\n-- Hannes\n"},{"id":"139959","messageId":"alpine.DEB.1.00.1004201114150.7651@pacific.mpi-cbg.de","threadId":"23525","inReplyTo":"7vaaszqinh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-04-20T09:15:25Z","receivedAt":"2010-04-20T09:15:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 19 Apr 2010, Junio C Hamano wrote:\n\n> Dscho, I saw that you are planning to queue this to your tree; would it \n> make your job easier if I queue this myself now, or if I refrain from \n> queueing this myself and instead wait for you to tell me to pull or \n> apply?\n\nIt seems to be contested still, but I have no problem with cooking it in \ndevel directly.\n\nCiao,\nDscho\n"},{"id":"139970","messageId":"t2xbdca99241004200542ud4e8ea5azcad918c37bcacf1a@mail.gmail.com","threadId":"23525","inReplyTo":"4BCCC05E.4030206@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2010-04-20T12:42:56Z","receivedAt":"2010-04-20T12:42:56Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Mon, Apr 19, 2010 at 22:43, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:\n\n>> +             if (written < 0 && errno == EINVAL) {\n>> +                     // There seems to be a bug in the Windows XP network stack that\n>> +                     // causes writes with sizes > 64 MB to fail, so we halve the size\n>> +                     // until we succeed or ultimately fail.\n>\n> C style comments (/*...*/) are preferred over C++ style comments (//...)\n> for git.\n\nOh well, I've changed that.\n\n> Is there a known-good size, or at least a mostly-working one?  Would it\n> make sense to start with that size instead of halving and trying until\n> that size is reached?\n\nAs the comment says, the greatest size that worked in my experiments\nis 64 MB, and other posts on the Internert suggest the same limit, but\nit's still an unconfirmed / undocumented issue. Anyway, I'm now\nstarting with 64 MB right away if a write failed.\n\n>> +                     size /= 2;\n>> +             } else {\n>> +                     buf += written;\n>> +                     total += written;\n>\n> What about other errors?  You need to break out of the loop instead of\n> adding -1 to buf and total, right?\n\nRight, I've fixed that, thanks.\n\n>> +                     if (total + size > count)\n>> +                             size = count - total;\n>> +             }\n>> +     }\n>\n> Shouldn't the loop be left in the successful case, too?  write(2) is\n> allowed to write less than requested, so the caller already needs to\n> deal with that case anyway.\n\nI prefer to make the wrapper as transparent as possible. If a direct\ncall to write would not write less than requested, the wrapper should\nnot either.\n\nI've updated work/issue-409 in 4msysgit.git accordingly.\n\n-- \nSebastian Schuberth\n"},{"id":"139971","messageId":"4BCDA49C.4090405@viscovery.net","threadId":"23525","inReplyTo":"t2xbdca99241004200542ud4e8ea5azcad918c37bcacf1a@mail.gmail.com","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-04-20T12:57:00Z","receivedAt":"2010-04-20T12:57:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 4/20/2010 14:42, schrieb Sebastian Schuberth:\n> On Mon, Apr 19, 2010 at 22:43, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:\n>> Shouldn't the loop be left in the successful case, too?  write(2) is\n>> allowed to write less than requested, so the caller already needs to\n>> deal with that case anyway.\n> \n> I prefer to make the wrapper as transparent as possible. If a direct\n> call to write would not write less than requested, the wrapper should\n> not either.\n\nSure, but René meant the opposite case: When fewer bytes than requested\nwere written, then you shouldn't retry to write more! That is, you should\nexit the loop when write(fd, buf, n) does not return n.\n\nI still find your code unnecessarily hard to read. In particular, you\nshould extract the non-problematic case out of the loop. If you followed\nmy suggestion elsewhere in the thread, you wouldn't have to write any\nconditionals that 'break' out of a loop.\n\n-- Hannes\n"},{"id":"139975","messageId":"i2kbdca99241004200721l7cbd0052oa6921533f7efb8a4@mail.gmail.com","threadId":"23525","inReplyTo":"4BCDA49C.4090405@viscovery.net","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2010-04-20T14:21:36Z","receivedAt":"2010-04-20T14:21:36Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Tue, Apr 20, 2010 at 14:57, Johannes Sixt <j.sixt@viscovery.net> wrote:\n\n> Am 4/20/2010 14:42, schrieb Sebastian Schuberth:\n>> On Mon, Apr 19, 2010 at 22:43, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:\n>>> Shouldn't the loop be left in the successful case, too?  write(2) is\n>>> allowed to write less than requested, so the caller already needs to\n>>> deal with that case anyway.\n>>\n>> I prefer to make the wrapper as transparent as possible. If a direct\n>> call to write would not write less than requested, the wrapper should\n>> not either.\n>\n> Sure, but René meant the opposite case: When fewer bytes than requested\n> were written, then you shouldn't retry to write more! That is, you should\n> exit the loop when write(fd, buf, n) does not return n.\n\nI see what you mean, but I do not fully agree. Who guarantees that (on\nsome obscure OS) a following call to write(fd, buf, n) will not return\nn again, maybe because write() temporarily decided to write fewer\nbytes than requested to make the next write() call do aligned writes\nto something? That case then is probably already handled in the caller\nto write(), but at least my code is not wrong in that respect.\n\n> I still find your code unnecessarily hard to read. In particular, you\n> should extract the non-problematic case out of the loop. If you followed\n> my suggestion elsewhere in the thread, you wouldn't have to write any\n> conditionals that 'break' out of a loop.\n\nI didn't follow your suggestion on purpose because I experimented with\nit and I found *yours* to be hard to read. It has three calls to\nwrite() and more places where errors need to be checked. As I do not\nhave the will to waste more time on style discussions about code that\nfixes other people's issues, and not the time to test the code on\nWindows XP over and over again, I hope you are willing to accept code\nthat is different from how you would have written it. So it's take it\nor leave it (or modify it yourself, if you feel like it).\n\n-- \nSebastian Schuberth\n"},{"id":"139995","messageId":"4BCE134C.8090802@lsrfire.ath.cx","threadId":"23525","inReplyTo":"t2xbdca99241004200542ud4e8ea5azcad918c37bcacf1a@mail.gmail.com","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-04-20T20:49:16Z","receivedAt":"2010-04-20T20:49:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.04.2010 14:42, schrieb Sebastian Schuberth:\n>> Shouldn't the loop be left in the successful case, too?  write(2) is\n>> allowed to write less than requested, so the caller already needs to\n>> deal with that case anyway.\n> \n> I prefer to make the wrapper as transparent as possible. If a direct\n> call to write would not write less than requested, the wrapper should\n> not either.\n\nAfter the call failed, we don't know how many bytes would have been\nwritten had it succeeded.\n\nBut I agree with Albert's reasoning to use the knowledge of a working\nchunk size in order to minimize the number of write(2) calls.  Otherwise\nwe'd have to search for a working size again and again, generating lots\nof failing calls.\n\n> I've updated work/issue-409 in 4msysgit.git accordingly.\n\nThis patch doesn't help in the test case I cobbled together quickly.\nIt's a Windows XP SP3 client on VMWare mapping a file share exported by\na Netapps filer, over a VPN.  It's very slow, and I admit that it's a\nweird setup.  I wouldn't actually use it that way, but couldn't find\nanother file share momentarily.\n\nI can check out a 1MB file, but checking out a 32MB file fails.  I've\nadded a fprintf() to the loop and I can see that it's halving the size\nand retries, as intended, until it eventually hits zero.\n\nThe file is created using the correct file size (32MB), though.The first\nfailed write(2) call needs to be undone somehow before we can try again,\nit seems.  Do we have to seek back or truncate the file?\n\nReplacing the body of mingw_write() with the following line allows me to\ncheck out the 32MB file, by the way:\n\n\treturn write(fd, buf, min(count, 1024 * 1024));\n\nRené\n"},{"id":"140623","messageId":"4BD9E58E.8020406@lsrfire.ath.cx","threadId":"23525","inReplyTo":"4BCE134C.8090802@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-04-29T20:01:18Z","receivedAt":"2010-04-29T20:01:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.04.2010 22:49, schrieb René Scharfe:\n> Am 20.04.2010 14:42, schrieb Sebastian Schuberth:\n>> I've updated work/issue-409 in 4msysgit.git accordingly.\n> \n> This patch doesn't help in the test case I cobbled together quickly.\n> It's a Windows XP SP3 client on VMWare mapping a file share exported by\n> a Netapps filer, over a VPN.  It's very slow, and I admit that it's a\n> weird setup.  I wouldn't actually use it that way, but couldn't find\n> another file share momentarily.\n> \n> I can check out a 1MB file, but checking out a 32MB file fails.  I've\n> added a fprintf() to the loop and I can see that it's halving the size\n> and retries, as intended, until it eventually hits zero.\n> \n> The file is created using the correct file size (32MB), though.The first\n> failed write(2) call needs to be undone somehow before we can try again,\n> it seems.  Do we have to seek back or truncate the file?\n> \n> Replacing the body of mingw_write() with the following line allows me to\n> check out the 32MB file, by the way:\n> \n> \treturn write(fd, buf, min(count, 1024 * 1024));\n\nOK, I've been searching the web looking for documentation that explains\nthe issue, but haven't found any watertight evidence.\n\nWith Process Monitor [1], I found out that local write() calls are\ntranslated to WriteFile() calls with a maximum size of 64KB on Windows XP\nand 256KB on Vista.  Writes to a network drive are translated to\nWriteFile() calls without applying a size cap.\n\nIn a forum discussion [2], someone explained that unbuffered writes using\nWriteFile() have certain size limits, depending on the processor:\n\n\tx86:  67,076,096\n\tX64:  33,525,760\n\tIA64: 67,051,520\n\nThis post is cited by an MS employee in the comment section of the online\ndocumentation for WriteFile() [3].\n\nA Knowledge Base article [4] documents a size limit of 67,076,095 for\nfwrite() to a file on a network drive.  fwrite() calls translate to\nWriteFile() calls, too.  The reason for this is said to be an OS problem,\nbut no OS versions are named at all in this article.\n\nPlease note that the size limit of fwrite() is suspiciously close to the\none for an unbuffered WriteFile().\n\nIn my test setup, write() calls to a network drive fail with a size\nparameter of 32MB, while with 33,525,760 bytes they succeed.\n\nBased on these two observations, I suspect that there's a connection\nbetween writes to a network drive and unbuffered writes.  Perhaps writes\nover the net are passed to the NIC driver in one piece which is locked\ninto RAM?\n\nThe connection is a bit weak (it would be good to have someone comment on\nthis who actually knows something about this topic and doesn't have to\nguesswork through a bunch of websites), but I think it's enough to create\na patch.  Based on the numbers above, I think 31MB is a good size to\nsimply cap writes.\n\nWhen we learn that other systems need a lower limit still, we can easily\nreduce our cap, without affecting local performance.\n\nIt would be nice to reach chris.gcode, who originally reported this\nproblem [5], and ask him to test.  I couldn't find an email address on\nthat webpage, though.  His proposed patch there also used an upper limit\nslightly below 32MB, but tried to compensate for capping by looping until\nthe full requested size was written.  That's not really needed.\n\nRené\n\n\n[1] http://technet.microsoft.com/en-us/sysinternals/bb896645.aspx\n[2] http://social.msdn.microsoft.com/Forums/en-US/vcgeneral/thread/fef1c9b5-fd92-4ada-8de5-44c2eb30b516\n[3] http://msdn.microsoft.com/en-us/library/aa365747(VS.85).aspx#5\n[4] http://support.microsoft.com/kb/899149\n[5] http://code.google.com/p/msysgit/issues/detail?id=409\n\n-- >8 --\nBigger writes to network drives on Windows XP fail.  Cap them at 31MB to\nallow them to succeed.  Callers need to be prepared for write() calls\nthat do less work than requested anyway.\n\nOn local drives, write() calls are translated to WriteFile() calls with\na cap of 64KB on Windows XP and 256KB on Vista.  Thus a cap of 31MB won't\naffect the number of WriteFile() calls which do the actual work.  There's\nstill room for some other version of Windows to use a chunk size of 1MB\nwithout increasing the number of system calls.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n compat/mingw.c |   17 +++++++++++++++++\n compat/mingw.h |    3 +++\n 2 files changed, 20 insertions(+), 0 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex f90a114..9a8e336 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -140,6 +140,23 @@ int mingw_open (const char *filename, int oflags, ...)\n \treturn fd;\n }\n \n+#undef write\n+ssize_t mingw_write(int fd, const void *buf, size_t count)\n+{\n+\t/*\n+\t * While write() calls to a file on a local disk are translated\n+\t * into WriteFile() calls with a maximum size of 64KB on Windows\n+\t * XP and 256KB on Vista, no such cap is placed on writes to\n+\t * files over the network on Windows XP.  Unfortunately, there\n+\t * seems to be a limit of 32MB-28KB on X64 and 64MB-32KB on x86;\n+\t * bigger writes fail on Windows XP.\n+\t * So we cap to a nice 31MB here to avoid write failures over\n+\t * the net without changing the number of WriteFile() calls in\n+\t * the local case.\n+\t */\n+\treturn write(fd, buf, min(count, 31 * 1024 * 1024));\n+}\n+\n #undef fopen\n FILE *mingw_fopen (const char *filename, const char *otype)\n {\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 7c2ab64..0e3e743 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -170,6 +170,9 @@ int link(const char *oldpath, const char *newpath);\n int mingw_open (const char *filename, int oflags, ...);\n #define open mingw_open\n \n+ssize_t mingw_write(int fd, const void *buf, size_t count);\n+#define write mingw_write\n+\n FILE *mingw_fopen (const char *filename, const char *otype);\n #define fopen mingw_fopen\n \n-- \n1.7.1\n"},{"id":"140624","messageId":"4BDA98FC.1080007@viscovery.net","threadId":"23525","inReplyTo":"4BD9E58E.8020406@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-04-30T08:46:52Z","receivedAt":"2010-04-30T08:46:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 4/29/2010 22:01, schrieb René Scharfe:\n> +#undef write\n> +ssize_t mingw_write(int fd, const void *buf, size_t count)\n> +{\n> +\t/*\n> +\t * While write() calls to a file on a local disk are translated\n> +\t * into WriteFile() calls with a maximum size of 64KB on Windows\n> +\t * XP and 256KB on Vista, no such cap is placed on writes to\n> +\t * files over the network on Windows XP.  Unfortunately, there\n> +\t * seems to be a limit of 32MB-28KB on X64 and 64MB-32KB on x86;\n> +\t * bigger writes fail on Windows XP.\n> +\t * So we cap to a nice 31MB here to avoid write failures over\n> +\t * the net without changing the number of WriteFile() calls in\n> +\t * the local case.\n> +\t */\n> +\treturn write(fd, buf, min(count, 31 * 1024 * 1024));\n> +}\n> +\n\nThanks, I have verified that this fixes the problem in my setup as well.\nI'll queue the patch with the below test case squashed in.\n\n-- Hannes\n\ndiff --git a/t/t5705-clone-2gb.sh b/t/t5705-clone-2gb.sh\nindex adfaae8..8afbdd4 100755\n--- a/t/t5705-clone-2gb.sh\n+++ b/t/t5705-clone-2gb.sh\n@@ -12,7 +12,7 @@ test_expect_success 'setup' '\n\n \tgit config pack.compression 0 &&\n \tgit config pack.depth 0 &&\n-\tblobsize=$((20*1024*1024)) &&\n+\tblobsize=$((100*1024*1024)) &&\n \tblobcount=$((2*1024*1024*1024/$blobsize+1)) &&\n \ti=1 &&\n \t(while test $i -le $blobcount\n@@ -36,9 +36,15 @@ test_expect_success 'setup' '\n\n '\n\n-test_expect_success 'clone' '\n+test_expect_success 'clone - bare' '\n\n-\tgit clone --bare --no-hardlinks . clone\n+\tgit clone --bare --no-hardlinks . clone-bare\n+\n+'\n+\n+test_expect_success 'clone - with worktree, file:// protocol' '\n+\n+\tgit clone file://. clone-wt\n\n '\n"},{"id":"140625","messageId":"j2tbdca99241004300208qf03f0a5u9e497119e3d42924@mail.gmail.com","threadId":"23525","inReplyTo":"4BD9E58E.8020406@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2010-04-30T09:08:55Z","receivedAt":"2010-04-30T09:08:55Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Thu, Apr 29, 2010 at 22:01, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:\n\n> OK, I've been searching the web looking for documentation that explains\n> the issue, but haven't found any watertight evidence.\n\nThanks René for your research and for finally finishing the fix!\n\n> It would be nice to reach chris.gcode, who originally reported this\n> problem [5], and ask him to test.  I couldn't find an email address on\n> that webpage, though.  His proposed patch there also used an upper limit\n> slightly below 32MB, but tried to compensate for capping by looping until\n> the full requested size was written.  That's not really needed.\n\nI believe to have found his e-mail address on the msysGit mailing list\nand put him on CC.\n\n-- \nSebastian Schuberth\n"},{"id":"153631","messageId":"4CB9DF7E.1020203@lsrfire.ath.cx","threadId":"23525","inReplyTo":"290b11b5-5dd5-4b83-a6f5-217797ebd5af@t8g2000yqk.googlegroups.com","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2010-10-16T17:23:10Z","receivedAt":"2010-10-16T17:23:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Hi,\n\nI've cc:'d the mailing lists and the participants of the earlier\ndiscussion to share your findings and to see if someone else may have an\nexplanation for this behaviour, or a solution.\n\nThe patch in question is c8b296450e5148c576697ea4709072b7855aacd5 and\nhas made it into git versions 1.7.1.1 and 1.7.2 (and up).  It caps\nwrites at 31MB to fix a problem with network drives in certain versions\nof Windows.\n\nAm 14.10.2010 21:26, schrieb Chad Warner:\n> I am experiencing issues with the patch you suggested.  I am working\n> with a repository via its UNC path and it is failing on large files.\n> I first tried lowing to several different values such as 4MB and still\n> had problems.  I eventually got it to work with the following:\n> \n> return write(fd, buf, min(count, 1024 * 27));\n> \n> I didn't notice any real delays with having to call write that many\n> more times.  However, I really don't know how to go about fixing this\n> issue or validating that this really fixes the problem.\n\nReducing the write cap to 27KB unconditionally sounds pretty drastic to\nme; it could hurt the local case.  I didn't measure it, though, so I\nmight be wrong.  Depending on that measurement we perhaps need to find a\ndifferent solution.\n\nAlso, 27KB is an odd number.  I take it that 28KB won't fix the issue\nfor you?\n\nI wonder about the cause of this threshold.  Wild guess: network driver\nbug or other network issue?\n\nWhich version of Windows do you use on the client?  Which OS runs on the\nserver?  Do you have any other information that might help in\nreproducing the problem?\n\nThanks,\nRené\n"},{"id":"153667","messageId":"AANLkTinpwpckOn3BEHWn=4Tq_kRyOYYmYF5UQQ0qSpmS@mail.gmail.com","threadId":"23525","inReplyTo":"4CB9DF7E.1020203@lsrfire.ath.cx","subject":"Re: [PATCH] Fix checkout of large files to network shares under Windows XP","fromName":"Dmitry Potapov","fromEmail":"dpotapov@gmail.com","sentAt":"2010-10-17T10:54:40Z","receivedAt":"2010-10-17T10:54:40Z","isPatch":true,"sender":{"key":"dpotapov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6568595?v=4"},"body":"On Sat, Oct 16, 2010 at 9:23 PM, René Scharfe\n<rene.scharfe@lsrfire.ath.cx> wrote:\n>\n> Also, 27KB is an odd number.  I take it that 28KB won't fix the issue\n> for you?\n\nI guess the beginning of the written buffer is not page aligned, and the\nsystem locks pages. Thus depending on the buffer alignment, the written\nmaximum can vary up to the size of one page.\n\n>\n> I wonder about the cause of this threshold.  Wild guess: network driver\n> bug or other network issue?\n\nMemory pressure? Git may consume large amount of virtual memory\nduring some operations, but I am not sure how it is translated in the\nnumber of physical memory consumed by the process as well as what\nquotas could be on that system. It would be interesting to see what\nGetProcessMemoryInfo() returns when write() fails.\n\n\nDmitry\n"}]}