Adding an option to link jemalloc as the malloc implementation #780
Conversation
configure.ac
Outdated
| @@ -114,6 +114,9 @@ AC_ARG_ENABLE(static, | |||
| AC_ARG_ENABLE(unix_socket, | |||
| [AS_HELP_STRING([--disable-unix-socket], [Disable unix domain socket])]) | |||
|
|
|||
| AC_ARG_ENABLE(enable_linking_jemalloc, | |||
| [AS_HELP_STRING([--enable_linking_jemalloc], [Link jemalloc as the memory allocator implementation])]) | |||
There was a problem hiding this comment.
help string should be dashes not underscores. Also maybe just --enable-jemalloc ?
| @@ -217,6 +220,10 @@ if test "x$enable_unix_socket" = "xno"; then | |||
| AC_DEFINE([DISABLE_UNIX_SOCKET],1,[Set to nonzero if you want to disable unix domain socket]) | |||
| fi | |||
|
|
|||
| if test "x$enable_linking_jemalloc" = "xyes"; then | |||
| LIBS="$LIBS -ljemalloc" | |||
There was a problem hiding this comment.
Are there any easy tests we can do here? pkg-config or something?
There was a problem hiding this comment.
Added t/jemalloc.t which uses ldd to check the binary.
There was a problem hiding this comment.
I can't remember what I was thinking at the time, but I bet what I meant was detecting if jemalloc existed on the system before trying to link it. So by "Tests" I meant autoconf tests. That would make more sense, I think? :)
There was a problem hiding this comment.
Got it. dpkg seems to be providing both lib existence and the version.
There was a problem hiding this comment.
Let me try with ldconfig
|
@tharanga pinging again |
|
Test fails here :) |
|
@dormando if everything is good, I will squash this. |
Makefile.am
Outdated
| test: memcached-debug sizes testapp | ||
| $(builddir)/sizes | ||
| $(builddir)/testapp | ||
| if ENABLE_JEMALLOC | ||
| @JEMALLOC_TEST=1 prove $(srcdir)/t/jemalloc.t |
There was a problem hiding this comment.
Is there another runtime/startitme way to detect if jemalloc exists? This causes this one test to run not in parallel. I spend a lot of time running these tests :)
There was a problem hiding this comment.
We need to somehow wire the fact jemalloc is linked from configuration to tests. Though a bit of an overkill, a different make target would work I believe?
There was a problem hiding this comment.
jemalloc/jemalloc#388 - I see a couple decent approaches here. Should be easy to add a supports_jemalloc() test to the ones in t/lib/MemcachedTest.pm
There was a problem hiding this comment.
Looking at the test again.. trying to figure out what the goal of it is. If jemalloc is enabled, it's checking that it's enabled via ldd?
So the supports() test could be looking for something that configure would dump (like a "HAVE_JEMALLOC" in config.h, would be pretty easy to add)
There was a problem hiding this comment.
maybe ignore this comment and see the other one.
removing the linked lib check
No description provided.