Re: [PATCH v1 3/5] mem-pool: fill out functionality

[Jonathan Tan <jonathantanmy@google.com>]

On Mon, 23 Apr 2018 13:27:09 -0400
Jameson Miller <jameson.miller81@gmail.com> wrote:

> > This seems overly complicated - the struct mem_pool already has a linked
> > list of pages, so couldn't you create a custom page and insert it behind
> > the current front page instead whenever you needed a large-size page?
> 
> Yes - that is another option. However, the linked list of pages includes 
> memory that *could* have space for an allocation, while the "custom" 
> region will never have left over memory that can be used for other 
> allocations. When searching pages for memory to satisfy a request, there 
> is no reason to search through the "custom" pages. There is a trade-off 
> between complexity and implementation, so I am open to suggestions.
> 
> This was discussed in [1], where it originally was implemented closer to 
> what you describe here.
> 
> > Also, when combining, there could be some wasted space on one of the
> > pages. I'm not sure if that's worth calling out, though.
> 
> Yes, we bring over the whole page. However, these pages are now 
> available for new allocations.
> 
> [1]
> https://public-inbox.org/git/xmqqk1u2k91l.fsf@gitster-ct.c.googlers.com/

Ah, I didn't realize that the plan was to search over all pages when
allocating memory from the pool, instead of only searching the last
page. This seems like a departure from the fast-import.c way, where as
far as I can tell, new_object() searches only one page. If we do plan to
do this, searching all pages doesn't seem like a good idea to me,
especially since the objects we're storing in the pool are of similar
size.

If we decide to go ahead with searching all the pages, though, the
"custom" pages should probably be another linked list instead of an
array.