[PATCH v3] malloc: send freed small chunks to smallbin

k4lizen k4lizen@proton.me
Fri Nov 1 18:36:01 GMT 2024


Hello,

>> The only possible effect I could think of is the fact that the patch affects
>> the order in which small chunks are (re)allocated. Previously, recently
>> freed chunks had a chance to get realloacted (due to the unsorted) quickly, while
>> with the patch they would always follow the LIFO that smallbin enforces.

Ah wait my bad, I misspoke here. I mixed up what LIFO and FIFO mean. Swap them in
my email to get the intended meaning.

> Both the smallbin and unsorted bin are LIFO.

I guess you probably followed along without noticing my mistake Wilco. Both the 
smallbin and unsorted bin add free chunks to the beginning and take them
from the end i.e. they behave like a queue which is FIFO. This comment also
implies that: https://elixir.bootlin.com/glibc/glibc-2.40.9000/source/malloc/malloc.c#L1714 .

> Only in some cases. Both the smallbin and unsorted bin are LIFO. So freeing
> several blocks of smallbin size and then allocating them will be LIFO both
> before and after.

> The difference would be if a malloc for a different size would start scanning most
> of the unsorted bin and moving chunks to small bins. Then you end up with
> 2x LIFO = FIFO for the smallbin ordering.

I get what you meant to say. The only thing I would note is that unsorted (queue)
 + smallbin (queue) = queue, so the order isn't swapped if chunks get moved.

Either way I just realized all of this isn't really representative of the actual
behaviour due to tcache stashing. Because both the smallbin and the unsorted bin
put chunks in tcache on allocation, the behaviour is messy both before and after
the patch.

>> There's this project for benchmarking malloc implementations:
>> https://github.com/daanx/mimalloc-bench . It's pretty cool and has neat tests
>> like running redis, fraction factorization etc. You can read about them in the
>> "Current benchmarks" section of the README if you want.
> 
> Yes that is a great idea, we should probably run that for every malloc change.

Yeah it would be nice if there was a more standardized/documented way to do this.

Sooo what now? I submitted the patch with the signoff:
https://patchwork.sourceware.org/project/glibc/patch/YYU7Cu7CCrVtJDV7Es3bAL4ZTpH0Uxw2KOmSqKmLyTdrZrd8Tp5Z2vGmCdBZXGLuGwYD1x_hTT-8Buk_QcFZJinKpAPZeYrZiydWacvjj6E=@proton.me/
I don't have commit privs so if you approve could you commit it? Or are we waiting
to see if anyone else has any other comments?

Best regards,
k4lizen



More information about the Libc-alpha mailing list