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

Use libjemalloc if it's available for better memory usage #7919

Closed
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
2 participants
@SuperTux88
Member

SuperTux88 commented Nov 1, 2018

Fixes #6763

@SuperTux88 SuperTux88 added this to the 0.7.8.0 milestone Nov 1, 2018

@jhass

This comment has been minimized.

Member

jhass commented Nov 2, 2018

I think I'd prefer checking ldconfig -p output first if available, then whereis if available and then do the debian fallback even if both a unavailable. If you'd want to get extra fancy you could even throw pkg-config into the cascade.

@SuperTux88

This comment has been minimized.

Member

SuperTux88 commented Nov 2, 2018

ldconfig -p only works as root, while hopefully script/server is never executed as root. That's why I didn't use ldconfig.

How can I get the library path from pkg-config?

@jhass

This comment has been minimized.

Member

jhass commented Nov 2, 2018

Weird seems to work without root on the machines I have access to.

Ah nvm about pkgconfig then, looks like it won't tell us the full soname, just the libdir :(

@SuperTux88 SuperTux88 force-pushed the SuperTux88:jemalloc branch from 5195231 to 0c7a3a7 Nov 2, 2018

@SuperTux88

This comment has been minimized.

Member

SuperTux88 commented Nov 2, 2018

Weird seems to work without root on the machines I have access to.

Oh, I just tried that again and it works. I thought I already tried that, but maybe I had another error. It's not in the $PATH as user, because it's in /sbin/, but with the full path it works as user too. So I just changed it to use ldconfig and since that even works on debian, I removed the other checks, because I think we don't need them anymore.

@jhass

This comment has been minimized.

Member

jhass commented Nov 3, 2018

Oh we cannot assume it always is, rather we should check if it's in path (with command -v) and then fallback to sbin. Most distros merged all of these into /usr/bin by now or at least got rid of the bin/sbin distinction in the rootfs

@SuperTux88 SuperTux88 force-pushed the SuperTux88:jemalloc branch from 0c7a3a7 to e7a032c Nov 3, 2018

@SuperTux88

This comment has been minimized.

Member

SuperTux88 commented Nov 3, 2018

Most distros merged all of these into /usr/bin by now

I don't know which distros are "most distros", all distros I checked (debian, ubuntu and gentoo) had it in /sbin/ldconfig and not in users path, so I falsely thought it's everywhere at that place. But I check now the path first too, so it should work for "most distros" too. :) I don't know if there are distros that don't have it in the path and not in /sbin/ldconfig?

@jhass

This comment has been minimized.

Member

jhass commented Nov 3, 2018

I doubt so. The "most" came from systemd pushing for this change :)

@jhass

jhass approved these changes Nov 3, 2018

@SuperTux88 SuperTux88 closed this in b2712eb Nov 4, 2018

@SuperTux88

This comment has been minimized.

Member

SuperTux88 commented Nov 4, 2018

Merged as b2712eb

Thanks for the review @jhass 🍪

@SuperTux88 SuperTux88 deleted the SuperTux88:jemalloc branch Nov 4, 2018

comzeradd pushed a commit to libreops/librenet-ansible that referenced this pull request Dec 7, 2018

Achilleas Pipinellis
Add LD_PRELOAD variable in unicorn and sidekiq services
This change was added upstream in diaspora/diaspora#7919
and will presumably drop the Sidekiq memory usage
diaspora/diaspora#6763 (comment)

Find the path with:

```
ldconfig  -p | grep jemalloc | tr ' ' '\n' | grep '^/' | head -1
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment