Update jemalloc to version 5.3.0

[minsoochoo0122_proton.me]

Summary

jemalloc 5.3.0 was released in 2022 but has not been imported into FreeBSD base system yet. The new version of jemalloc includes new features and bug fixes which are listed on release note.

Test Plan

cd lib/libc
make
cd /usr/src/
make buildworld && make installworld

then reboot and test several programs and applications

[minsoochoo0122_proton.me]

When I build new jemalloc, clang generates an error:

jemalloc_prof_sys.c:310:9: error: call to undeclared function 'pthread_getname_np'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
        return pthread_getname_np(pthread_self(), buf, limit);
               ^
jemalloc_prof_sys.c:310:9: note: did you mean '_pthread_getname_np'?
/usr/src/include/pthread.h:305:6: note: '_pthread_getname_np' declared here
int             pthread_getname_np(pthread_t, char *, size_t);
                ^
/usr/src/lib/libc/include/namespace.h:141:30: note: expanded from macro 'pthread_getname_np'
#define         pthread_getname_np              _pthread_getname_np
                                                ^
1 error generated.
*** [jemalloc_prof_sys.o] Error code 1

although <pthread.h> is already included. Any ideas?

[minsoochoo0122_proton.me]

Solved libc build error. However, linking fails with error that _pthread_getname_np is undefined symbol.

Building /usr/obj/usr/src/amd64.amd64/lib/csu/tests/dynamic/init_test.o
Building /usr/obj/usr/src/amd64.amd64/lib/csu/tests/dynamic/init_test.full
ld: error: undefined reference due to --no-allow-shlib-undefined: _pthread_getname_np
>>> referenced by /usr/obj/usr/src/amd64.amd64/tmp/lib/libc.so.7
clang: error: linker command failed with exit code 1 (use -v to see invocation)
*** Error code 1

[lwhsu]

Thanks for the work! One thing I would like to mention is that it seems having some issue building or running on 32-bit older CURRENT: https://github.com/jemalloc/jemalloc/pull/2228

[minsoochoo0122_proton.me]

In D41421#943128, @lwhsu wrote:

Thanks for the work! One thing I would like to mention is that it seems having some issue building or running on 32-bit older CURRENT: https://github.com/jemalloc/jemalloc/pull/2228

I tested again on FreeBSD 13, and the same problem occurs during test. I guess a commit in the base system between 13.0 and 13.1 causes this problem. The CI result in the pull request shows that 32 bit jemalloc worked on FreeBSD 13 on Feburary 2022.

[minsoochoo0122_proton.me]

In D41421#943128, @lwhsu wrote:

Thanks for the work! One thing I would like to mention is that it seems having some issue building or running on 32-bit older CURRENT: https://github.com/jemalloc/jemalloc/pull/2228

I did some test on 32-bit FreeBSD and I found out that when clock_gettime is used on 64bit machine but compiled in 32bit, it sets tv_sec (time_t) as -1. On 32bit FreeBSD, clock_gettime works as expected.

[imp]

In D41421#943562, @minsoochoo0122_proton.me wrote:

In D41421#943128, @lwhsu wrote:

Thanks for the work! One thing I would like to mention is that it seems having some issue building or running on 32-bit older CURRENT: https://github.com/jemalloc/jemalloc/pull/2228

I did some test on 32-bit FreeBSD and I found out that when clock_gettime is used on 64bit machine but compiled in 32bit, it sets tv_sec (time_t) as -1. On 32bit FreeBSD, clock_gettime works as expected.

that seems like it's a bug. On powerpc and amrv7 tv_sec should be the same. On i386, tv_sec is only32-bit, but we still have a dozen years left until it's inadequate. Can you finle a bug, if you haven't already, with a way to reproduce this?

[minsoochoo0122_proton.me]

In D41421#943650, @imp wrote:

In D41421#943562, @minsoochoo0122_proton.me wrote:

In D41421#943128, @lwhsu wrote:

Thanks for the work! One thing I would like to mention is that it seems having some issue building or running on 32-bit older CURRENT: https://github.com/jemalloc/jemalloc/pull/2228

I did some test on 32-bit FreeBSD and I found out that when clock_gettime is used on 64bit machine but compiled in 32bit, it sets tv_sec (time_t) as -1. On 32bit FreeBSD, clock_gettime works as expected.

that seems like it's a bug. On powerpc and amrv7 tv_sec should be the same. On i386, tv_sec is only32-bit, but we still have a dozen years left until it's inadequate. Can you finle a bug, if you haven't already, with a way to reproduce this?

Here is a bug report which includes an explanation and a way to reproduce the problem.

[minsoochoo0122_proton.me]

Updated FREEBSD-diffs

[minsoochoo0122_proton.me]

I still get error

cd /usr/src; _PARALLEL_SUBDIR_OK=1 time env MACHINE_ARCH=amd64  MACHINE=amd64  CPUTYPE= BUILD_TOOLS_META=.NOMETA CC="cc -target x86_64-unknown-freebsd14.0 --sysroot=/usr/obj/usr/src/amd64.amd64/tmp -B/usr/obj/usr/src/amd64.amd64/tmp/usr/bin" CXX="c++  -target x86_64-unknown-freebsd14.0 --sysroot=/usr/obj/usr/src/amd64.amd64/tmp -B/usr/obj/usr/src/amd64.amd64/tmp/usr/bin"  CPP="cpp -target x86_64-unknown-freebsd14.0 --sysroot=/usr/obj/usr/src/amd64.amd64/tmp -B/usr/obj/usr/src/amd64.amd64/tmp/usr/bin"  AS="as" AR="ar" ELFCTL="elfctl" LD="ld"  LLVM_LINK="" NM=nm OBJCOPY="objcopy"  RANLIB=ranlib STRINGS=  SIZE="size" STRIPBIN="strip"  INSTALL="install -U"  PATH=/usr/obj/usr/src/amd64.amd64/tmp/bin:/usr/obj/usr/src/amd64.amd64/tmp/usr/sbin:/usr/obj/usr/src/amd64.amd64/tmp/usr/bin:/usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/sbin:/usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/bin:/usr/obj/usr/src/amd64.amd64/tmp/legacy/bin:/usr/obj/usr/src/amd64.amd64/tmp/legacy/usr/libexec::/sbin:/bin:/usr/sbin:/usr/bin  SYSROOT=/usr/obj/usr/src/amd64.amd64/tmp make  -f Makefile.inc1  BWPHASE=everything  DESTDIR=/usr/obj/usr/src/amd64.amd64/tmp all
(cd /usr/src/lib/csu/tests/dynamic &&  DEPENDFILE=.depend.init_test  NO_SUBDIR=1 make -f /usr/src/lib/csu/tests/dynamic/Makefile _RECURSING_PROGS=t  PROG=init_test )
Building /usr/obj/usr/src/amd64.amd64/lib/csu/tests/dynamic/init_test.o
Building /usr/obj/usr/src/amd64.amd64/lib/csu/tests/dynamic/init_test.full
ld: error: undefined reference due to --no-allow-shlib-undefined: _pthread_getname_np
>>> referenced by /usr/obj/usr/src/amd64.amd64/tmp/lib/libc.so.7
cc: error: linker command failed with exit code 1 (use -v to see invocation)
*** Error code 1

Stop.

Why doesn't the build system accept _pthread_getname_np but accepts other _pthread_*s like _pthread_join? I already tried adding _pthread_getname_np to FBSDprivate_1.0 in lib/libthr/pthread.map and lib/libc/gen/Symbol.map but it doesn't work either.

[brooks]

It seems extremely late in the release to be doing a malloc update.

lib/libc/stdlib/jemalloc/Makefile.inc

17 ↗

(On Diff #125955)

This changes the C standard used for all of libc. It can't be part of this commit.

[minsoochoo0122_proton.me]

Fixed build error with missing _pthread_getname_np symbol reference

[minsoochoo0122_proton.me]

lib/libc/stdlib/jemalloc/Makefile.inc

17 ↗

(On Diff #125955)

jemalloc 5.3.0 uses gnu11 for build. Without CSTD= gnu11 flag, the build generates these errors:

In file included from jemalloc_cache_bin.c:2:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/jemalloc_internal_includes.h:52:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/arena_structs.h:4:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/arena_stats.h:8:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/pa.h:11:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/pac.h:6:
/usr/src/contrib/jemalloc/include/jemalloc/internal/san_bump.h:12:25: error: redefinition of typedef 'ehooks_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef struct ehooks_s ehooks_t;
                        ^
/usr/src/contrib/jemalloc/include/jemalloc/internal/ehooks.h:21:25: note: previous definition is here
typedef struct ehooks_s ehooks_t;
                        ^
In file included from jemalloc_cache_bin.c:2:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/jemalloc_internal_includes.h:52:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/arena_structs.h:4:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/arena_stats.h:8:
In file included from /usr/src/contrib/jemalloc/include/jemalloc/internal/pa.h:11:
/usr/src/contrib/jemalloc/include/jemalloc/internal/pac.h:77:22: error: redefinition of typedef 'pac_t' is a C11 feature [-Werror,-Wtypedef-redefinition]
typedef struct pac_s pac_t;
                     ^
/usr/src/contrib/jemalloc/include/jemalloc/internal/san_bump.h:13:22: note: previous definition is here
typedef struct pac_s pac_t;

Therefore, I think that bumping CSTD for libc to c11 is a necessary step to adopt jemalloc 5.3.0. I tried cleanworld, buildworld, installworld and rebooted, and luckily, the system boots and works fine without any crash or error messages. In this case, should I open a new revision regarding to changing CSTD to gnu11 for libc?

[brooks]

lib/libc/stdlib/jemalloc/Makefile.inc

17 ↗

(On Diff #125955)

If we're going to change the C standard for libc we can't just rely on a reboot smoke test. We need to examine any new warnings generated and solicit input about that change on its own. It's probably a good thing to do, but we can't sneak it in as a part of another change.

FWIW should be trivial to add ifdef/define/endif guards around the above typedefs to address the errors above as a workaround.

[jrtc27]

lib/libc/stdlib/jemalloc/Makefile.inc

17 ↗

(On Diff #125955)

.for src in ${JEMALLOCSRCS}
CFLAGS.${src}+=-std=gnu11
.endfor

should work, FWIW, if needed

[minsoochoo0122_proton.me]

Created separate revision for pthread_getname_np and _pthread_getname_np. This revision depends on D41461

[minsoochoo0122_proton.me]

Removed gnu11 flag and fixed gnu99 incompatibility

[minsoochoo0122_proton.me]

Updating diff file to conform recent changes in lib/libc/stdlib/malloc

[minsoochoo0122_proton.me]

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

[instructionset_gmail.com]

In D41421#946051, @minsoochoo0122_proton.me wrote:

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

[minsoochoo0122_proton.me]

In D41421#948012, @instructionset_gmail.com wrote:

In D41421#946051, @minsoochoo0122_proton.me wrote:

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

In the release notes, there are some new features added in jemalloc 5.3.0. For example,

• Make the behavior of realloc(ptr, 0) configurable with opt.zero_realloc
• Add the thread.idle mallctl which hints that the calling thread will be idle for a nontrivial period of time.
• Add mallctl interfaces:
  • opt.zero_realloc (@davidtgoldblatt)
  • opt.cache_oblivious (@interwq)
  • opt.prof_leak_error (@yunxuo)
  • opt.stats_interval (@interwq)
  • opt.stats_interval_opts (@interwq)
  • opt.tcache_max (@interwq)
  • opt.trust_madvise (@azat)
  • prof.prefix (@zhxchen17)
  • stats.zero_reallocs (@davidtgoldblatt)
  • thread.idle (@davidtgoldblatt)
  • thread.peak.{read,reset} (@davidtgoldblatt)

and so on.

By the way, because this is not a major version bump (5.x.x), will this version bump be abled to be MFCed to stable/14?

[lwhsu]

In D41421#950213, @minsoochoo0122_proton.me wrote:

In D41421#948012, @instructionset_gmail.com wrote:

In D41421#946051, @minsoochoo0122_proton.me wrote:

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

In the release notes, there are some new features added in jemalloc 5.3.0. For example,

• Make the behavior of realloc(ptr, 0) configurable with opt.zero_realloc
• Add the thread.idle mallctl which hints that the calling thread will be idle for a nontrivial period of time.
• Add mallctl interfaces:
  • opt.zero_realloc (@davidtgoldblatt)
  • opt.cache_oblivious (@interwq)
  • opt.prof_leak_error (@yunxuo)
  • opt.stats_interval (@interwq)
  • opt.stats_interval_opts (@interwq)
  • opt.tcache_max (@interwq)
  • opt.trust_madvise (@azat)
  • prof.prefix (@zhxchen17)
  • stats.zero_reallocs (@davidtgoldblatt)
  • thread.idle (@davidtgoldblatt)
  • thread.peak.{read,reset} (@davidtgoldblatt)

and so on.

It would be really nice if we can test this against with the tests under /usr/tests . I know it is still very tricky to run it. If you are interested, please check the scripts at https://github.com/freebsd/freebsd-ci and let me know (and/or -testing mailing list) if you have any questions.

By the way, because this is not a major version bump (5.x.x), will this version bump be abled to be MFCed to stable/14?

Theoretically, yes, but it would be nice to check the compatibility thoroughly.

[brooks]

I think it might make sense to split the switch to the jemalloc memalign() into a separate commit.

[minsoochoo0122_proton.me]

• Use memalign from jemalloc

[jrtc27]

lib/libc/gen/memalign.c

38 ↗

(On Diff #127096)

Does jemalloc's implementation handle this compatibly?..

[minsoochoo0122_proton.me]

lib/libc/gen/memalign.c

38 ↗

(On Diff #127096)

While glibc implementation passes to malloc when alignment is 0, jemalloc implementation and illumos implementaion sets errno EINVAL and return NULL.

However, from glibc man page:

The obsolete function memalign() allocates size bytes and returns a pointer to the allocated memory. The memory address will be a multiple of alignment, which must be a power of two.

glibc's implementation for align=0 was added through PR 269688, but it is not documented in glibc. Whereas, the glibc man page says it "must be a power of two". This function is also obsolete according to glibc man page and undocumented in FreeBSD man page. Therefore, I think that there isn't enough reason for maintaining compatability despite the fact that it is obsolete and undocumented, and it is okay to adopt jemalloc/illumos way to set errno to EINVAL and return NULL

[minsoochoo0122_proton.me]

In D41421#950671, @lwhsu wrote:

In D41421#950213, @minsoochoo0122_proton.me wrote:

In D41421#948012, @instructionset_gmail.com wrote:

In D41421#946051, @minsoochoo0122_proton.me wrote:

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

In the release notes, there are some new features added in jemalloc 5.3.0. For example,

• Make the behavior of realloc(ptr, 0) configurable with opt.zero_realloc
• Add the thread.idle mallctl which hints that the calling thread will be idle for a nontrivial period of time.
• Add mallctl interfaces:
  • opt.zero_realloc (@davidtgoldblatt)
  • opt.cache_oblivious (@interwq)
  • opt.prof_leak_error (@yunxuo)
  • opt.stats_interval (@interwq)
  • opt.stats_interval_opts (@interwq)
  • opt.tcache_max (@interwq)
  • opt.trust_madvise (@azat)
  • prof.prefix (@zhxchen17)
  • stats.zero_reallocs (@davidtgoldblatt)
  • thread.idle (@davidtgoldblatt)
  • thread.peak.{read,reset} (@davidtgoldblatt)

and so on.

It would be really nice if we can test this against with the tests under /usr/tests . I know it is still very tricky to run it. If you are interested, please check the scripts at https://github.com/freebsd/freebsd-ci and let me know (and/or -testing mailing list) if you have any questions.

By the way, because this is not a major version bump (5.x.x), will this version bump be abled to be MFCed to stable/14?

Theoretically, yes, but it would be nice to check the compatibility thoroughly.

# cd /usr/src
# kyua test -k /usr/tests/Kyuafile lib/libc

passes with 0 failures.

[lwhsu]

@vangyzen: are you interested in reviewing this? as you did the update jemalloc to 5.2.1.

[vangyzen]

Thank you for asking, but I didn't actually do the last update. @jasone did it, but had to revert it. I simply un-reverted his work.

[minsoochoo0122_proton.me]

How does jemalloc 5.3.0 handle when alignment is 0

jemalloc follows the illumos way of handling alignment when it is zero, whereas the current FreeBSD and glibc returns memory space using malloc(). If we are okay with this compatability issue, this can be landed to 15-CURRENT branch. I think 12-STABLE, 13-STABLE, 14-STABLE should stick with jemalloc 5.2.1 to keep the compatability with FreeBSD's memalign.

Any ideas?

[brooks]

In D41421#977182, @minsoochoo0122_proton.me wrote:

How does jemalloc 5.3.0 handle when alignment is 0

jemalloc follows the illumos way of handling alignment when it is zero, whereas the current FreeBSD and glibc returns memory space using malloc(). If we are okay with this compatability issue, this can be landed to 15-CURRENT branch. I think 12-STABLE, 13-STABLE, 14-STABLE should stick with jemalloc 5.2.1 to keep the compatability with FreeBSD's memalign.

Any ideas?

IMO the only reason to support memalign is to be compatible. We should do what glibc does regardless of its official documentation.

[minsoochoo0122_proton.me]

In D41421#977186, @brooks wrote:

In D41421#977182, @minsoochoo0122_proton.me wrote:

How does jemalloc 5.3.0 handle when alignment is 0

jemalloc follows the illumos way of handling alignment when it is zero, whereas the current FreeBSD and glibc returns memory space using malloc(). If we are okay with this compatability issue, this can be landed to 15-CURRENT branch. I think 12-STABLE, 13-STABLE, 14-STABLE should stick with jemalloc 5.2.1 to keep the compatability with FreeBSD's memalign.

Any ideas?

IMO the only reason to support memalign is to be compatible. We should do what glibc does regardless of its official documentation.

I totally agree with that point, but I'm not sure how we can implement memalign exception and maintain it in future releases of jemalloc. This might cause confusion to some users who are already using jemalloc in other projects or systems because they know FreeBSD uses jemalloc, but we're making an exception here.

If we are going make this exception, what would be the best way to implement it? Should I patch je_memalign() thorugh FREEBSD-diffs?

[brooks]

In D41421#977198, @minsoochoo0122_proton.me wrote:

In D41421#977186, @brooks wrote:

In D41421#977182, @minsoochoo0122_proton.me wrote:

How does jemalloc 5.3.0 handle when alignment is 0

jemalloc follows the illumos way of handling alignment when it is zero, whereas the current FreeBSD and glibc returns memory space using malloc(). If we are okay with this compatability issue, this can be landed to 15-CURRENT branch. I think 12-STABLE, 13-STABLE, 14-STABLE should stick with jemalloc 5.2.1 to keep the compatability with FreeBSD's memalign.

Any ideas?

IMO the only reason to support memalign is to be compatible. We should do what glibc does regardless of its official documentation.

I totally agree with that point, but I'm not sure how we can implement memalign exception and maintain it in future releases of jemalloc. This might cause confusion to some users who are already using jemalloc in other projects or systems because they know FreeBSD uses jemalloc, but we're making an exception here.

If we are going make this exception, what would be the best way to implement it? Should I patch je_memalign() thorugh FREEBSD-diffs?

Yes, I'd restore patching it out as was done in previous releases.

[minsoochoo0122_proton.me]

In D41421#977444, @brooks wrote:

In D41421#977198, @minsoochoo0122_proton.me wrote:

In D41421#977186, @brooks wrote:

In D41421#977182, @minsoochoo0122_proton.me wrote:

How does jemalloc 5.3.0 handle when alignment is 0

jemalloc follows the illumos way of handling alignment when it is zero, whereas the current FreeBSD and glibc returns memory space using malloc(). If we are okay with this compatability issue, this can be landed to 15-CURRENT branch. I think 12-STABLE, 13-STABLE, 14-STABLE should stick with jemalloc 5.2.1 to keep the compatability with FreeBSD's memalign.

Any ideas?

IMO the only reason to support memalign is to be compatible. We should do what glibc does regardless of its official documentation.

I totally agree with that point, but I'm not sure how we can implement memalign exception and maintain it in future releases of jemalloc. This might cause confusion to some users who are already using jemalloc in other projects or systems because they know FreeBSD uses jemalloc, but we're making an exception here.

If we are going make this exception, what would be the best way to implement it? Should I patch je_memalign() thorugh FREEBSD-diffs?

Yes, I'd restore patching it out as was done in previous releases.

Would you like me to keep memalign() by using the original version in memalign.c or by adding if branch in jemalloc.c 's je_memalign?

I'll comment on diff to show what I mean

[minsoochoo0122_proton.me]

contrib/jemalloc/include/jemalloc/jemalloc.h

32

Option 1: If this is set to #undef, we can keep using our own memalign() by restoring deleted memalign.c file and related makefiles and symbol maps.

contrib/jemalloc/src/jemalloc.c

3185

Option 2:

if (alignment == 0)
    return je_malloc(size);

[brooks]

contrib/jemalloc/include/jemalloc/jemalloc.h

32

Option 1: If this is set to #undef, we can keep using our own memalign() by restoring deleted memalign.c file and related makefiles and symbol maps.

I prefer this option. The trivial implementation is fine and reduces the burden on other malloc implementations.

[minsoochoo0122_proton.me]

Restore memalign()