{"thread":{"id":"4780","subject":"comparing file contents in is_exact_match?","startedAt":"2006-07-06T05:57:29Z","lastAt":"2006-07-18T18:49:48Z","messageCount":32,"participants":["Martin Waitz","Junio C Hamano","Florian Weimer","Johannes Schindelin","Yakov Lerner","Alex Riesen","Linus Torvalds","Juergen Ruehle","Geert Bosch"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"23302","messageId":"20060706055729.GA12512@admingilde.org","threadId":"4780","inReplyTo":null,"subject":"comparing file contents in is_exact_match?","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-07-06T05:57:29Z","receivedAt":"2006-07-06T05:57:29Z","isPatch":false,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nI created a git repository for my photo collection and then renamed\nsome photos (about 600). Now git status and commit get hit by\nthe OOM killer.\n\nThe reason for that is that is_exact_match (in diffcore-rename.c) maps\nboth the source and destination file into memory and then compares them\nbyte for byte. This is a little bit too much for my little machine.\n\nWhen I remove the content comparation and only leave the sha1\ncomparision, then my renames are correctly found in a second. But\nunluckily, some other renames in the testcases are not correctly\nhandled any more.\n\nSo is there an easy solution? Why is the content comparision in\nis_exact_match needed?\n\n-- \nMartin Waitz\n"},{"id":"23303","messageId":"7vd5cj9rvm.fsf@assigned-by-dhcp.cox.net","threadId":"4780","inReplyTo":"20060706055729.GA12512@admingilde.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-06T06:20:13Z","receivedAt":"2006-07-06T06:20:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz writes:\n\n> The reason for that is that is_exact_match (in diffcore-rename.c) maps\n> both the source and destination file into memory and then compares them\n> byte for byte. This is a little bit too much for my little machine.\n>\n> When I remove the content comparation and only leave the sha1\n> comparision, then my renames are correctly found in a second. But\n> unluckily, some other renames in the testcases are not correctly\n> handled any more.\n>\n> So is there an easy solution? Why is the content comparision in\n> is_exact_match needed?\n\nBecause your working tree can be out of sync with respect to\nwhat's in the index, in which case we cannot trust the sha1\nwhile running diff-index (without --cached flag).\n\ngit-update-index --refresh before doing anything might help.\n"},{"id":"23306","messageId":"20060706071629.GB12512@admingilde.org","threadId":"4780","inReplyTo":"7vd5cj9rvm.fsf@assigned-by-dhcp.cox.net","subject":"Re: comparing file contents in is_exact_match?","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-07-06T07:16:29Z","receivedAt":"2006-07-06T07:16:29Z","isPatch":false,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Wed, Jul 05, 2006 at 11:20:13PM -0700, Junio C Hamano wrote:\n> Martin Waitz writes:\n> > Why is the content comparision in is_exact_match needed?\n> \n> Because your working tree can be out of sync with respect to\n> what's in the index, in which case we cannot trust the sha1\n> while running diff-index (without --cached flag).\n\nso perhaps we need three phases instead of two:\nfirst sort out all renames that can be detected by the sha1,\nthen compare file contents and finally do the diff.\n\n> git-update-index --refresh before doing anything might help.\n\nAt the moment it is doing (N-1)^2 content compares, even if\nthe index is in sync.\n\n-- \nMartin Waitz\n"},{"id":"23307","messageId":"7vwtar89wy.fsf@assigned-by-dhcp.cox.net","threadId":"4780","inReplyTo":"20060706071629.GB12512@admingilde.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-06T07:33:33Z","receivedAt":"2006-07-06T07:33:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Waitz writes:\n\n>> Because your working tree can be out of sync with respect to\n>> what's in the index, in which case we cannot trust the sha1\n>> while running diff-index (without --cached flag).\n>\n> so perhaps we need three phases instead of two:\n> first sort out all renames that can be detected by the sha1,\n> then compare file contents and finally do the diff.\n\nMakes sort-of sense.\n\nAlthough I am not sure how much this would help with a regular\nworkload, maybe something like this untested patch might help\nyour situation?\n\n-- >8 --\ndiffcore-rename: try matching up renames without populating filespec first\n\nSigned-off-by: Junio C Hamano \n\n---\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex d57e865..affff7a 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -96,11 +96,15 @@ static struct diff_rename_src *register_\n \treturn &(rename_src[first]);\n }\n \n-static int is_exact_match(struct diff_filespec *src, struct diff_filespec *dst)\n+static int is_exact_match(struct diff_filespec *src,\n+\t\t\t struct diff_filespec *dst,\n+\t\t\t int contents_too)\n {\n \tif (src->sha1_valid && dst->sha1_valid &&\n \t !memcmp(src->sha1, dst->sha1, 20))\n \t\treturn 1;\n+\tif (!contents_too)\n+\t\treturn 0;\n \tif (diff_populate_filespec(src, 1) || diff_populate_filespec(dst, 1))\n \t\treturn 0;\n \tif (src->size != dst->size)\n@@ -242,7 +246,7 @@ void diffcore_rename(struct diff_options\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct diff_queue_struct outq;\n \tstruct diff_score *mx;\n-\tint i, j, rename_count;\n+\tint i, j, rename_count, contents_too;\n \tint num_create, num_src, dst_cnt;\n \n \tif (!minimum_score)\n@@ -273,16 +277,23 @@ void diffcore_rename(struct diff_options\n \n \t/* We really want to cull the candidates list early\n \t * with cheap tests in order to avoid doing deltas.\n+\t * The first round matches up the up-to-date entries,\n+\t * and then during the second round we try to match\n+\t * cache-dirty entries as well.\n \t */\n-\tfor (i = 0; i < rename_dst_nr; i++) {\n-\t\tstruct diff_filespec *two = rename_dst[i].two;\n-\t\tfor (j = 0; j < rename_src_nr; j++) {\n-\t\t\tstruct diff_filespec *one = rename_src[j].one;\n-\t\t\tif (!is_exact_match(one, two))\n-\t\t\t\tcontinue;\n-\t\t\trecord_rename_pair(i, j, MAX_SCORE);\n-\t\t\trename_count++;\n-\t\t\tbreak; /* we are done with this entry */\n+\tfor (contents_too = 0; contents_too < 2; contents_too++) { \n+\t\tfor (i = 0; i < rename_dst_nr; i++) {\n+\t\t\tstruct diff_filespec *two = rename_dst[i].two;\n+\t\t\tif (rename_dst[i].pair)\n+\t\t\t\tcontinue; /* dealt with an earlier round */\n+\t\t\tfor (j = 0; j < rename_src_nr; j++) {\n+\t\t\t\tstruct diff_filespec *one = rename_src[j].one;\n+\t\t\t\tif (!is_exact_match(one, two, contents_too))\n+\t\t\t\t\tcontinue;\n+\t\t\t\trecord_rename_pair(i, j, MAX_SCORE);\n+\t\t\t\trename_count++;\n+\t\t\t\tbreak; /* we are done with this entry */\n+\t\t\t}\n \t\t}\n \t}\n \n"},{"id":"23308","messageId":"20060706074134.GC12512@admingilde.org","threadId":"4780","inReplyTo":"7vwtar89wy.fsf@assigned-by-dhcp.cox.net","subject":"Re: comparing file contents in is_exact_match?","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-07-06T07:41:34Z","receivedAt":"2006-07-06T07:41:34Z","isPatch":false,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"On Thu, Jul 06, 2006 at 12:33:33AM -0700, Junio C Hamano wrote:\n> Martin Waitz writes:\n> > so perhaps we need three phases instead of two:\n> > first sort out all renames that can be detected by the sha1,\n> > then compare file contents and finally do the diff.\n> \n> Makes sort-of sense.\n> \n> Although I am not sure how much this would help with a regular\n> workload, maybe something like this untested patch might help\n> your situation?\n\npatch looks good, I'll try it in the evening.\n\n-- \nMartin Waitz\n"},{"id":"23329","messageId":"20060706175545.GD12512@admingilde.org","threadId":"4780","inReplyTo":"7vwtar89wy.fsf@assigned-by-dhcp.cox.net","subject":"Re: comparing file contents in is_exact_match?","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-07-06T17:55:45Z","receivedAt":"2006-07-06T17:55:45Z","isPatch":false,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Thu, Jul 06, 2006 at 12:33:33AM -0700, Junio C Hamano wrote:\n> Although I am not sure how much this would help with a regular\n> workload, maybe something like this untested patch might help\n> your situation?\n\nworks perfectly!\nThanks!\n\n-- \nMartin Waitz\n"},{"id":"23379","messageId":"87k66p8jee.fsf@mid.deneb.enyo.de","threadId":"4780","inReplyTo":"20060706055729.GA12512@admingilde.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2006-07-07T16:33:13Z","receivedAt":"2006-07-07T16:33:13Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Martin Waitz:\n\n> I created a git repository for my photo collection and then renamed\n> some photos (about 600). Now git status and commit get hit by\n> the OOM killer.\n>\n> The reason for that is that is_exact_match (in diffcore-rename.c) maps\n> both the source and destination file into memory and then compares them\n> byte for byte. This is a little bit too much for my little machine.\n\nUhm, this shouldn't trigger the OOM killer, really. You already have\nphysical backing storage for both files, so this shouldn't count\ntowards the OOM limit. Ah, diff_populate_filespec has the following:\n\n s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n\nPerhaps the following patch is in order? On some systems, MAP_PRIVATE\nmight guarantee some form of repeatable reads, but I don't think GIT\nneeds this to guard against concurrent modification.\n\n-- >8 --\ndiff_populate_filespec: use shared mapping\n\nIt seems that on some systems, PROT_READ + MAP_PRIVATE counts towards\nthe OOM limit, even though no additional backing store is required.\nRequesting MAP_SHARED mapping should fix this.\n\nSigned-off-by: Florian Weimer \n---\n diff.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 428ff78..2b4367e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1007,7 +1007,7 @@ int diff_populate_filespec(struct diff_f\n fd = open(s->path, O_RDONLY);\n if (fd < 0)\n goto err_empty;\n- s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+ s->data = mmap(NULL, s->size, PROT_READ, MAP_SHARED, fd, 0);\n close(fd);\n if (s->data == MAP_FAILED)\n goto err_empty;\n--\n1.4.0\n"},{"id":"23404","messageId":"Pine.LNX.4.63.0607080450100.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4780","inReplyTo":"87k66p8jee.fsf@mid.deneb.enyo.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-08T02:50:59Z","receivedAt":"2006-07-08T02:50:59Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 7 Jul 2006, Florian Weimer wrote:\n\n> - s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n> + s->data = mmap(NULL, s->size, PROT_READ, MAP_SHARED, fd, 0);\n\nBe advised that this breaks setups with NO_MMAP, in particular most (all) \ncygwin setups I know of.\n\nHth,\nDscho\n"},{"id":"23875","messageId":"87fyh1ncm0.fsf@mid.deneb.enyo.de","threadId":"4780","inReplyTo":"Pine.LNX.4.63.0607080450100.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2006-07-16T09:05:43Z","receivedAt":"2006-07-16T09:05:43Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Johannes Schindelin:\n\n> Hi,\n>\n> On Fri, 7 Jul 2006, Florian Weimer wrote:\n>\n>> - s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n>> + s->data = mmap(NULL, s->size, PROT_READ, MAP_SHARED, fd, 0);\n>\n> Be advised that this breaks setups with NO_MMAP, in particular most (all) \n> cygwin setups I know of.\n\nAre these Cygwin setups running on top of the Windows 95 code base by\nchance?\n"},{"id":"23878","messageId":"Pine.LNX.4.63.0607161559010.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4780","inReplyTo":"87fyh1ncm0.fsf@mid.deneb.enyo.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-16T14:00:03Z","receivedAt":"2006-07-16T14:00:03Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 16 Jul 2006, Florian Weimer wrote:\n\n> * Johannes Schindelin:\n> \n> > Hi,\n> >\n> > On Fri, 7 Jul 2006, Florian Weimer wrote:\n> >\n> >> - s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n> >> + s->data = mmap(NULL, s->size, PROT_READ, MAP_SHARED, fd, 0);\n> >\n> > Be advised that this breaks setups with NO_MMAP, in particular most (all) \n> > cygwin setups I know of.\n> \n> Are these Cygwin setups running on top of the Windows 95 code base by\n> chance?\n\nNo. One is Windows2000, the other WindowsXP, and both need the NO_MMAP \nset. For obvious reasons, NO_MMAP does not support MAP_SHARED...\n\nCiao,\nDscho\n"},{"id":"23879","messageId":"f36b08ee0607160803s27dac6a6k476e3dd7742346fc@mail.gmail.com","threadId":"4780","inReplyTo":"87fyh1ncm0.fsf@mid.deneb.enyo.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Yakov Lerner","fromEmail":"iler.ml@gmail.com","sentAt":"2006-07-16T15:03:49Z","receivedAt":"2006-07-16T15:03:49Z","isPatch":false,"sender":{"key":"iler.ml@gmail.com","avatar":null},"body":"On 7/16/06, Florian Weimer wrote:\n> * Johannes Schindelin:\n>\n> > Hi,\n> >\n> > On Fri, 7 Jul 2006, Florian Weimer wrote:\n> >\n> >> - s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n> >> + s->data = mmap(NULL, s->size, PROT_READ, MAP_SHARED, fd, 0);\n> >\n> > Be advised that this breaks setups with NO_MMAP, in particular most (all)\n> > cygwin setups I know of.\n>\n> Are these Cygwin setups running on top of the Windows 95 code base by\n> chance?\n\nCygwin has mmap. But cygwin's mmap() not good enough for git.\nWhat happens is that git does rename() when target file has active mmap().\nIn cygwin, this makes rename() to fail. This is what makes cygwin's\nmmap unusable for git. (BTW for read-only git access, mmap() will work\non cygwin, for what I saw. But attempts to modify index will break).\n\nYakov\n"},{"id":"23884","messageId":"20060716223607.GA6023@steel.home","threadId":"4780","inReplyTo":"f36b08ee0607160803s27dac6a6k476e3dd7742346fc@mail.gmail.com","subject":"Re: comparing file contents in is_exact_match?","fromName":"Alex Riesen","fromEmail":"fork0@t-online.de","sentAt":"2006-07-16T22:36:07Z","receivedAt":"2006-07-16T22:36:07Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Yakov Lerner, Sun, Jul 16, 2006 17:03:49 +0200:\n> Cygwin has mmap. But cygwin's mmap() not good enough for git.\n> What happens is that git does rename() when target file has active mmap().\n> In cygwin, this makes rename() to fail. This is what makes cygwin's\n> mmap unusable for git. (BTW for read-only git access, mmap() will work\n> on cygwin, for what I saw. But attempts to modify index will break).\n\nIt is not Cygwin really. It's windows. You can't rename or delete an\nopen or mmapped file in that thing.\n"},{"id":"23893","messageId":"87d5c4ajlu.fsf@mid.deneb.enyo.de","threadId":"4780","inReplyTo":"20060716223607.GA6023@steel.home","subject":"Re: comparing file contents in is_exact_match?","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2006-07-17T05:25:17Z","receivedAt":"2006-07-17T05:25:17Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Alex Riesen:\n\n> It is not Cygwin really. It's windows. You can't rename or delete an\n> open or mmapped file in that thing.\n\nAnd GIT's workaround is to read the whole file into memory and close\nit after that? Uh-oh.\n"},{"id":"23895","messageId":"f36b08ee0607170511u97b1c35s546a47bf8035eb70@mail.gmail.com","threadId":"4780","inReplyTo":"20060716223607.GA6023@steel.home","subject":"Re: comparing file contents in is_exact_match?","fromName":"Yakov Lerner","fromEmail":"iler.ml@gmail.com","sentAt":"2006-07-17T12:11:27Z","receivedAt":"2006-07-17T12:11:27Z","isPatch":false,"sender":{"key":"iler.ml@gmail.com","avatar":null},"body":"Florian Weimer wrote:\n> And GIT's workaround is to read the whole file into memory and close\n> it after that? Uh-oh.\n\nOn 7/16/06, Alex Riesen wrote:\n> Yakov Lerner, Sun, Jul 16, 2006 17:03:49 +0200:\n> > Cygwin has mmap. But cygwin's mmap() not good enough for git.\n> > What happens is that git does rename() when target file has active mmap().\n> > In cygwin, this makes rename() to fail. This is what makes cygwin's\n> > mmap unusable for git. (BTW for read-only git access, mmap() will work\n> > on cygwin, for what I saw. But attempts to modify index will break).\n>\n> It is not Cygwin really. It's windows. You can't rename or delete an\n> open or mmapped file in that thing.\n\nWe have right to expect cygwin to do some effort to emulate the unix\nmmap() semantics on top of different semantics of windows memory-mapped files.\nWhy doesn't cygwin do some effort to detect active mmap()s on the file\nbefore rename and do something like (1) unmap (2) rename the target to temp name\n(3) remap. (4) take care of temp file cleanup ? Probably too messy even if\npossible. Is it reasonable to expect git to do this mess ?\nIt probably belongs to the generic cygwin layer anyway.\n\nI did try another kind of workaround, in which I unmapped the target file\nbefore rename. But it didn't work in all situations and was too intrusive\nto the git-apply logic. Didn't work.\n\nRegarding the existing workaround (NO_MMAP), well, cygwin is not exactly a\nspeed champion in many areas. I didn't really notice speed loss with\nNO_MMAP, maybe because cygwin loses time here and there anyway ?\n\nYakov\n"},{"id":"23896","messageId":"Pine.LNX.4.63.0607171439270.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4780","inReplyTo":"87d5c4ajlu.fsf@mid.deneb.enyo.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-17T12:41:32Z","receivedAt":"2006-07-17T12:41:32Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Jul 2006, Florian Weimer wrote:\n\n> * Alex Riesen:\n> \n> > It is not Cygwin really. It's windows. You can't rename or delete an\n> > open or mmapped file in that thing.\n> \n> And GIT's workaround is to read the whole file into memory and close\n> it after that? Uh-oh.\n\nIf you have a better idea (which does not make git source code ugly), go \nahead, write a patch.\n\nBut note that we usually use the whole contents of the mmap()ed file \nanyway.\n\nCiao,\nDscho\n"},{"id":"23900","messageId":"Pine.LNX.4.64.0607170840280.15611@evo.osdl.org","threadId":"4780","inReplyTo":"Pine.LNX.4.63.0607171439270.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-17T15:43:09Z","receivedAt":"2006-07-17T15:43:09Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 17 Jul 2006, Johannes Schindelin wrote:\n> \n> But note that we usually use the whole contents of the mmap()ed file \n> anyway.\n\nNot for pack-files, though. You may have a half-gigabyte pack-file, and \nonly use a small small fraction of it.\n\n(You also never really rename them, so windows should be fine for that \ncase)\n\n\t\tLinus\n"},{"id":"23901","messageId":"Pine.LNX.4.63.0607171804030.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4780","inReplyTo":"Pine.LNX.4.64.0607170840280.15611@evo.osdl.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-17T16:05:27Z","receivedAt":"2006-07-17T16:05:27Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Jul 2006, Linus Torvalds wrote:\n\n> On Mon, 17 Jul 2006, Johannes Schindelin wrote:\n> > \n> > But note that we usually use the whole contents of the mmap()ed file \n> > anyway.\n> \n> Not for pack-files, though. You may have a half-gigabyte pack-file, and \n> only use a small small fraction of it.\n\nRight.\n\n> (You also never really rename them, so windows should be fine for that \n> case)\n\nSo we could introduce a second mmap() which is normally the same as \nmmap(), except for windows, where it is a rename() and unlink() safe \nversion of mmap() by reading the thing into RAM. Not very pretty.\n\nCiao,\nDscho\n"},{"id":"23902","messageId":"17595.48003.145000.414361@lapjr.intranet.kiel.bmiag.de","threadId":"4780","inReplyTo":"Pine.LNX.4.63.0607171439270.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2006-07-17T16:32:03Z","receivedAt":"2006-07-17T16:32:03Z","isPatch":false,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"Johannes Schindelin writes:\n > > > It is not Cygwin really. It's windows. You can't rename or delete an\n > > > open or mmapped file in that thing.\n > > \n > > And GIT's workaround is to read the whole file into memory and close\n > > it after that? Uh-oh.\n > \n > If you have a better idea (which does not make git source code ugly), go \n > ahead, write a patch.\n\nOn several boxes I've tested the mmap code passes the tests on NTFS.\n\nIt is usable for my simple work even on FAT32 with an unlink before\nthe rename in lockfile.c, but that probably breaks in more involved\nscenarios.\n\n jr\n"},{"id":"23903","messageId":"Pine.LNX.4.63.0607171907080.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4780","inReplyTo":"17595.48003.145000.414361@lapjr.intranet.kiel.bmiag.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-17T17:10:07Z","receivedAt":"2006-07-17T17:10:07Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Jul 2006, Juergen Ruehle wrote:\n\n> Johannes Schindelin writes:\n> > > > It is not Cygwin really. It's windows. You can't rename or delete an\n> > > > open or mmapped file in that thing.\n> > > \n> > > And GIT's workaround is to read the whole file into memory and close\n> > > it after that? Uh-oh.\n> > \n> > If you have a better idea (which does not make git source code ugly), go \n> > ahead, write a patch.\n> \n> On several boxes I've tested the mmap code passes the tests on NTFS.\n> \n> It is usable for my simple work even on FAT32 with an unlink before\n> the rename in lockfile.c, but that probably breaks in more involved\n> scenarios.\n\nIn my not-so-simple tests, it failed. Reproducibly. If I am not mistaken, \nyour test just does not hit the problem, namely fork()ing after \nrename()ing a mmap()ed file.\n\nNow, with the diff changes a few months ago, there are way less fork()s in \nthe code, that might be one reason you do not hit the wall, but I could \nimagine that a \"git-read-tree -m -u\" still has problems. Note: I did not \ntest what I just said, but I _did_ test that there are problems with \nmmap() when you fork() with the map in use.\n\nCiao,\nDscho\n"},{"id":"23904","messageId":"17595.51940.887000.442225@lapjr.intranet.kiel.bmiag.de","threadId":"4780","inReplyTo":"Pine.LNX.4.63.0607171907080.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2006-07-17T17:37:40Z","receivedAt":"2006-07-17T17:37:40Z","isPatch":false,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"Johannes Schindelin writes:\n > In my not-so-simple tests, it failed. Reproducibly. If I am not mistaken, \n > your test just does not hit the problem, namely fork()ing after \n > rename()ing a mmap()ed file.\n\nNo doubt about that.\n\nBut until you post additional test cases, I can continue to claim my\nversion passes all tests:-)\n\n jr\n"},{"id":"23905","messageId":"Pine.LNX.4.64.0607171055230.15611@evo.osdl.org","threadId":"4780","inReplyTo":"Pine.LNX.4.63.0607171804030.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-17T17:56:04Z","receivedAt":"2006-07-17T17:56:04Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOn Mon, 17 Jul 2006, Johannes Schindelin wrote:\n> \n> So we could introduce a second mmap() which is normally the same as \n> mmap(), except for windows, where it is a rename() and unlink() safe \n> version of mmap() by reading the thing into RAM. Not very pretty.\n\nWell, not too ugly either.\n\nImagine having a \"map_packfile()\" interface, and letting different targets \njust implement their own version. That doesn't sound too bad, does it?\n\n\t\tLinus\n"},{"id":"23906","messageId":"20060717181528.GA30212@admingilde.org","threadId":"4780","inReplyTo":"Pine.LNX.4.64.0607171055230.15611@evo.osdl.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Martin Waitz","fromEmail":"tali@admingilde.org","sentAt":"2006-07-17T18:15:28Z","receivedAt":"2006-07-17T18:15:28Z","isPatch":false,"sender":{"key":"tali@admingilde.org","avatar":"https://gravatar.com/avatar/3f89b03eee362187effabe257898735b475673a12265c398ea9161259ae91553?d=mp&s=160"},"body":"hoi :)\n\nOn Mon, Jul 17, 2006 at 10:56:04AM -0700, Linus Torvalds wrote:\n> Imagine having a \"map_packfile()\" interface, and letting different targets \n> just implement their own version. That doesn't sound too bad, does it?\n\nwe'd need something like that anyway if we want to support larger\npack files.\nWhen it is only possible to map a part of the file into memory at a\ngiven time then we have to special case all accesses to the pack file,\ntoo. Or is there an easier way?\n\n-- \nMartin Waitz\n"},{"id":"23909","messageId":"1F90D448-5347-4CEB-80DE-3CC86C1CC16F@adacore.com","threadId":"4780","inReplyTo":"Pine.LNX.4.63.0607171804030.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Geert Bosch","fromEmail":"bosch@adacore.com","sentAt":"2006-07-17T19:37:06Z","receivedAt":"2006-07-17T19:37:06Z","isPatch":false,"sender":{"key":"bosch@adacore.com","avatar":null},"body":"\nOn Jul 17, 2006, at 12:05, Johannes Schindelin wrote:\n>> Not for pack-files, though. You may have a half-gigabyte pack- \n>> file, and\n>> only use a small small fraction of it.\n>\n> Right.\n>\n>> (You also never really rename them, so windows should be fine for \n>> that\n>> case)\n>\n> So we could introduce a second mmap() which is normally the same as\n> mmap(), except for windows, where it is a rename() and unlink() safe\n> version of mmap() by reading the thing into RAM. Not very pretty.\n\nOr we can avoid doing an mmap of the entire pack file, and instead\ntry to be somewhat smart on limiting the size of the mmap's.\nThis might be sufficient to help Windows and also solve the\nissue of finding contiguous address space for large packs on\n32-bit systems.\n\n -Geert\n"},{"id":"23910","messageId":"20060717202046.GA8119@steel.home","threadId":"4780","inReplyTo":"Pine.LNX.4.64.0607171055230.15611@evo.osdl.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Alex Riesen","fromEmail":"fork0@t-online.de","sentAt":"2006-07-17T20:20:46Z","receivedAt":"2006-07-17T20:20:46Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Linus Torvalds, Mon, Jul 17, 2006 19:56:04 +0200:\n> > So we could introduce a second mmap() which is normally the same as \n> > mmap(), except for windows, where it is a rename() and unlink() safe \n> > version of mmap() by reading the thing into RAM. Not very pretty.\n> \n> Well, not too ugly either.\n> \n> Imagine having a \"map_packfile()\" interface, and letting different targets \n> just implement their own version. That doesn't sound too bad, does it?\n> \n\nMaybe the patch below will at least help to identify the place where\nthe interface could be used. It's a bit more than packfiles.\nI use it since march (I really meant to post it, but probably forgot).\nBut careful, it has an ugly piece in git-compat-util.h, which probably\nmakes the patch only useful on cygwin and nowhere else (is there any\nother platform which has NO_MMAP defined?).\n\n---\n Makefile | 2 +-\n compat/realmmap.c | 26 ++++++++++++++++++++++++++\n diff.c | 4 ++--\n git-compat-util.h | 10 ++++++++--\n sha1_file.c | 24 ++++++++++++------------\n 5 files changed, 49 insertions(+), 17 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 01fb9cf..f16b466 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -432,7 +432,7 @@ ifdef NO_SETENV\n endif\n ifdef NO_MMAP\n \tCOMPAT_CFLAGS += -DNO_MMAP\n-\tCOMPAT_OBJS += compat/mmap.o\n+\tCOMPAT_OBJS += compat/mmap.o compat/realmmap.o\n endif\n ifdef NO_IPV6\n \tALL_CFLAGS += -DNO_IPV6\ndiff --git a/compat/realmmap.c b/compat/realmmap.c\nnew file mode 100644\nindex 0000000..8f26641\n--- /dev/null\n+++ b/compat/realmmap.c\n@@ -0,0 +1,26 @@\n+#include \n+#include \n+#include \n+#include \n+#include \n+#include \"../git-compat-util.h\"\n+\n+#undef mmap\n+#undef munmap\n+\n+void *realmmap(void *start, size_t length, int prot , int flags, int fd, off_t offset)\n+{\n+\tif (start != NULL || !(flags & MAP_PRIVATE)) {\n+\t\terrno = ENOTSUP;\n+\t\treturn MAP_FAILED;\n+\t}\n+\tstart = mmap(start, length, prot, flags, fd, offset);\n+\treturn start;\n+}\n+\n+int realmunmap(void *start, size_t length)\n+{\n+\treturn munmap(start, length);\n+}\n+\n+\ndiff --git a/diff.c b/diff.c\nindex 8b44756..831bf82 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1094,7 +1094,7 @@ int diff_populate_filespec(struct diff_f\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n-\t\ts->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\ts->data = realmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n \t\tclose(fd);\n \t\tif (s->data == MAP_FAILED)\n \t\t\tgoto err_empty;\n@@ -1126,7 +1126,7 @@ void diff_free_filespec_data(struct diff\n \tif (s->should_free)\n \t\tfree(s->data);\n \telse if (s->should_munmap)\n-\t\tmunmap(s->data, s->size);\n+\t\trealmunmap(s->data, s->size);\n \ts->should_free = s->should_munmap = 0;\n \ts->data = NULL;\n \tfree(s->cnt_data);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 93f5580..503b8e4 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -46,22 +46,28 @@ extern void set_error_routine(void (*rou\n \n #ifdef NO_MMAP\n \n-#ifndef PROT_READ\n+#include \n+/*#ifndef PROT_READ\n #define PROT_READ 1\n #define PROT_WRITE 2\n #define MAP_PRIVATE 1\n #define MAP_FAILED ((void*)-1)\n-#endif\n+#endif*/\n \n #define mmap gitfakemmap\n #define munmap gitfakemunmap\n extern void *gitfakemmap(void *start, size_t length, int prot , int flags, int fd, off_t offset);\n extern int gitfakemunmap(void *start, size_t length);\n \n+extern void *realmmap(void *start, size_t length, int prot , int flags, int fd, off_t offset);\n+extern int realmunmap(void *start, size_t length);\n+\n #else /* NO_MMAP */\n \n #include \n \n+#define realmmap mmap\n+#define realmunmap munmap\n #endif /* NO_MMAP */\n \n #ifdef NO_SETENV\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e666aec..38ab710 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -331,14 +331,14 @@ static void read_info_alternates(const c\n \t\tclose(fd);\n \t\treturn;\n \t}\n-\tmap = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\tmap = realmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n \tif (map == MAP_FAILED)\n \t\treturn;\n \n \tlink_alt_odb_entries(map, map + st.st_size, '\\n', relative_base, depth);\n \n-\tmunmap(map, st.st_size);\n+\trealmunmap(map, st.st_size);\n }\n \n void prepare_alt_odb(void)\n@@ -394,7 +394,7 @@ static int check_packed_git_idx(const ch\n \t\treturn -1;\n \t}\n \tidx_size = st.st_size;\n-\tidx_map = mmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\tidx_map = realmmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n \tif (idx_map == MAP_FAILED)\n \t\treturn -1;\n@@ -439,7 +439,7 @@ static int unuse_one_packed_git(void)\n \t}\n \tif (!lru)\n \t\treturn 0;\n-\tmunmap(lru->pack_base, lru->pack_size);\n+\trealmunmap(lru->pack_base, lru->pack_size);\n \tlru->pack_base = NULL;\n \treturn 1;\n }\n@@ -476,7 +476,7 @@ int use_packed_git(struct packed_git *p)\n \t\t}\n \t\tif (st.st_size != p->pack_size)\n \t\t\tdie(\"packfile %s size mismatch.\", p->pack_name);\n-\t\tmap = mmap(NULL, p->pack_size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\tmap = realmmap(NULL, p->pack_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \t\tclose(fd);\n \t\tif (map == MAP_FAILED)\n \t\t\tdie(\"packfile %s cannot be mapped.\", p->pack_name);\n@@ -511,7 +511,7 @@ struct packed_git *add_packed_git(char *\n \t/* do we have a corresponding .pack file? */\n \tstrcpy(path + path_len - 4, \".pack\");\n \tif (stat(path, &st) || !S_ISREG(st.st_mode)) {\n-\t\tmunmap(idx_map, idx_size);\n+\t\trealmunmap(idx_map, idx_size);\n \t\treturn NULL;\n \t}\n \t/* ok, it looks sane as far as we can check without\n@@ -676,7 +676,7 @@ static void *map_sha1_file_internal(cons\n \t\t */\n \t\tsha1_file_open_flag = 0;\n \t}\n-\tmap = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\tmap = realmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n \tif (map == MAP_FAILED)\n \t\treturn NULL;\n@@ -1220,7 +1220,7 @@ int sha1_object_info(const unsigned char\n \t\t\t*sizep = size;\n \t}\n \tinflateEnd(&stream);\n-\tmunmap(map, mapsize);\n+\trealmunmap(map, mapsize);\n \treturn status;\n }\n \n@@ -1246,7 +1246,7 @@ void * read_sha1_file(const unsigned cha\n \tmap = map_sha1_file_internal(sha1, &mapsize);\n \tif (map) {\n \t\tbuf = unpack_sha1_file(map, mapsize, type, size);\n-\t\tmunmap(map, mapsize);\n+\t\trealmunmap(map, mapsize);\n \t\treturn buf;\n \t}\n \treprepare_packed_git();\n@@ -1543,7 +1543,7 @@ int write_sha1_to_fd(int fd, const unsig\n \n \tif (buf) {\n \t\tretval = write_buffer(fd, buf, objsize);\n-\t\tmunmap(buf, objsize);\n+\t\trealmunmap(buf, objsize);\n \t\treturn retval;\n \t}\n \n@@ -1726,7 +1726,7 @@ int index_fd(unsigned char *sha1, int fd\n \n \tbuf = \"\";\n \tif (size)\n-\t\tbuf = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\tbuf = realmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tclose(fd);\n \tif (buf == MAP_FAILED)\n \t\treturn -1;\n@@ -1740,7 +1740,7 @@ int index_fd(unsigned char *sha1, int fd\n \t\tret = 0;\n \t}\n \tif (size)\n-\t\tmunmap(buf, size);\n+\t\trealmunmap(buf, size);\n \treturn ret;\n }\n \n-- \n1.4.1.gb944\n"},{"id":"23911","messageId":"f36b08ee0607171332k1da1ef77j352b31c78039d06c@mail.gmail.com","threadId":"4780","inReplyTo":"17595.48003.145000.414361@lapjr.intranet.kiel.bmiag.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Yakov Lerner","fromEmail":"iler.ml@gmail.com","sentAt":"2006-07-17T20:32:42Z","receivedAt":"2006-07-17T20:32:42Z","isPatch":false,"sender":{"key":"iler.ml@gmail.com","avatar":null},"body":"On 7/17/06, Juergen Ruehle wrote:\n> Johannes Schindelin writes:\n> > > > It is not Cygwin really. It's windows. You can't rename or delete an\n> > > > open or mmapped file in that thing.\n> > >\n> > > And GIT's workaround is to read the whole file into memory and close\n> > > it after that? Uh-oh.\n> >\n> > If you have a better idea (which does not make git source code ugly), go\n> > ahead, write a patch.\n>\n> On several boxes I've tested the mmap code passes the tests on NTFS.\n\nOn me, it failed me on git-apply with more than 1 patches on\nthe commandline.\n\ngit-apply with 1 patch on the commandline passed, with two, failed.\n\ngit-apply with two patches on commandline is the simplest\ntestcase that exposes this problem, AFAIK.\n\nYakov\n"},{"id":"23912","messageId":"Pine.LNX.4.64.0607171431010.2478@evo.osdl.org","threadId":"4780","inReplyTo":"1F90D448-5347-4CEB-80DE-3CC86C1CC16F@adacore.com","subject":"Re: comparing file contents in is_exact_match?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-17T21:31:44Z","receivedAt":"2006-07-17T21:31:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nOn Mon, 17 Jul 2006, Geert Bosch wrote:\n> \n> Or we can avoid doing an mmap of the entire pack file, and instead\n> try to be somewhat smart on limiting the size of the mmap's.\n> This might be sufficient to help Windows and also solve the\n> issue of finding contiguous address space for large packs on\n> 32-bit systems.\n\nWell, the thing is, you really _do_ want to mmap as much as possible of \nthe pack-file as possible, if mmap() works.\n\nSo even with large pack-files, you do want to mmap a huge chunk at a time, \neven if it turns out that you only need a very small part of it.\n\nFor example, the commit data is at the very beginning of the pack-file, so \nif you only look at the history, you only look at a very small part of the \npack-file, but you should not have to know how much you'll need ahead of \ntime, so you'd still want the pack-file operations to act in a way that is \nefficient for the general case (which is to mmap as much as possible).\n\nSo yes, we'll need to have some chunking layer at some point (when people \nhave gigabyte pack-files) but I think that's a totally separate issue from \nthe fact that we _do_ actually want mmap() (the _real_ kind of mmap) for \npack-files.\n\n\t\t\tLinus\n"},{"id":"23914","messageId":"17596.4771.377000.843941@lapjr.intranet.kiel.bmiag.de","threadId":"4780","inReplyTo":"f36b08ee0607171332k1da1ef77j352b31c78039d06c@mail.gmail.com","subject":"Re: comparing file contents in is_exact_match?","fromName":"Juergen Ruehle","fromEmail":"j.ruehle@bmiag.de","sentAt":"2006-07-17T22:43:47Z","receivedAt":"2006-07-17T22:43:47Z","isPatch":false,"sender":{"key":"j.ruehle@bmiag.de","avatar":null},"body":"Yakov Lerner writes:\n > On me, it failed me on git-apply with more than 1 patches on\n > the commandline.\n > \n > git-apply with 1 patch on the commandline passed, with two, failed.\n > \n > git-apply with two patches on commandline is the simplest\n > testcase that exposes this problem, AFAIK.\n\nIn that case tests 4109 and 4110 should fail, shouldn't they? They\nsucced for me on NTFS (and fail on other FS). But anyway, I did some\nquick performance check and the NO_MMAP code path seems to be as fast\nas the mmap one (even much faster in some cases). So the combination\nof windows' memory management and git mmap usage doesn't seem so hot\n(especially considering the fact that git runs about twice as fast on\nFAT32 for me).\n\n jr\n"},{"id":"23918","messageId":"f36b08ee0607180215n7a68d8dfl935ff00194d23484@mail.gmail.com","threadId":"4780","inReplyTo":"17596.4771.377000.843941@lapjr.intranet.kiel.bmiag.de","subject":"Re: comparing file contents in is_exact_match?","fromName":"Yakov Lerner","fromEmail":"iler.ml@gmail.com","sentAt":"2006-07-18T09:15:15Z","receivedAt":"2006-07-18T09:15:15Z","isPatch":false,"sender":{"key":"iler.ml@gmail.com","avatar":null},"body":"On 7/17/06, Juergen Ruehle wrote:\n> Yakov Lerner writes:\n> > On me, it failed me on git-apply with more than 1 patches on\n> > the commandline.\n> >\n> > git-apply with 1 patch on the commandline passed, with two, failed.\n> >\n> > git-apply with two patches on commandline is the simplest\n> > testcase that exposes this problem, AFAIK.\n>\n> In that case tests 4109 and 4110 should fail, shouldn't they? They\n> succed for me on NTFS (and fail on other FS).\n\nMine FAT32 FS.\n\nYakov\n\n> But anyway, I did some\n> quick performance check and the NO_MMAP code path seems to be as fast\n> as the mmap one (even much faster in some cases). So the combination\n> of windows' memory management and git mmap usage doesn't seem so hot\n> (especially considering the fact that git runs about twice as fast on\n> FAT32 for me).\n\nYakov\n"},{"id":"23919","messageId":"f36b08ee0607180238i34cde4deib17426f121ae269e@mail.gmail.com","threadId":"4780","inReplyTo":"Pine.LNX.4.64.0607171431010.2478@evo.osdl.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Yakov Lerner","fromEmail":"iler.ml@gmail.com","sentAt":"2006-07-18T09:38:10Z","receivedAt":"2006-07-18T09:38:10Z","isPatch":false,"sender":{"key":"iler.ml@gmail.com","avatar":null},"body":"Linus Torvalds wrote:\n> Well, the thing is, you really _do_ want to mmap as much as possible of\n> the pack-file as possible, if mmap() works.\n\nJuergen Ruehle wrote:\n> I did some\n> quick performance check and the NO_MMAP code path seems to be as fast\n> as the mmap one (even much faster in some cases). So the combination\n> of windows' memory management and git mmap usage doesn't seem so hot\n\nHow about making this parameter (do-use-mmap vs not-use-mmap)\na *runtime* parameter ? (Env. var. $GIT_MMAP or $GIT_USE_MMAP ?).\nWhat do you think ? I see two benefits:\n(1) much easier to benchmark two methods against each other\n(2) will always work on cygwin (automatic fallback to working\n method at runtime; say depending on filesystem)\n\nWhat do you say ? (Only in case when MMAP is enabled at build-time, of course)\n\nYakov\n"},{"id":"23920","messageId":"Pine.LNX.4.63.0607181216290.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4780","inReplyTo":"f36b08ee0607180238i34cde4deib17426f121ae269e@mail.gmail.com","subject":"Re: comparing file contents in is_exact_match?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-07-18T10:20:40Z","receivedAt":"2006-07-18T10:20:40Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 18 Jul 2006, Yakov Lerner wrote:\n\n> How about making this parameter (do-use-mmap vs not-use-mmap)\n> a *runtime* parameter ? (Env. var. $GIT_MMAP or $GIT_USE_MMAP ?).\n> What do you think ? I see two benefits:\n> (1) much easier to benchmark two methods against each other\n\nYou will benchmark it only once, right? No need to have this in git for \none-time use.\n\n> (2) will always work on cygwin (automatic fallback to working\n> method at runtime; say depending on filesystem)\n\nI think it is too complicated to depend on the filesystem, and too system \nspecific (but hey, those who submit a patch are more right than \nothers...).\n\nBut an automatic fallback is not feasible: you would try to mmap(), and it \nwould throw an \"Access violation\", which is MS speak for \"Segmentation \nfault\".\n\nBesides, if you would be able to detect a failed mmap(), it would be too \nlate: the rename() or unlink() would already have taken place.\n\nCiao,\nDscho\n"},{"id":"23922","messageId":"Pine.LNX.4.64.0607180837260.3386@evo.osdl.org","threadId":"4780","inReplyTo":"f36b08ee0607180238i34cde4deib17426f121ae269e@mail.gmail.com","subject":"Re: comparing file contents in is_exact_match?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-18T15:37:58Z","receivedAt":"2006-07-18T15:37:58Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 18 Jul 2006, Yakov Lerner wrote:\n> \n> How about making this parameter (do-use-mmap vs not-use-mmap)\n> a *runtime* parameter ?\n\nThe thing is, there are really two different kinds of users, and one of \nthem in particular (the pack-file case) shouldn't have the problems under \nWindows at all (because the pack-files aren't moved), and have a much \nbigger reason to use mmap in the first place (because accesses will be \nsparse, so reading the whole file is wasteful).\n\nThe other user was the index file, and _that_ one is the one that gets \nrenamed (to replace the previous index file), and that one is also not \never read only partially, so using \"read()\" instead of mmap is much less \nof an issue.\n\nSo I really think we should just basically always mmap the pack-files (it \nshould work everywhere), and make NO_MMAP just trigger on the other cases.\n\n\t\tLinus\n"},{"id":"23925","messageId":"81b0412b0607181149k74165724pa0a513d14704ce73@mail.gmail.com","threadId":"4780","inReplyTo":"Pine.LNX.4.64.0607180837260.3386@evo.osdl.org","subject":"Re: comparing file contents in is_exact_match?","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-07-18T18:49:48Z","receivedAt":"2006-07-18T18:49:48Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 7/18/06, Linus Torvalds wrote:\n> So I really think we should just basically always mmap the pack-files (it\n> should work everywhere), and make NO_MMAP just trigger on the other cases.\n\nAnd while we are at it make a mental note about avoiding platforms with\nbroken mmap. For instance, QNX6 mmap with MAP_PRIVATE _always_\nmakes a copy of the whole! file, the stupid thing (they even documented it so!).\nThe alternative flag, MAP_SHARED, does not work on some filesystems at all.\n"}]}