On 04/23/2018 01:49 PM, Jonathan Tan wrote:
On Mon, 23 Apr 2018 13:27:09 -0400
Jameson Miller <jameson.miller81@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx/
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.
I see. However, the new_object() logic in fast-import is a different
than the logic mem_pool was abstracting, and is not covered by the
mem_pool functionality. The behavior of searching over all pages for one
to satisfy the request existed previously and was not changed in the
mem_pool implementation.
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.
This is an option - I went with the current design because we only need
pointers to a block of memory (as well tracking how large the allocation
is for verification purposes). We don't necessarily need the extra
overhead of a structure to track the linked list nodes, when it is
provided by the existing array manipulation functions. I am open to
feedback on this point, however.
