Kevin Willford <kewi...@microsoft.com> writes: > When writing the index for each entry an ondisk struct will be > allocated and freed in ce_write_entry. We can do better by > using a ondisk struct on the stack for each entry. > > This is accomplished by using a stack ondisk_cache_entry_extended > outside looping through the entries in do_write_index. Only the > fixed fields of this struct are used when writing and depending on > whether it is extended or not the flags2 field will be written. > The name field is not used and instead the cache_entry name field > is used directly when writing out the name. Because ce_write is > using a buffer and memcpy to fill the buffer before flushing to disk, > we don't have to worry about doing multiple ce_write calls.
The code already heavily assumes that the only difference between ondisk_cache_entry and its _extended variant are identical in their earlier parts, so having a single instance of the larger one on the stack and passing it around makes sense as an optimization. Multiple chomped calls to ce_write() does make me nervous, not due to potential I/O cost (which is none) but to potential programming mistakes for people who touch the code. The version immediately after this patch looks correct in that once "result" is set to non zero to indicate an error nothing is written out, but keeping that property over time, when a newer index format is eventually invented, may become an additional burden. But overall, I think this is a good change. Will queue all three. Thanks.
