Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 65 additions & 53 deletions build/jemalloc.m4
Original file line number Diff line number Diff line change
Expand Up @@ -19,59 +19,71 @@ dnl jemalloc.m4: Trafficserver's jemalloc autoconf macros
dnl

AC_DEFUN([TS_CHECK_JEMALLOC], [
enable_jemalloc=no
AC_ARG_WITH([jemalloc], [AC_HELP_STRING([--with-jemalloc=DIR], [use a specific jemalloc library])],
[
if test "$withval" != "no"; then
if test "x${enable_tcmalloc}" = "xyes"; then
AC_MSG_ERROR([Cannot compile with both jemalloc and tcmalloc])
fi
enable_jemalloc=yes
jemalloc_base_dir="$withval"
case "$withval" in
yes)
jemalloc_base_dir="/usr"
AC_MSG_CHECKING(checking for jemalloc includes standard directories)
;;
*":"*)
jemalloc_include="`echo $withval |sed -e 's/:.*$//'`"
jemalloc_ldflags="`echo $withval |sed -e 's/^.*://'`"
AC_MSG_CHECKING(checking for jemalloc includes in $jemalloc_include libs in $jemalloc_ldflags)
;;
*)
jemalloc_include="$withval/include"
jemalloc_ldflags="$withval/lib"
AC_MSG_CHECKING(checking for jemalloc includes in $withval)
;;
esac
fi
])
has_jemalloc=0
AC_ARG_WITH([jemalloc], [AC_HELP_STRING([--with-jemalloc=DIR], [use a specific jemalloc library])])
AS_IF([test -n "$with_jemalloc" -a "x$with_jemalloc" != "xno" -a "x$enable_tcmalloc" = "xyes"],
[ AC_MSG_ERROR([Cannot compile with both jemalloc and tcmalloc]) ])

jemalloch=0
if test "$enable_jemalloc" != "no"; then
saved_ldflags=$LDFLAGS
saved_cppflags=$CPPFLAGS
jemalloc_have_headers=0
jemalloc_have_libs=0
if test "$jemalloc_base_dir" != "/usr"; then
TS_ADDTO(CPPFLAGS, [-I${jemalloc_include}])
TS_ADDTO(LDFLAGS, [-L${jemalloc_ldflags}])
TS_ADDTO_RPATH(${jemalloc_ldflags})
fi
# On Darwin, jemalloc symbols are prefixed with je_. Search for that first, then fall back
# to unadorned symbols.
AC_SEARCH_LIBS([je_malloc_stats_print], [jemalloc], [jemalloc_have_libs=1],
[AC_SEARCH_LIBS([malloc_stats_print], [jemalloc], [jemalloc_have_libs=1])]
)
if test "$jemalloc_have_libs" != "0"; then
AC_CHECK_HEADERS(jemalloc/jemalloc.h, [jemalloc_have_headers=1])
fi
if test "$jemalloc_have_headers" != "0"; then
jemalloch=1
else
CPPFLAGS=$saved_cppflags
LDFLAGS=$saved_ldflags
AS_IF([test -n "$with_jemalloc" -a "x$with_jemalloc" != "xno" ],[
case "$withval" in
(yes)
PKG_CHECK_MODULES([JEMALLOC], [jemalloc >= 1.0], [have_jemalloc=yes], [:])
jemalloc_libdir="$($PKG_CONFIG jemalloc --variable=libdir)"
;;
(*":"*)
jemalloc_incdir="$(echo $withval |sed -e 's/:.*$//')"
jemalloc_libdir="$(echo $withval |sed -e 's/^.*://')"
;;
(*)
jemalloc_incdir="$withval/include"
jemalloc_libdir="$withval/lib"
;;
esac

test -z "$jemalloc_libdir" -o -d "$jemalloc_libdir" || jemalloc_libdir=
test -z "$jemalloc_incdir" -o -d "$jemalloc_incdir" || jemalloc_incdir=

jemalloc_incdir="${jemalloc_incdir:+$(cd $jemalloc_incdir; pwd -P)}"
jemalloc_libdir="${jemalloc_libdir:+$(cd $jemalloc_libdir; pwd -P)}"

: ${JEMALLOC_CFLAGS:="${jemalloc_incdir:+ -I$jemalloc_incdir}"}
: ${JEMALLOC_LDFLAGS:="${jemalloc_libdir:+ -L$jemalloc_libdir}"}

save_cppflags="$CPPFLAGS"
save_ldflags="$LDFLAGS"
LDFLAGS="$LDFLAGS $JEMALLOC_LDFLAGS"
CPPFLAGS="$CPPFLAGS $JEMALLOC_CFLAGS"

dnl
dnl define HAVE_LIBJEMALLOC is to be set and -ljemalloc added into LIBS?
dnl
AC_CHECK_LIB([jemalloc],[je_malloc_stats_print],[],
[AC_CHECK_LIB([jemalloc],[malloc_stats_print],[],
[have_jemalloc=no])])

dnl
dnl define HAVE_JEMALLOC_JEMALLOC_H is to be set?
dnl define HAVE_JEMALLOC_H is to be set?
dnl
AC_CHECK_HEADERS(jemalloc/jemalloc.h, [],
[AC_CHECK_HEADERS(jemalloc.h, [], [have_jemalloc=no])])

LDFLAGS="$save_ldflags"
CPPFLAGS="$save_cppflags"
LIBS="$(echo "$LIBS" | sed -e 's/ *-ljemalloc//' -e 's/$/ -ljemalloc/')"

AS_IF([test "x$have_jemalloc" == "xno" ],
[AC_MSG_ERROR([Failed to compile with jemalloc.h and -ljemalloc]) ])

has_jemalloc=1
#
# flags must be global setting for all libs
#
TS_ADDTO(CPPFLAGS,$JEMALLOC_CFLAGS)
if test -n "$jemalloc_libdir"; then
TS_ADDTO_RPATH($jemalloc_libdir)
TS_ADDTO(LDFLAGS,$JEMALLOC_LDFLAGS)
fi
fi
AC_SUBST(jemalloch)
])
AC_SUBST([has_jemalloc])
])
8 changes: 7 additions & 1 deletion iocore/eventsystem/I_ProxyAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@
#ifndef _I_ProxyAllocator_h_
#define _I_ProxyAllocator_h_

#include "ts/ink_platform.h"
#include "ts/ink_assert.h"

#include "ts/Allocator.h"

#ifdef ProxyAllocator
#error "cannot compile together with new JEMalloc includes"
#endif

class EThread;

Expand Down
6 changes: 5 additions & 1 deletion iocore/eventsystem/I_Thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,15 @@
#error "include I_EventSystem.h or P_EventSystem.h"
#endif

#include "ts/Allocator.h"
#if !HAVE_LIBJEMALLOC
#include "I_ProxyAllocator.h"
#endif

#include <functional>

#include "ts/ink_platform.h"
#include "ts/ink_thread.h"
#include "I_ProxyAllocator.h"

class ProxyMutex;

Expand Down
10 changes: 8 additions & 2 deletions iocore/eventsystem/ProxyAllocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,17 @@
See the License for the specific language governing permissions and
limitations under the License.
*/
#include "I_EventSystem.h"

int thread_freelist_high_watermark = 512;
int thread_freelist_low_watermark = 32;

// must include for defines
#include "ts/ink_config.h"

#undef HAVE_LIBJEMALLOC

// safe to use older system
#include "I_ProxyAllocator.h"

void *
thread_alloc(Allocator &a, ProxyAllocator &l)
{
Expand Down
63 changes: 63 additions & 0 deletions lib/ts/Allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,39 @@

#define RND16(_x) (((_x) + 15) & ~15)

struct _InkFreeList {
volatile head_p head;
const char *name;
uint32_t type_size, chunk_size, used, allocated, alignment;
uint32_t allocated_base, used_base;
int advice;
};

typedef struct ink_freelist_ops InkFreeListOps;
typedef struct _InkFreeList InkFreeList;

extern "C" {

const InkFreeListOps *ink_freelist_malloc_ops();
const InkFreeListOps *ink_freelist_freelist_ops();
void ink_freelist_init_ops(const InkFreeListOps *);

/*
* alignment must be a power of 2
*/
InkFreeList *ink_freelist_create(const char *name, uint32_t type_size, uint32_t chunk_size, uint32_t alignment);

inkcoreapi void ink_freelist_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32_t chunk_size, uint32_t alignment);
inkcoreapi void ink_freelist_madvise_init(InkFreeList **fl, const char *name, uint32_t type_size, uint32_t chunk_size,
uint32_t alignment, int advice);
inkcoreapi void *ink_freelist_new(InkFreeList *f);
inkcoreapi void ink_freelist_free(InkFreeList *f, void *item);
inkcoreapi void ink_freelist_free_bulk(InkFreeList *f, void *head, void *tail, size_t num_item);
void ink_freelists_dump(FILE *f);
void ink_freelists_dump_baselinerel(FILE *f);
void ink_freelists_snap_baseline();
}

/** Allocator for fixed size memory blocks. */
class Allocator
{
Expand Down Expand Up @@ -252,4 +285,34 @@ template <class C> class TrackerClassAllocator : public ClassAllocator<C>
ink_mutex trackerLock;
};

#if HAVE_LIBJEMALLOC

// these calls can be removed
#define ink_freelists_dump(...)
#define ink_freelists_dump_baselinerel(...)
#define ink_freelists_snap_baseline(...)

#define ink_freelist_init_ops(...)

#include "ts/StdAllocWrapper.h"

// cover up use of original Allocators above
#define Allocator AlignedAllocator
#define ClassAllocator ObjAllocator
#define ProxyAllocator ThreadAllocatorStub

class ThreadAllocatorStub
{
};

// define use of for thread Allocators
#define THREAD_ALLOC(allocObj, t) (::allocObj.alloc())
#define THREAD_ALLOC_INIT(allocObj, t) (::allocObj.alloc())
#define THREAD_FREE(ptr, allocObj, t) (::allocObj.free(ptr))

extern int thread_freelist_high_watermark;
extern int thread_freelist_low_watermark;

#endif

#endif // _Allocator_h_
5 changes: 5 additions & 0 deletions lib/ts/Arena.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@

#include "ts/ink_platform.h"
#include "ts/ink_memory.h"

// need standard Allocator for this
#undef HAVE_LIBJEMALLOC

#include "ts/Allocator.h"
#include "ts/Arena.h"

#include <cassert>
#include <cstring>

Expand Down
22 changes: 21 additions & 1 deletion lib/ts/Arena.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@

#ifndef __ARENA_H__
#define __ARENA_H__
#include "ts/ink_assert.h"
#include "ts/ink_memory.h"

#include <sys/types.h>
#include <memory.h>
#include "ts/ink_assert.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to reorder these for a particular reason?


struct ArenaBlock {
ArenaBlock *next;
Expand Down Expand Up @@ -166,4 +167,23 @@ Arena::str_store(const char *str, size_t len)
return mem;
}

struct Arena_NoCache : public Arena {
std::allocator<uint64_t> m_alloc;

inkcoreapi void *
alloc(size_t size, size_t alignment = sizeof(double))
{
return m_alloc.allocate((size + 1) / sizeof(uint64_t));
}
void
free(void *mem, size_t size)
{
m_alloc.deallocate(static_cast<uint64_t *>(mem), size);
}
};

#if HAVE_LIBJEMALLOC
#define Arena Arena_NoCache
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels odd. You're over-riding the alloc/free functions in a non-virtual class by way of the preprocessor. A person reading the code is unlikely to properly guess which function is actually going to be called. I'd move the #ifdef into each of the alloc and free functions and the body of the class Arena.

As it is, if someone changes the signature of the first alloc (and the places that call it) without changing the jemalloc, not only will it compile, it will mostly work, but with the wrong allocator.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. nope. C++ blocks all signature overloads from a base class that have the same function name.

Lots and lots and lots of different techniques of specialized memory-hoarding doesn't speed things up, but often makes them vulnerable to unchecked and invisible bad-pointer usage.

There's also no way to profile deletions or allocations ... when they're hidden deep inside a memory-hoarder. The thread-cache in Jemalloc is far more efficient.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, fair enough. It's still odd. I'm not seeing how the claim about memory-hoarding is relevant to this observation thought? I'm claiming that a reader is unlikely to determine correctly which code is actually going to execute by reading the function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently I'm trying to make a non-mainline Jemalloc install (which is the point of the commit) work such that we can estimate it's performance. As it is ... the burden of #ifdef is worse than the burden of two overridden calls.

#endif

#endif /* __ARENA_H__ */
8 changes: 6 additions & 2 deletions lib/ts/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ libtsutil_la_LIBADD = \
@OPENSSL_LIBS@ \
@LIBTCL@ \
@LIBRESOLV@ \
@LIBCAP@ \
-lc
@LIBCAP@

libtsutil_la_SOURCES = \
jemallctl.cc \
jemallctl.h \
Allocator.h \
StdAllocWrapper.h \
StdAllocWrapper.cc \
Arena.cc \
Arena.h \
BaseLogFile.cc \
Expand Down Expand Up @@ -255,6 +258,7 @@ test_tslib_SOURCES = \
unit-tests/string_view.cpp

CompileParseRules_SOURCES = CompileParseRules.cc
CompileParseRules_LDADD = ;true # link command doesn't need $(LIBS)

clean-local:
rm -f ParseRulesCType ParseRulesCTypeToLower ParseRulesCTypeToUpper
Expand Down
61 changes: 61 additions & 0 deletions lib/ts/StdAllocWrapper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/** @file

A brief file description

@section license License

Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

#include "StdAllocWrapper.h"
#include "ts/jemallctl.h"

#include "ts/ink_queue.h"
#include "ts/ink_defs.h"
#include "ts/ink_resource.h"
#include "ts/ink_align.h"
#include "ts/ink_memory.h"

void
AlignedAllocator::re_init(const char *name, unsigned int element_size, unsigned int chunk_size, unsigned int alignment, int advice)
{
_name = name;
_sz = aligned_spacing(element_size, std::max(sizeof(uint64_t), alignment + 0UL)); // increase to aligned size

if (advice == MADV_DONTDUMP) {
_arena = jemallctl::proc_arena_nodump;
} else if (advice == MADV_NORMAL) {
_arena = jemallctl::proc_arena;
} else {
ink_abort("allocator re_init: unknown madvise() flags: %x", advice);
}

void *preCached[chunk_size];

for (int n = chunk_size; n--;) {
preCached[n] = mallocx(_sz, (MALLOCX_ALIGN(_sz) | MALLOCX_ARENA(_arena)));
}
for (int n = chunk_size; n--;) {
deallocate(preCached[n]);
}
}

AlignedAllocator::AlignedAllocator(const char *name, unsigned int element_size)
: _name(name), _sz(aligned_spacing(element_size, sizeof(uint64_t)))
{
// don't pre-allocate before main() is called
}
Loading