Re: [PATCH v2 1/8] packfile: prepare for the existence of '*.rev' files
Taylor Blau <me@ttaylorr.com> writes:
Show 21 quoted lines
>> Thinking out loud a bit: a .rev file means we're spending an extra map
>> per pack (but not a descriptor, since we close after mmap). And like the
>> .idx files (but unlike .pack file maps), we don't keep track of these
>> and try to close them when under memory pressure. I think that's
>> probably OK in terms of bytes. It may mean running up against operating
>> system number-of-mmap limits more quickly ...
>> ...
>> > if (ends_with(name, ".idx"))
>> > return 3;
>> > - return 4;
>> > + if (ends_with(name, ".rev"))
>> > + return 4;
>> > + return 5;
>> > }
>>
>> Probably not super important, but: should the .idx file still come last
>> here? Simultaneous readers won't start using the pack until the .idx
>> file is present. We'd probably prefer they see the whole thing
>> atomically, than see a .idx missing its .rev (they won't ever produce a
>> wrong answer, but they'll generate the in-core revindex on the fly when
>> they don't need to).
At some point, we may want to
- introduce .idx version 3 that is more extensible, so that the
reverse info is included in one of its chunks;
- make the .rev data for all packs stored as a chunk in .midx, so
we can first check with .midx and not open any .rev files.
either of which would reduce the numberfrom 30k down to 10k ;-)