OVDB-96: CMake Memory Allocators #447

danrbailey · 2019-05-28T18:59:39Z

Add support for Jemalloc and TBB malloc and enable Jemalloc by default.

This seems to build for me on Linux and Windows fine. @jmlait - a quote from your chapter in "Multithreading for Visual Effects":

On platforms where linking to jemalloc is not practical, we resort to tbb::malloc_proxy.

Can you expand on this? Are there any platforms that you'd suggest we make TBB malloc the default? Also curious to know if there is any configuration in which Houdini is shipped without using Jemalloc?

Signed-off-by: Dan Bailey <danbailey@ilm.com>

pcucka · 2019-05-28T22:16:41Z

When building for Houdini or Maya will this prefer the version of jemalloc or tbbmalloc that ships with the app? If not, should it?

Signed-off-by: Dan Bailey <danbailey@ilm.com>

danrbailey · 2019-05-29T06:49:27Z

When building for Houdini or Maya will this prefer the version of jemalloc or tbbmalloc that ships with the app? If not, should it?

Yes, it should. I just added a change which sets the Jemalloc library directory path in OpenVDBHoudiniSetup.cmake so it will now prefer to pick it up from the Houdini install. TBB malloc was already being picked up from the Houdini install if USE_TBBMALLOC was set to on.

I also updated Circle CI to only install jemalloc for non-Houdini builds as well, just to be sure it doesn't fall back to the pre-installed version. I don't know about Maya.

Idclip · 2019-05-29T10:30:00Z

CMakeLists.txt

@@ -152,6 +162,7 @@ mark_as_advanced(
  OPENVDB_ENABLE_RPATH
  USE_HOUDINI
  USE_MAYA
+  USE_JEMALLOC


USE_TBBMALLOC here as well?

And actually, if we decide to set USE_JEMALLOC to ON by default, USE_JEMALLOC probably shouldn't be an advanced option.

As I understand it, mark_as_advanced is just about controlling what shows up when using CMake from the UI? So we should include both jemalloc and tbbmalloc in that?

Yeah that's correct, I'd classify choosing a concurrent allocator as an advanced option, one which we should try to automate or at least choose a good default for (in this case jemalloc). But if jemalloc isn't always available (and can thus cause CMake errors) it probably shouldn't be hidden from the user.

Idclip · 2019-05-29T10:33:52Z

CMakeLists.txt

+Use Jemalloc while building openvdb components. If USE_JEMALLOC is OFF, CMake attempts to query the
+located openvdb build configuration to decide on Jemalloc support. You may set this to off to disable Jemalloc.
+Although not strictly required, it is strongly recommended to use a concurrent memory allocator.
+Note that you must choose between Jemalloc or TBB malloc, or otherwise disable them both.]=] ON)


This is on by default, is that correct? is jemalloc usually available on mac/centos/windows outside of houdini/maya builds?

FWIW, Maya ships with both tbbmalloc and (as of Maya 2017) "awjemalloc", which seems to be jemalloc with renamed symbols. Since we already require TBB, it might be better to make tbbmalloc the default allocator outside of Houdini builds.

I have found jemalloc to provide a decent performance improvement over tbbmalloc, so perhaps we make jemalloc the default outside of non-Maya builds?

@Idclip - do you have a good idea how we would do this in the CMake? Make USE_TBBMALLOC default to ON and USE_JEMALLOC default to OFF when USE_MAYA is ON otherwise OFF and ON respectively?

Does CMake support enum options? jemalloc and tbbmalloc should be mutually exclusive, so what we probably really want is a CONCURRENT_MALLOC option for which the choices are NONE, JEMALLOC, TBBMALLOC, and APPLICATION, where the latter specifies jemalloc for Houdini builds and tbbmalloc for Maya builds.

Does CMake support enum options?

afaik no - you have to use strings i.e.

if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel." FORCE ) endif()

You can have dependent CMake options - we use them a bit:

CMAKE_DEPENDENT_option(USE_JEMALLOC "use jemalloc" ON "USE_MAYA" OFF) CMAKE_DEPENDENT_option(USE_TBBMALLOC "use tbbmalloc" OFF "USE_MAYA" ON)

https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html

For AUTO I would say the logic should be jemalloc for Houdini builds; tbbmalloc for Maya builds; and for all other builds jemalloc if it is available, otherwise tbbmalloc.

I agree that's ideal, but I'm not sure how easy it is to implement the "if it is available" part.

You can remove the REQUIRED keyword from the find_package call so that it doesn't error when searching for jemalloc. Then you can use if(TARGET Jemalloc::jemalloc) ... To check to see if the imported target was created/found

Thanks for the suggestion, I'll give that a go.

Idclip · 2019-05-29T10:36:43Z

This looks great, thanks @danrbailey. Do we have any more specific advice for choosing a mem allocator with vdb aside from "use a concurrent one" - i.e. none vs je vs tbb? Not part of this PR, but it would be great to run some tests and get some numbers out so we can support the recommendation.

danrbailey · 2019-05-29T16:38:07Z

This looks great, thanks @danrbailey. Do we have any more specific advice for choosing a mem allocator with vdb aside from "use a concurrent one" - i.e. none vs je vs tbb? Not part of this PR, but it would be great to run some tests and get some numbers out so we can support the recommendation.

I happen to have some benchmarks I ran recently. I did a very simple test case where I was comparing a vector of around 100 million or so "512 float" buffers to mimic the data storage in OpenVDB and attempt to isolate allocator performance (run across 24 logical cores):

Allocator	Allocate (seconds)	Deallocate (seconds)
glibc malloc (serial)	22.6	24.6
glibc malloc (parallel)	90.0	12.9
tbb malloc (serial)	8.33	31.5
tbb malloc (parallel)	2.73	15.4
je malloc (serial)	2.21	1.64
je malloc (parallel)	1.15	0.37

I haven't looked at any tuning, in particular jemalloc offers options to balance memory usage vs performance.

I think what surprised me most was how much the deallocation time varied. I ran an additional test with jemalloc allocating in serial and then deallocating in parallel (related to OVDB-93) and the deallocation time remained the same as the serial one. I'm still trying to understand what causes this. Right now, we always deserialize .vdb files in serial, so this is an interesting aspect to consider regarding how we do IO.

Idclip · 2019-05-29T17:47:38Z

I happen to have some benchmarks I ran recently. I did a very simple test case where I was comparing a vector of around 100 million or so "512 float" buffers to mimic the data storage in OpenVDB and attempt to isolate allocator performance (run across 24 logical cores):

This is really interesting, thanks! Maybe even worth adding to the documentation, though perhaps we should do some more vdb specific testing first. Either way this definitely seems like enough of a reason to continue to default to jemalloc

Signed-off-by: Dan Bailey <danbailey@ilm.com>

pcucka · 2019-05-29T22:54:03Z

CMakeLists.txt

+    set(CONCURRENT_MALLOC "Jemalloc")
+  endif()
+endif()
+


Can you add a warning here if CONCURRENT_MALLOC is something other than "Auto", "Tbbmalloc", "Jemalloc", or "None", and then maybe set it to either "Auto" or "None"?

I made it fallback on Auto.

pcucka · 2019-05-29T22:54:18Z

CMakeLists.txt

+  set(CONCURRENT_MALLOC Auto CACHE STRING
+    "Choose a concurrent malloc library, options are: None Auto Jemalloc Tbbmalloc.
+    Although not required, it is strongly recommended to use a concurrent memory allocator.
+    Auto is the default and implies Tbbmalloc for Maya and Jemalloc for non-MAYA,


How about "Auto is the default and implies Jemalloc, unless USE_MAYA is ON or Jemalloc is unavailable, in which case Tbbmalloc is used"?

Done, thanks.

jmlait · 2019-05-30T00:25:53Z

Can you expand on this? Are there any platforms that you'd suggest we make TBB malloc the default? Also curious to know if there is any configuration in which Houdini is shipped without using Jemalloc?

JE Malloc is only used on Linux. Windows uses tbb_malloc. It is very hard to inject into the windows runtime to replace the system allocator. TBB actually has to do some very suspicious binary hacking at the machine code level to replace the base allocator. (And it is a patch that windows updates sometimes breaks...) We've not managed to get such a patch for Windows.

We're also still running jemalloc 3.6.0. I've tried several times to move up to 5.0, 5.1, and 5.2. The problem is that the 5.X series uses lots of address space. I've got nothing against this personally, it is a wise way to get faster performance with no significant cost (other than TLB allocation). But users will freak out and submit bugs if the VIRT column of top goes to 128GB, even if it is only using 20GB of actual physical RAM.

Signed-off-by: Dan Bailey <danbailey@ilm.com>

danrbailey · 2019-05-31T02:10:39Z

I think I've addressed all the feedback now, let me know if there's anything else outstanding in this implementation.

Idclip · 2019-05-31T18:16:33Z

I've just tried to run vdb_test on MacOS against Houdini 17.0.506 and immediately get the following error:
vdb_test(65605,0x1177d65c0) malloc: *** malloc_zone_unregister() failed for 0x7fff95729000
The last address doesn't seem to change.

Seems like a related issue here: facebook/rocksdb#1616 jemalloc/jemalloc#420
and the solution they went with: rust-lang/jemalloc#15

danrbailey · 2019-05-31T21:42:51Z

I've just tried to run vdb_test on MacOS against Houdini 17.0.506 and immediately get the following error:
vdb_test(65605,0x1177d65c0) malloc: *** malloc_zone_unregister() failed for 0x7fff95729000
The last address doesn't seem to change.

Seems like a related issue here: facebook/rocksdb#1616 jemalloc/jemalloc#420
and the solution they went with: rust-lang/jemalloc#15

Thanks for highlighting this - I think the simplest solution is to default to using tbbmalloc on Mac, which would leave just Linux on jemalloc by default. I'll give that a go and verify that fixes this issue.

Signed-off-by: Dan Bailey <danbailey@ilm.com>

danrbailey · 2019-05-31T22:56:07Z

I've pushed a change to use TBB malloc on Mac and that seems to be working correctly now.

Idclip · 2019-06-01T10:15:08Z

CMakeLists.txt

@@ -417,6 +432,16 @@ if(USE_MAYA)
  include(OpenVDBMayaSetup)
 endif()

+# Configure malloc library. Use Tbbmalloc for Windows or Maya and Jemalloc otherwise.


Update this comment to include the macos decision with a reference to the problem (as looks like we can actually use jemalloc on mac after a specific version). Also perhaps expand on why windows uses tbbmalloc too for future reference.

Idclip · 2019-06-01T10:15:36Z

openvdb/doc/changes.txt

@@ -17,6 +17,8 @@ Improvements:
  @vdblink::tree::LeafNode::modifyValueAndActiveState()
  LeafNode::modifyValueAndActiveState@endlink now modify voxel values
  in place for improved performance.
+- Add CMake support for linking against Jemalloc and TBB malloc and enable
+  Jemalloc by default.


Should probably add windows/mac notes here

Idclip · 2019-06-01T10:32:48Z

Re-tested on mac, looks good

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Idclip

Looks good! Haven't tested on centos, will do that tomorrow, but worked for me fine on macos

danrbailey added 3 commits May 28, 2019 11:45

Add support for concurrent memory allocators in CMake

346909c

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Add jemalloc to CI

8f536c2

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Update changes

489888f

Signed-off-by: Dan Bailey <danbailey@ilm.com>

danrbailey requested review from jmlait and Idclip May 28, 2019 18:59

For Houdini, use shipped version of Jemalloc

1647bf6

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Idclip reviewed May 29, 2019

View reviewed changes

Switch from USE_*MALLOC to CONCURRENT_MALLOC in CMake

ed808ed

Signed-off-by: Dan Bailey <danbailey@ilm.com>

pcucka suggested changes May 29, 2019

View reviewed changes

Add fallback if CONCURRENT_MALLOC set to unrecognized value

c750819

Signed-off-by: Dan Bailey <danbailey@ilm.com>

danrbailey force-pushed the memoryallocator branch from cf5c4c8 to c750819 Compare May 30, 2019 18:39

danrbailey added 2 commits May 30, 2019 11:42

Use Tbbmalloc for Windows

958f9c7

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Merge branch 'master' into memoryallocator

3a44e08

Disable jemalloc by default on Mac

7909854

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Idclip reviewed Jun 1, 2019

View reviewed changes

Address documentation feedback

60a8459

Signed-off-by: Dan Bailey <danbailey@ilm.com>

Idclip approved these changes Jun 3, 2019

View reviewed changes

Idclip approved these changes Jun 4, 2019

View reviewed changes

pcucka approved these changes Jun 4, 2019

View reviewed changes

danrbailey merged commit 9c0bbd5 into AcademySoftwareFoundation:master Jun 4, 2019

danrbailey deleted the memoryallocator branch June 4, 2019 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OVDB-96: CMake Memory Allocators #447

OVDB-96: CMake Memory Allocators #447

danrbailey commented May 28, 2019

pcucka commented May 28, 2019

danrbailey commented May 29, 2019

Idclip May 29, 2019

Idclip May 29, 2019

danrbailey May 29, 2019

Idclip May 29, 2019

Idclip May 29, 2019

pcucka May 29, 2019

danrbailey May 29, 2019

danrbailey May 29, 2019

pcucka May 29, 2019

Idclip May 29, 2019 •

edited

pcucka May 29, 2019

danrbailey May 29, 2019

Idclip May 29, 2019

danrbailey May 29, 2019

Idclip commented May 29, 2019

danrbailey commented May 29, 2019

Idclip commented May 29, 2019

pcucka May 29, 2019

danrbailey May 31, 2019

pcucka May 29, 2019

danrbailey May 31, 2019

jmlait commented May 30, 2019

danrbailey commented May 31, 2019

Idclip commented May 31, 2019 •

edited

danrbailey commented May 31, 2019

danrbailey commented May 31, 2019

Idclip Jun 1, 2019

danrbailey Jun 3, 2019

Idclip Jun 1, 2019

danrbailey Jun 3, 2019

Idclip commented Jun 1, 2019

Idclip left a comment

OVDB-96: CMake Memory Allocators #447

OVDB-96: CMake Memory Allocators #447

Conversation

danrbailey commented May 28, 2019

pcucka commented May 28, 2019

danrbailey commented May 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Idclip May 29, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Idclip commented May 29, 2019

danrbailey commented May 29, 2019

Idclip commented May 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmlait commented May 30, 2019

danrbailey commented May 31, 2019

Idclip commented May 31, 2019 • edited

danrbailey commented May 31, 2019

danrbailey commented May 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Idclip commented Jun 1, 2019

Idclip left a comment

Choose a reason for hiding this comment

Idclip May 29, 2019 •

edited

Idclip commented May 31, 2019 •

edited