hse
hse copied to clipboard
hse-project
Add tcmalloc support
We had discussed getting rid of alloc_aligned() and free_aligned() since they are pretty much footguns within the repo. You can't use free() on memory that was allocated with alloc_aligned(). Greg originally created these functions because the standard library aligned_alloc() was not as performant as it should have been.
During this discussion, Greg did some preliminary testing on various allocators, and tcmalloc beat all allocators, including the alloc_aligned() one from this repo.
tcmalloc support will be optional, and the build will gracefully degrade should it not exist by default.
Signed-off-by: Tristan Partin [email protected]
Strawman proposal. This would allow us to get Alex's sweet allocator API :). I would like SQA to do a bit of testing between master, -Dtcmalloc=disabled, and -Dtcmalloc=enabled.
Greg were you LD_PRELOADing previously? Is dynamic linking here going to hurt us without preloading going to hurt us by default? Not really sure how allocators work in this regard.
Please review all the changes that used to be alloc_aligned()
I think we should use whatever allocator the app is using, malloc by default, and if the app wants something different they'll specify it and we'll just pick it up organically. You really don't want multiple allocators competing for resources, not even sure if that's possible, but if it is it would be really bad. You also don't want to override the app's choice of allocators, not sure if that's possible.
Now, I don't have an issue with our tools specifying which allocator to use, but libhse should just use malloc by default and let the app override it.
Hmmm, so do you still want to keep alloc_aligned() and free_aligned()? I had a different understanding of our conversation from the other day.
No, I think we should get rid of our alloc_aligned / free_aligned. I just don't think we need to do anything else with the build wrt the malloc library, as such decisions should be entirely up to the app IMO.