Skip to content

Conversation

@mho22
Copy link
Collaborator

@mho22 mho22 commented Nov 17, 2025

Motivation for the change

We need this pull request in order for our PHP.wasm build to be built without RELOCATABLE and avoid size increases when building with MAIN_MODULE. The pull request is in 4.0.19 release.

Implementation details

  • Modified emscripten version in base-image Dockerfile.
  • Removed setErrNo and malloc from EXPORT_RUNTIME_METHODS
  • Replaced setErrNo by __errno_location in phpwasm-emscripten-library
  • Replaced PHPLoader["free"] with _free in phpwasm-emscripten-library
  • Bound wasmExports['free'] to PHPLoader['free'] in php/Dockerfile
  • Bound wasmExports['malloc'] to PHPLoader['malloc'] in php/Dockerfile
  • Removed fd_close from JSPI_IMPORTS and JSPI_EXPORTS due to SuspendError: trying to suspend JS frames
  • Ignored default.profraw file in .gitignore
  • Added __funcs_on_exit function in JSPI_EXPORTS to solve issues from PHP7.4 and PHP7.3
  • Added __funcs_on_exit function in EXPORTED_FUNCTIONS to solve issues from PHP7.4 and PHP7.3
  • Modified patches related to signature mismatch in zend_list_free_wrapper in PHP7.4 and PHP7.3
  • Added HEAPU32 and HEAPU8 in EXPORT_RUNTIME_METHODS
  • Refactored php-crash.spec.ts file to run test on each supported php version
  • Added _malloc function in EXPORTED_FUNCTIONS
  • Rearranged first test in php-crash.spec.ts
✅ make all

✅ npm run recompile:php:node:jspi:all
✅ nx run php-wasm-compile:xdebug:jspi:all
✅ nx run php-wasm-compile-shared:intl:jspi:all 

✅ npm run recompile:php:node:asyncify:all
✅ nx run php-wasm-compile:xdebug:asyncify:all
✅ nx run php-wasm-compile-shared:intl:asyncify:all

✅ npm run recompile:php:web:jspi:all

✅ npm run recompile:php:web:asyncify:all

Testing Instructions

CI

@adamziel
Copy link
Collaborator

The remaining failure seems unrelated to this PR 👍 Let's rebuild all the wasm files to make sure all the tests pass and we're good! I love how much smaller the built binaries are becoming.

@mho22
Copy link
Collaborator Author

mho22 commented Nov 19, 2025

@adamziel Me too! I am currently rebuilding PHP.wasm node Asyncify as the 5th step in the description list. This will unlock the Intl dynamic extension for Web PR.

I will create a new pull request to try to correct the failed test as I think the current failure could be related to PHP not running exit() in the afterEach step in the blueprints tests. I couldn't reproduce the failure locally so this is difficult to diagnose but I remember doing this for the different test-unit-asyncify some months ago.

@mho22
Copy link
Collaborator Author

mho22 commented Nov 19, 2025

@adamziel The Does not crash due to an unhandled Asyncify error in php-crash.spec.ts doesn't return an unreachable Error anymore. It only hangs indefinitely. In order to make the tests pass with this upgrade I commented out the test and I suggest creating a follow-up pull request if you consider that a test like that is mandatory. What do you think?

@mho22 mho22 force-pushed the upgrade-emscripten-version-to-nineteen branch from 45be38f to 0f20c02 Compare November 19, 2025 17:01
@adamziel
Copy link
Collaborator

@mho22 following up sounds good, but I'm suspicious about it not failing anymore. Are we adding something to the asyncify list in this PR? Why is it not failing anymore?

@mho22
Copy link
Collaborator Author

mho22 commented Nov 19, 2025

@adamziel I will investigate. I didn't add anything to the ASYNCIFY list but I think there could be two reasons :

  1. The new EXPORTED_RUNTIME_METHODS [ HEAPU32 and HEAPU8 ] and new EXPORTED_FUNCTIONS [ ___errno_location ]. These functions could have been related to that. But I doubt it.

  2. The multiple improvements between emscripten 4.0.5 and 4.0.19.

I suspect the second option.

@mho22
Copy link
Collaborator Author

mho22 commented Nov 19, 2025

In order to keep consistency between every PHP version I will need to re-run the compilation of every PHPs... Since a new version of PHP 8.4 has been released and a minor change has been added in Dockerfile for PHP Web ASYNCIFY...

I hope we won’t need to upgrade Emscripten again for a long time after this pull request 😅

@mho22
Copy link
Collaborator Author

mho22 commented Nov 19, 2025

@adamziel That's it! It is now ready for review!

@mho22 mho22 marked this pull request as ready for review November 19, 2025 23:45
@mho22 mho22 requested a review from a team as a code owner November 19, 2025 23:45
adamziel pushed a commit that referenced this pull request Nov 20, 2025
## Motivation for the change, related issues

Based on [this
comment](#2910 (comment))

The `nx run playground-blueprints:"test:vite"` command systematically
fails due to 10 seconds timeout on the `should activate a plugin file
located in the plugins directory` test.

IIRC I also faced that situation with the PHP.wasm node tests and adding
dispose methods made the trick. Let's see if it works here too.

## Implementation details

Adding the following portion of code where necessary :

```typescript
afterEach(async () => {
	php.exit();
	await handler[Symbol.asyncDispose]();
});
```

## Testing Instructions

CI
@mho22 mho22 force-pushed the upgrade-emscripten-version-to-nineteen branch from 42fd81c to d8c329a Compare November 20, 2025 10:44
@mho22
Copy link
Collaborator Author

mho22 commented Nov 20, 2025

I will convert this to draft for now by adding the failing crash test again. It works for versions 8.4 8.3 and 8.2 so I guess it times out even if it returns the unreachable errors.

@mho22 mho22 marked this pull request as draft November 20, 2025 11:19
@mho22
Copy link
Collaborator Author

mho22 commented Nov 20, 2025

@adamziel I was wrong earlier, nothing was broken, the code had to be rearranged! Let's wait the check results. Ready for review again!

@mho22 mho22 marked this pull request as ready for review November 20, 2025 13:56
@adamziel adamziel merged commit 35e97de into trunk Nov 20, 2025
32 checks passed
@adamziel adamziel deleted the upgrade-emscripten-version-to-nineteen branch November 20, 2025 15:06
@adamziel
Copy link
Collaborator

🎉 🎉 🎉

@adamziel
Copy link
Collaborator

Node.js WASM binaries are significantly smaller, I understand that's because we've extracted the INTL extension:

CleanShot 2025-11-21 at 17 08 01@2x

However, Web Binaries are a little bit larger:

CleanShot 2025-11-21 at 17 08 59@2x

Do we understand why? Let's not block the intl work on that, but it would be great to investigate as a follow-up. Maybe we have to live with this now, but maybe there's a way to remove 6MB or so?

@mho22
Copy link
Collaborator Author

mho22 commented Nov 24, 2025

@adamziel Ow! This pull request still had Intl supported as a static extension. The Intl dynamic extension in web pull request should have done the 6Mb decrease. However it didn't... I don't know why. I am on it.

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.

3 participants