stalwart-mail: build against system jemalloc [second try]#411265
stalwart-mail: build against system jemalloc [second try]#411265happysalada merged 2 commits intoNixOS:masterfrom
Conversation
github-actions
bot
added
6.topic: nixos
yuyuyureka
force-pushed
the
stalwart-rocksdb
branch
from
972c5ea to
5ce757c
Compare
|
Also submitted the jemalloc changes upstream: stalwartlabs/stalwart#1577 |
|
Its good that you submitted this upstream. The maintainer is quite responsive, lets see at least what he says, if it was merged upstream it would be much simpler in nixpkgs. |
|
Sadly they don't want to accept my changes unless I sign a contract with them. If anyone wants to try their luck submitting the same changes under a CLA, I released them into the public domain. |
There was a problem hiding this comment.
I believe you should update the rocksdb in passthru with rocksdbJemalloc
yuyuyureka
force-pushed
the
stalwart-rocksdb
branch
2 times, most recently
from
b570d5e to
87bb56e
Compare
ofborg
bot
added
the
2.status: merge conflict
yuyuyureka
force-pushed
the
stalwart-rocksdb
branch
from
87bb56e to
3320029
Compare
ofborg
bot
removed
the
2.status: merge conflict
|
I changed this to a less invasive approach of enabling the nonprefixed feature using substituteInPlace. |
yuyuyureka
force-pushed
the
stalwart-rocksdb
branch
from
3320029 to
1768010
Compare
happysalada
left a comment
happysalada
left a comment
There was a problem hiding this comment.
This is much simpler.
Let me know when its good to merge, i see you are pushing new commits.
|
It's good to merge now |
Since NixOS#411265 and NixOS#412054 were merged at the same time, the test ended up not waiting for port 80 after the service restart.
3 participants
The reason we want to build against our own prebuilt jemalloc is that we can centrally manage all the required parameters to build a working jemalloc for many different platforms (e.g. set a fixed max-supported pagesize instead of auto-detecting it) and also it's good practise to unvendor dependencies in distro packaging.
This has previously caused problems: #411146
Stalwart sets the Rust allocator to jemalloc via the jemallocator crate.
But rocksdb does not properly distinguish between pointers it has allocated itself, and pointers which were passed in and might be registered with a different allocator.
Thus, rocksdb with jemalloc can only work properly when the malloc library is globally overridden (as can be obtained by setting the
unprefixed_malloc_on_supported_platformsfeature of jemalloc-sys, as rust-rocksdb does), but because stalwart never enables the jemalloc feature of the rocksdb crate, this never happens.I attached a patch to fix this by enabling the jemalloc feature of the rocksdb crate, which also required changing stalwart to the more maintained tikv-jemallocator to match what rust-rocksdb uses, since we can't have two crates attempting to link jemalloc at the same time.
I suggest doing this only on NixOS master/unstable, and implementing a less invasive workaround for the aarch64 pagesize issue on 25.05 stable.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.