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

Add --enable-rtld-deepbind configure flag, disable it by default except for apache #16779

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danog
Copy link
Contributor

@danog danog commented Nov 13, 2024

This pull request adds an --enable-rtld-deepbind flag, which enables the use of RTLD_DEEPBIND for dlopen, and changes the default behavior of dlopen to use RTLD_DEEPBIND only when compiling the Apache SAPI.

Re: #10670, #11094

This fixes compatibility with custom allocators like jemalloc on newer glibc versions, while still retaining the old DEEPBIND behavior for apache, which seems to have been the reason why it was added.

@danog
Copy link
Contributor Author

danog commented Nov 13, 2024

Might be worth backporting into 8.3, as it is technically a bugfix IMO...

@danog
Copy link
Contributor Author

danog commented Nov 19, 2024

Ping? :)

@danog
Copy link
Contributor Author

danog commented Nov 21, 2024

Ping @nielsdos / @cmb69

@nielsdos
Copy link
Member

I was not a fan of the idea before and I still am not. Why do people want to change the allocator using a preload hack anyway? For almost anything request-related, PHP has its own allocator so that would bypass jemalloc or whatever you use anyway.
Also, how does this patch play with building for multiple SAPIs at the same time?
This seems like one of those configurations that will cause more trouble than it's worth tbh.

@danog
Copy link
Contributor Author

danog commented Nov 21, 2024

Changing the allocator with LD_PRELOAD is pretty much the standard way it is done for jemalloc, mimalloc, and the many other allocators currently being used in production to improve performance.

I understand PHP also has its own internal custom allocator with its own memory pool, which greatly improves performance, but I don't see why should a legacy change made in 2005 to workaround some apache issues (not relevant on standalone executables like php-cli/php-fpm) completely prevent users from making use of the additional (even if small) performance improvements offered by jemalloc (on top of zend's allocator).

Users may wish to replace the allocator for the entire systems (i.e. by setting an ENV LD_PRELOAD in a docker container, affecting all applications within), and PHP would be the only major application that breaks this.

Regarding how RTLD_DEEPBIND is disabled, I do not have any strong opinions (i.e. it could even be an ini entry), but the approach in this PR is the approach recommended by @dstogov in #11094 (comment).

@danog
Copy link
Contributor Author

danog commented Nov 21, 2024

Also, as a small factoid, the improvements of using jemalloc or other custom allocators instead of the system's libc malloc are non-trivial, depending on the flavor of libc being used: for example, @Bilge's ScriptFUSION project imports suffered 30% slowdowns on MUSL libc, with the zend allocator enabled; using jemalloc (on top of the usual zend allocator) restored glibc-like performance.

@Bilge
Copy link

Bilge commented Nov 21, 2024

Can confirm. Jemalloc is only fractionally slower than glibc (but it is slower) and I use it in production on Alpine to benefit from smaller images.

@danog
Copy link
Contributor Author

danog commented Nov 21, 2024

Can confirm. Jemalloc is only fractionally slower than glibc (but it is slower) and I use it in production on Alpine to benefit from smaller images.

Yep, same here, I use jemalloc on my alpine MadelineProto images, because MUSL libc malloc is absolutely unusable.

PHP has its own allocator so that would bypass jemalloc or whatever you use anyway.

The zend allocator does use mmap directly, but the libc allocator can still significantly affect performance

@nielsdos
Copy link
Member

I don't care about this enough to want to spend energy on this. But if this breaks and causes issue reports I will ignore them too.

@cmb69
Copy link
Member

cmb69 commented Nov 21, 2024

I can't say much about this (mostly working on Windows), but if MUSL's allocator is so slow, why don't they use jemalloc (or something else) in the first place?

@danog
Copy link
Contributor Author

danog commented Nov 22, 2024

Well, that's a question for the MUSL libc maintainers :)

@danog
Copy link
Contributor Author

danog commented Nov 22, 2024

@iluuu1994 Mind taking a look too & merging if possible?

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