Re: [PATCH v2 14/21] read_packed_refs(): ensure that references are ordered when read

[Jeff King <peff@peff.net>]

On Tue, Sep 19, 2017 at 08:22:22AM +0200, Michael Haggerty wrote:

> If `packed-refs` was already sorted, then (if the system allows it) we
> can use the mmapped file contents directly. But if the system doesn't
> allow a file that is currently mmapped to be replaced using
> `rename()`, then it would be bad for us to keep the file mmapped for
> any longer than necessary. So, on such systems, always make a copy of
> the file contents, either as part of the sorting process, or
> afterwards.

So this sort-of answers my question from the previous commit (why we
care about the distinction between NONE and TEMPORARY), since we now
start treating them differently.

But I'm a bit confused why there's any advantage in the TEMPORARY case
to doing the mmap-and-copy versus just treating it like NONE and reading
it directly.

Hmm, I guess it comes down to the double-allocation thing again? Let me
see if I have this right:

  1. For NO_MMAP, we'd read the buffer once. If it's sorted, good. If
     not, then we temporarily hold it in memory twice while we copy it
     into the new sorted buffer.

  2. For TEMPORARY, we mmap once. If it's sorted, then we make a single
     copy. If it's not sorted, then we do the copy+sort as a single
     step.

  3. For MMAP_OK, if it's sorted, we're done. Otherwise, we do the
     single copy.

So this is really there to help the TEMPORARY case reading an old
unsorted file avoid needing to use double-the-ram during the copy?

That seems like a good reason (and it does not seem to add too much
complexity).

-Peff