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

php-wasm: easier to hit memory limit without PHP using mmap() and munmap() #1278

Open
brandonpayton opened this issue Apr 19, 2024 · 5 comments
Labels
[Feature] PHP.wasm [Priority] Medium [Type] Bug An existing feature does not function as intended

Comments

@brandonpayton
Copy link
Member

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 and C be the desired change in size. When adjusting a region in place, we require M + C bytes, but without the ability to adjust regions in place, we require M + (M + C) bytes to make that same adjustment.

This applies both for both truncating and extending, but extending requires the most space.

  • Truncating using two regions requires M + (M - C) bytes
  • Extending using two regions requires M + (M + C) bytes

Example

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.

* memory_get_usage(false): 251.38530731201 MB
* memory_get_usage(true): 255.00390625 MB
PHP Fatal error:  Allowed memory size of 268435456 bytes exhausted (tried to allocate 264241184 bytes) in /Users/brandonpayton/src/playground-temp/index.php on line 16

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.

* memory_get_usage(false): 118.42160797119 MB
* memory_get_usage(true): 138.0625 MB
Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 124780568 bytes) in /wordpress/index.php on line 16

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

@brandonpayton brandonpayton added [Type] Bug An existing feature does not function as intended [Priority] Medium [Feature] PHP.wasm labels Apr 19, 2024
@adamziel
Copy link
Collaborator

adamziel commented Apr 19, 2024

@brandonpayton let's also fill (or bump) an issue in the Emscripten repository, link here, and cc @ThomasTheDane.

@brandonpayton
Copy link
Member Author

brandonpayton commented Apr 19, 2024

let's also fill (or bump) an issue in the Emscripten repository,

@adamziel yep, that was on my list to do next, except for pinging the specific person. Will ping them. Thank you!

@brandonpayton
Copy link
Member Author

Filed an issue for Emscripten:
"Missing mmap()/munmap()/mremap() features for in-place adjustment of anonymous mappings"
emscripten-core/emscripten#21816

@brandonpayton
Copy link
Member Author

brandonpayton commented May 10, 2024

I added some notes to the Emscripten issue. The most recent is emscripten-core/emscripten#21816 (comment)

So the constant cost of "reallocating" memory with posix_memalign() is N + (N + G) because old must be copied to new.

And the comparative costs of using mremap/mmap are:

Better: N + G
Equivalent: N + (N + G)
Worse: N + ((N + G) + A)

where N is the size of the original region, G is the amount of growth, and A is the size of the alignment (2MB).

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.

@brandonpayton
Copy link
Member Author

There is some ongoing work that might eventually pave the way for deeper mmap support in Emscripten. See:
emscripten-core/emscripten#21816 (comment)

The work is being discussed in the following ticket which seems to be a very interesting read:
emscripten-core/emscripten#21620

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] PHP.wasm [Priority] Medium [Type] Bug An existing feature does not function as intended
Projects
Status: No status
Development

No branches or pull requests

2 participants