On 1/8/2021 1:17 PM, Taylor Blau wrote: > 'estimate_repack_memory()' takes into account the amount of memory > required to load the reverse index in memory by multiplying the assumed > number of objects by the size of the 'revindex_entry' struct. > > Prepare for hiding the definition of 'struct revindex_entry' by removing > a 'sizeof()' of that type from outside of pack-revindex.c. Instead, > guess that one off_t and one uint32_t are required per object. Strictly > speaking, this is a worse guess than asking for 'sizeof(struct > revindex_entry)' directly, since the true size of this struct is 16 > bytes with padding on the end of the struct in order to align the offset > field. This is so far the only not-completely-obvious change. > But, this is an approximation anyway, and it does remove a use of the > 'struct revindex_entry' from outside of pack-revindex internals. And this might be enough justification for it, but... > - heap += sizeof(struct revindex_entry) * nr_objects; > + heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects; ...outside of the estimation change, will this need another change when the rev-index is mmap'd? Should this instead be an API call, such as estimate_rev_index_memory(nr_objects)? That would centralize the estimate to be next to the code that currently interacts with 'struct revindex_entry' and will later interact with the mmap region. Thanks, -Stolee