Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Require an explicit --with-jemalloc to use it #3881

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dridi
Copy link
Member

@dridi dridi commented Dec 8, 2022

This removes the automatic check made only on Linux systems for jemalloc now that operating systems we care about ship a much more recent version than the 3.6 that is known to work really well with Varnish on x86_64 hardware.

Of the platforms we provide packages for, only Ubuntu 18.04 (bionic) and RHEL 7 (via EPEL) ship jemalloc 3.6.0 and the rest already moved on to a 5.x series.

It appears that jemalloc 3.6 on aarch64 results in unstable Varnish workloads while jemalloc 5 is known to generate a lot of waste with its default configuration with a highly threaded workload like Varnish would operate, based on feedback we have seen over years.

From now on, jemalloc is found via pkg-config, only if it is explicitly requested at configure time. With explicit opt-in, the Linux-only check is gone and we solely rely on pkg-config to find the library. This can of course be overriden as usual at configure time:

./configure --with-jemalloc JEMALLOC_CFLAGS=... JEMALLOC_LIBS=...

Or, if you don't like long command lines, via environment variables.

Refs #3867
Refs #3879

This removes the automatic check made only on Linux systems for jemalloc
now that operating systems we care about ship a much more recent version
than the 3.6 that is known to work really well with Varnish on x86_64
hardware.

Of the platforms we provide packages for, only Ubuntu 18.04 (bionic) and
RHEL 7 (via EPEL) ship jemalloc 3.6.0 and the rest already moved on to a
5.x series.

It appears that jemalloc 3.6 on aarch64 results in unstable Varnish
workloads while jemalloc 5 is known to generate a lot of waste with its
default configuration with a highly threaded workload like Varnish would
operate, based on feedback we have seen over years.

From now on, jemalloc is found via pkg-config, only if it is explicitly
requested at configure time. With explicit opt-in, the Linux-only check
is gone and we solely rely on pkg-config to find the library. This can
of course be overriden as usual at configure time:

    ./configure --with-jemalloc JEMALLOC_CFLAGS=... JEMALLOC_LIBS=...

Or, if you don't like long command lines, via environment variables.

Refs varnishcache#3867
Refs varnishcache#3879
Tested on my system, the ${PACKAGE}_CFLAGS variable is the one that
seems to override pkg-config's lookup in autoconf macros. I didn't
check the code, only the behavior.
Looks like el7 is so ancient that the autoconf macro PKG_CHECK_MODULES
does not have a mechanism to override the check. Good thing we don't
bootstrap release archives from el7.
@bsdphk
Copy link
Contributor

bsdphk commented Dec 19, 2022

We had a bit of a pow-wow about this one, and the result was:

Make it easier to configure with/without jemalloc (and/or other malloc implementations ?)

Disable jemalloc on ARM64 for now, as there are credible indications that it causes trouble.

But we do not want to disable jemalloc across the board.

... In particular because we have no contemporary benchmark results on glibc::malloc.

@dridi
Copy link
Member Author

dridi commented Dec 19, 2022

bugwash: ask issue reporters to try some jemalloc flags.

@dridi
Copy link
Member Author

dridi commented Dec 19, 2022

bugwash: we want pkg-varnish-cache to explicitly build with jemalloc support if we merge this change, removing automatic detection.

edit: possibly only for x64_64 packages

@nigoroll
Copy link
Member

bugwash: we want pkg-varnish-cache to explicitly build with jemalloc support if we merge this change, removing automatic detection.

I recall that at least two of us wanted to keep varnish-cache itself (not pkg-*) build with jemalloc by default on amd64, if available.

@jocel1
Copy link

jocel1 commented Jan 18, 2023

FWIW, arm build without jemalloc + x64 build with jemalloc are working perfectly for me (see my comments on #3879)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants