Re: [PATCH v2] alloc.h|c: migrate alloc_states to mem-pool

From: René Scharfe. <l.s.r@web.de>
Date: Feb 2, 2021, 16:36 UTC
Message-ID: <f10273ef-3d71-5220-9985-1a4fa2c84cd3@web.de>
In-Reply-To: <CAOLTT8Rf0vjB1+RuChbVPgf=YDif4B1mnro2MEF6E8+uGXM24Q@mail.gmail.com>

Am 02.02.21 um 14:12 schrieb 胡哲宁:

Show 19 quoted lines

> To René Scharfe:
>>> -     struct alloc_state *blob_state;
>>> -     struct alloc_state *tree_state;
>>> -     struct alloc_state *commit_state;
>>> -     struct alloc_state *tag_state;
>>> -     struct alloc_state *object_state;
>>> +     struct mem_pool *blob_pool;
>>> +     struct mem_pool *tree_pool;
>>> +     struct mem_pool *commit_pool;
>>> +     struct mem_pool *tag_pool;
>>> +     struct mem_pool *object_pool;
>>
>> Why have pointers here instead of the structs themselves?  It's not like
>> a struct parsed_object_pool is of much use without them, right?
>>
>> The same question applies to the original code as well, of course.
> Here I may have some questions: why use `struct mem_pool` instead of
> using `struct mem_pool *`?
> I hope you can answer my doubts, thank you!

If struct parsed_object_pool contains pointers to five instances of struct alloc_state or struct mem_pool then you have to allocate and eventually release those instances explicitly. Your patch introduced mem_pool_new() for the allocation part.

If the five instances are embedded in struct parsed_object_pool then you don't need to do that.

The indirection added by allocating explicitly and using pointers would be beneficial if some of five instances were optional, as you could skip their allocation and save some memory -- but you need them all to get a usable struct parsed_object_pool.

René