Hi Morten, Thanks for the comments.
> -----Original Message----- > From: Morten Brørup <m...@smartsharesystems.com> > Sent: Thursday, March 5, 2020 5:52 PM > To: Suanming Mou <suanmi...@mellanox.com>; Olivier Matz > <olivier.m...@6wind.com>; Xueming(Steven) Li <xuemi...@mellanox.com> > Cc: Andrew Rybchenko <arybche...@solarflare.com>; dev@dpdk.org; Asaf > Penso <as...@mellanox.com>; Ori Kam <or...@mellanox.com> > Subject: RE: [dpdk-dev] [RFC] mempool: introduce indexed memory pool > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Suanming Mou On > > 12/26/2019 7:05 PM, Olivier Matz wrote: > > > On Thu, Oct 17, 2019 at 06:55:01AM +0000, Xueming Li wrote: > > >> Indexed memory pool manages memory entries by index, allocation > > >> from pool returns both memory pointer and index(ID). users save ID > > >> as u32 or less(u16) instead of traditional 8 bytes pointer. Memory > > >> could be retrieved from pool or returned to pool later by index. > > >> > > >> Pool allocates backend memory in chunk on demand, pool size grows > > >> dynamically. Bitmap is used to track entry usage in chunk, thus > > >> management overhead is one bit per entry. > > >> > > >> Standard rte_malloc demands malloc overhead(64B) and minimal data > > >> size(64B). This pool aims to such cost saving also pointer size. > > >> For scenario like creating millions of rte_flows each consists of > > >> small pieces of memories, the difference is huge. > > >> > > >> Like standard memory pool, this lightweight pool only support fixed > > >> size memory allocation. Pools should be created for each different > > >> size. > > >> > > >> To facilitate memory allocated by index, a set of ILIST_XXX macro > > >> defined to operate entries as regular LIST. > > >> > > >> By setting entry size to zero, pool can be used as ID generator. > > >> > > >> Signed-off-by: Xueming Li <xuemi...@mellanox.com> > > So, you have a use case where 64 bit pointers use too much memory, and you > want to optimize for memory at the cost of performance by using 16, 24 or 32 > bit references instead. A lot of compilers have an option to do this, so this > is > generally a valid optimization from a high level point of view. > > I like the general concept, so I have a few high level comments to the RFC: > > Your API should separate pool creation from element allocation, i.e. define > one > function to create a pool and set the element size of that pool, and define > other > functions to allocate (get) and free (put) elements in a pool. > > Furthermore, your implementation takes a lock when referencing an element. > Dereferencing an element by its index should be optimized for speed, and > should > be lockless. Remember: DPDK is a data plane development kit, not a control > plane development kit. Agree with that, however, this is used for control plane. And there is already a lock need option for user to configure. It seems the v1 RFC misses some code which will free the pool trunk memory once the trunk is totally not used anymore. In this case, lock is required with multiple threads as trunk memory maybe freed. > > Also consider providing functions to allocate/free consecutive arrays of > elements, so they can be dereferenced even faster because only the address of > the first element in the array needs to be looked up through your library. > Alternatively, provide a function for bulk dereferencing. I don't know if > there is a > use case for this... just mentioning it. And if the library's dereferencing > function > is fast enough, this becomes less relevant. Currently, it is not needed. But once implemented as Mempool device driver, it will be supported. > > This library will be used for well defined structures, so the library should > resemble the Mempool library (for fixed size element allocations) more than > the > Malloc library (for variable size allocations). > > You can consider copying the Mempool API, but returning indexes instead of > pointers. > > You should also consider building your implementation on top of the Mempool > library, like the Mbuf library does. This will give you per-lcore caching and > other > benefits already provided by the Mempool library. Step by step, the on top of Mempool implementation will depend on the requirement. > > > Finally, a small detail: The macros for using your indexed mempool elements > in a > linked list should not be part of the library itself. They should be placed > in a > separate library, so similar macros/functions for using indexed mempool > elements in other structures (hashes, queues, etc.) can also be added as > separate libraries at a later time. Good suggestion. > > > Med venlig hilsen / kind regards > - Morten Brørup
