-
Notifications
You must be signed in to change notification settings - Fork 275
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
php-wasm: easier to hit memory limit without PHP using mmap() and munmap() #1278
Comments
@brandonpayton let's also fill (or bump) an issue in the Emscripten repository, link here, and cc @ThomasTheDane. |
@adamziel yep, that was on my list to do next, except for pinging the specific person. Will ping them. Thank you! |
Filed an issue for Emscripten: |
I added some notes to the Emscripten issue. The most recent is emscripten-core/emscripten#21816 (comment)
I'm not sure how important this is. It probably makes sense to wait and see how many issues there are with memory limits before investing further. |
There is some ongoing work that might eventually pave the way for deeper mmap support in Emscripten. See: The work is being discussed in the following ticket which seems to be a very interesting read: |
TL;DR: Without mmap() and munmap(), PHP cannot adjust memory regions in place and has to compensate by keeping both an old and a new region allocated at the same time. This requires more memory and triggers OOM errors more often than in-place adjustment.
Details
We stopped a memory leak in #1128 by stopping PHP's use of mmap() and munmap() to allocate memory. This was necessary because Emscripten's mmap() and munmap() implementations are incomplete and violate basic assumptions PHP makes about them.
Unfortunately, PHP relies upon mmap() to extend and munmap() to truncate memory in place, and when PHP cannot adjust memory regions in place, it has to hold onto the existing memory region while allocating a completely separate, new memory region with an updated size.
For the sake of discussion, let
M
be the size of the current memory region andC
be the desired change in size. When adjusting a region in place, we requireM + C
bytes, but without the ability to adjust regions in place, we requireM + (M + C)
bytes to make that same adjustment.This applies both for both truncating and extending, but extending requires the most space.
M + (M - C)
bytesM + (M + C)
bytesExample
We can see this using a version of the use-all-memory plugin from @sejas. Let's compare use-all-memory logs between vanilla PHP and php-wasm when running with a memory limit of 256MB.
Vanilla PHP 8.3
This version is able to adjust memory regions in place using mmap() and munmap() and reaches OOM just as it tries to extend beyond the memory limit.
php-wasm with PHP 8.3
This version is not able to adjust memory regions in place and reaches OOM earlier, when the combined sizes of the old and new regions exceed the memory limit.
How to fix
To fix this, we need Emscripten to provide mmap() and munmap() implementations that can resize anonymous mmap() regions in-place.
The Emscripten team has said certain things are not possible with the platform, including partial munmap(), but I do not believe those statements should apply to anonymous mmap() regions which are basically just allocated memory (rather than other devices mapped into virtual memory space). I am planning to file an Emscripten issue to discuss this, and if we do not get any traction, we might at least make an Emscripten PR as an example of what can be done.
cc folks who were involved or interested in the recent memory leak fix: @adamziel @sejas @dmsnell @wojtekn
The text was updated successfully, but these errors were encountered: