Duy Nguyen [off-list ref] writes:
quoted
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.
Without such a code churn, there won't be extra overhead or need to
fix it up later, no?
On Mon, May 7, 2018 at 5:37 PM, Junio C Hamano [off-list ref] wrote:
Duy Nguyen [off-list ref] writes:
quoted
quoted
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.