New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 779 - Add support for the tcmalloc memory allocator
Add support for the tcmalloc memory allocator
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Core - general
3.2
All All
: Normal Unknown
Assigned To: Nobody
:
Depends on: 761
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2014-03-31 20:16 UTC by Benoit Steiner
Modified: 2016-02-05 21:14 UTC (History)
2 users (show)



Attachments

Description Benoit Steiner 2014-03-31 20:16:01 UTC
Unlike gnu-malloc, tcmalloc doesn't guaranty alignment when it allocates less than 16 bytes of memory. This isn't a problem though, since it isn't possible to load these small buffers into vector registers.

I have created a patch at http://code.bsteiner.info/eigen-patches/src/1f4090835899bc7a45c55f0013cc46e16ca33366/tcmalloc?at=default. The patch relaxes a sanity check and updates 2 regression tests to prevent them from tripping on matrices of fewer than 16 bytes.
Comment 1 Benoit Jacob 2014-04-01 00:48:08 UTC
I have been thinking for a long time anyway (since I work at Mozilla and realized that people in the "real world" use custom memory allocators) that the whole MALLOC_ALREADY_ALIGNED trick is completely unsafe. I would vote for removing it altogether!
Comment 2 Benoit Steiner 2014-04-01 07:44:18 UTC
Removing MALLOC_ALREADY_ALIGNED would simplify the code and make it easier to maintain it across multiple platforms.
The only downside I can think of is that this might increase memory fragmentation if someone uses a custom memory allocator and allocates many vectors of 2 floats. I don't think this is something we should be overly concerned with as this is pretty unlikely and there are several ways to solve this (e.g. use a large vector of bytes for the actual storage and Eigen::Map the vectors).
If everyone agrees I'll work on another patch next week to remove MALLOC_ALREADY_ALIGNED.
Comment 3 Benoit Jacob 2014-04-01 07:57:23 UTC
I don't see how removing MALLOC_ALREADY_ALIGNED would increase fragmentation? Eigen would simply fall back to using proper aligned-malloc routines such as posix_memalign() instead of malloc() --- how would that affect fragmentation?

I would encourage you to write this patch. I introduced MALLOC_ALREADY_ALIGNED a long time ago, so it's my mistake :-) That said, I'm no longer really a maintainer these days. The person whom you need approval from is Gael.
Comment 4 Gael Guennebaud 2014-04-01 11:29:03 UTC
Finally, I'm all for it too. Maintaining this piece of code has proven to be a headache.
Comment 5 Benoit Jacob 2014-04-01 11:35:26 UTC
It's not just hard to maintain; it's just wrong, because many software projects use custom allocators and there's no way for Eigen to detect that!
Comment 6 Benoit Steiner 2014-04-02 19:33:12 UTC
I have deleted all the MALLOC_ALREADY_ALIGNED related code. The patch is available at http://code.bsteiner.info/eigen-patches/commits/540fc4baee27600eba575b766687de58c84b467c
Comment 7 Gael Guennebaud 2016-02-05 21:14:41 UTC
https://bitbucket.org/eigen/eigen/commits/4aaa98775a66/
Summary:     Bug 779: allow non aligned buffers for buffers smaller than the requested alignment.

And after long discussions on the ML, we'll keep MALLOC_ALREADY_ALIGNED related code.

Note You need to log in before you can comment on or make changes to this bug.