On Mon, May 7, 2018 at 5:37 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Duy Nguyen <pclouds@xxxxxxxxx> writes: > >>> So I think we could just replace it for now and optimize again later, if it >>> turns out to be a problem. I think the easiest optimisation is to increase >>> the allocation size of having a lot more objects per mp_block. >> >> Yeah. I also tested this from a different angle: memory overhead. For >> 2M objects with one mp_block containing 1024 objects (same setting as >> alloc.c), the overhead (not counting malloc() internal overhead) is >> 46KB and we don't have any extra overhead due to padding between >> objects. This is true for all struct blob, commit, tree and tag. This >> is really good. alloc.c has zero overhead when measured this way but >> 46KB is practically zero to me. > > Thanks. > > The above in short sounds like arguing "replacing alloc.c internal > with mempool incurs negligible memory overhead and performance > degradation, but that can be optimized later". It was unclear to me > why such a replacement needs to happen in the first place, though. The replacement with mem-pool might be easier than making sure that alloc.c has no globals and handles allocations per repository correctly. It would make the sb/object-store-alloc series shorter than it currently is, and maybe easier to review the code. However now that sb/object-store-alloc is rerolled with keeping the logic of alloc.c and not replacing it with mem-pool, the only reason would be ease of maintainability by less code On the other hand we have different implementations of lists, vectors and hashmaps, so having different memory allocators doesn't hurt. > Without such a code churn, there won't be extra overhead or need to > fix it up later, no? The original motivation is to get the object store on a per-repository basis. It might be cheaper to use an existing 'objectified' code instead of doing that again.
