[External] Re: [PATCH] malloc: fix malloc performance may becomes worse as the number of malloc increases

[Fengnan Chang]

Liang Ma <liangma at liangbit.com> 于2023年2月16日周四 22:04写道：
>
> On Wed, Feb 15, 2023 at 12:10:23PM +0100, Morten Brørup wrote:
> > +CC: Fidaullah Noonari <fidaullah.noonari at emumba.com>, your name also shows up in the git log; perhaps you can help review this patch.
> >
> >
> > I gave up reviewing in depth, because the existing code is not easy to quickly understand, and it would take too long for me. E.g. the malloc_elem->size is field is undocumented, and find_suitable_element() calls the malloc_elem_free_list_index() function with the raw size (as passed to the function), but malloc_elem_free_list_insert() calls the malloc_elem_free_list_index() with malloc_elem->size - MALLOC_ELEM_HEADER_LEN.
> >
> > Looking isolated at the patch itself...
> >
> > I agree with the way the patch modifies the ranges of the free list, and the consequential removal of the "- 1" from the calculation of log2.
> >
> > Intuitively, the lists should cover ranges such as [0x100..0x3FF], which this patch does, not [0x101..0x400], how it was previously... The ranges with this patch make much more sense.
> >
> > So if the existing code is otherwise correct, i.e. handles the size with/without MALLOC_ELEM_HEADER_LEN correctly, my gut feeling says this patch is an improvement.
> >
> > Acked-by: Morten Brørup <mb at smartsharesystems.com>
> I run the test with DPDK malloc perf test. The issue is replicated.
> IMO, the whole process is if application use rte_malloc with a relative
> large alignment size. e.g. 4K alignment. Currently implementation will
> generate lots "fragment" because of the large alignment in related mem
> element free list. In the test code, 4K malloc size + 4k alignment will
> lead to the actually space is needed has to take from upper level mem
> element idx free list. The consequence is that will generate lots
> fragment element and those element is inserted to the current level mem
> free-list. However, when the rte_malloc select which free list to start
> scan with, it doesn't take the alignment into account, which ends up
> with waste some time in the current level free-list. If the scan logic
> take alignment into account, it might "smartly" skip current level and
> jump to the upper level directly.
>

Thank you for the detailed explanation !
You may have already found the problem that this patch can only solve
the fragmentation problem of 4k/16k/64k scenes, not work for 3k/15k/63k.
For example, alloc 3k and align with 1k, also has this problem.

Now this patch can't handle all similar scenarios，We already started
working on a new soultion which aims to solve all similar problems .

BTW, this patch is still useful, because even without fragmentation
problems, the probability of finding a suitable elem from a larger size list
is greater than from a smaller size list.


> >
>