This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 779 - Add support for the tcmalloc memory allocator
Summary: Add support for the tcmalloc memory allocator
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Core - general (show other bugs)
Version: 3.2
Hardware: All All
: Normal Unknown
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on: 761
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2014-03-31 20:16 UTC by Benoit Steiner
Modified: 2019-12-04 13:10 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.
Comment 8 Nobody 2019-12-04 13:10:36 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/779.

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