Hi Michael, On Tue, 19 Sep 2017, Michael Haggerty wrote: > This is v2 of a patch series that changes the reading and caching of the > `packed-refs` file to use `mmap()`. Thanks to Junio, Stefan, and > Johannes for their comments about v1 [1]. Thank you for the new iteration. > The main change since v1 is to accommodate Windows, which doesn't let > you replace a file using `rename()` if the file is currently mmapped. > This is unfortunate, because it means that Windows will never get the > O(N) → O(lg N) improvement for reading single references that more > capable systems can now enjoy. Triggered by your enquiry, I looked into passing the FILE_SHARE_DELETE flag which I hoped would let us delete the file even if it still is open (and mapped). In my tests, this did not work. If anybody wants to have a look at what I did (and whether they can make it work): https://github.com/dscho/git/tree/replace-wopen > The background was discussed on the mailing list [2]. The bottom line > is that on Windows, keeping the `packed-refs` lock mmapped would be > tantamount to holding reader lock on that file, preventing anybody > (even unrelated processes) from changing the `packed-refs` file while > it is mmapped. This is even worse than the situation for packfiles > (which is solved using `close_all_packs()`), because a packfile, once > created, never needs to be replaced—every packfile has a filename that > is determined from its contents. The worst that can happen if a > packfile is locked is that another process cannot remove it, but that > is not critical for correctness. The `packed-refs` file, on the other > hand, always has the same filename and needs to be overwritten for > correctness. > > So the approach taken here is that a new compile-time option, > `MMAP_PREVENTS_DELETE`, is introduced. When this option is set, then > the `packed-refs` file is read quickly into memory then closed. Another approach would be to imitate close_all_packs() and rely on the Windows-specific code that retries renames in a staggered fashion, waiting a little longer and longer before retrying, and finally telling the user that some file cannot be overwritten: https://github.com/git-for-windows/git/blob/v2.14.1.windows.1/compat/mingw.c#L2439-L2441 This is not a new problem, by the way. If a file is in use while you try to run `git checkout` with a different version of that file, we have the exact same problem on Windows. And we deal with it using that retry_ask_yes_no() function. For this to work, the current process really would need to be able to release all snapshots in one go (for simplicity, I would not even check the filename but simply blow them all away when we want to overwrite packed-refs). I guess I should set aside some time to implement that on top of your series (I *really* want our in-house users to benefit from that O(lg n) improvement). In the meantime, I think this can go forward with the current design. Ciao, Dscho
