Add pthread support for Emscripten build. #842
Comments
|
Are trying to build the unit tests in WASM? Did you actually get performance gains compiling enkiTS in WASM? This does not appear to be a supported platform for enkiTS. https://github.com/dougbinks/enkiTS I don't think it makes sense for me to try to support this without a github action that verifies it builds and runs correctly. You might also work with the enkiTS team so they run tests in WASM. I don't see such a target: https://github.com/dougbinks/enkiTS/blob/master/.github/workflows/build.yml I would like to support WASM better, but it is a difficult platform to work with. |
|
Yes, I was trying to build the unit tests in WASM as the first step in writing WASM bindings for this library (similar to https://github.com/Birch-san/box2d-wasm for Box2D 2.x). Despite enkiTS not specifying it is WASM compatible, it appears to be with the changes I made to the Emscripten build script in [#841], as the unit tests run successfully in most modern browsers with the change. The change to the build script enables pthread support https://emscripten.org/docs/porting/pthreads.html, which is all that seems to be necessary to get enki’s TaskScheduler working as expected in WASM. However, I aim to dig deeper into this as I look at writing bindings for the WASM version. One more caveat, Emscriptens implementation for pthread requires SharedArrayBuffer https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer in a browser context, which has been sandboxed due to timing attacks in Spectre. So, in order to successfully load the WASM into a browser you need to do it via a web server with the page embedding the code served with the correct CORS settings. I have a working version of this using NPM and Express [https://github.com/expressjs/express]. Would it be helpful to provide a repo with this setup? The following is the console output from Chrome Version 131.0.6778.86 (Official Build) (arm64) for MacOS both before and after the changes to the Emscripten build process enabling pthread support. The MultithreadingTest that was failing before this change now passes, which is why I am assuming enkiTS’s TaskScheduler is successfully compiled to WASM. Before (failing): test.js:546 Aborted(native code called abort()) After (passing): test.js:2077 Starting Box2D unit tests |
|
I think it makes sense to run the unit tests on WASM. I would like to know if building Box2D for WASM with enkiTS yields better performance than single threaded. If not, I could simply add a cmake option to exclude enkiTS from the unit test build. It is also not clear to me if enkiTS is a good choice of a task schedular for WASM. Maybe there are other options that work better. Maybe @dougbinks has thoughts on this. |
|
@erincatto I'm afraid I know very little about WASM. If it supports threads and atomics (as it seems to from @aleph1's tests ) then there is little reason why it would not give a similar performance boost to enkiTS on a native platform. |
|
Until I write the bindings for the new version of Box2D it’s not easy to determine, unless I write some additional performance tests in C/C++ and also compile this to WASM. I would assume with the tests passing in the browser this is working as expected. I can try to look at the test code for Box2D and see if they can be profiled for performance in the browser? Most of the tests don’t seem to include information pertaining to code execution time, and I can’t actually compile for single thread successfully, but at least if we had execution time for native C and WASM it would give people considering using the library for WASM purposes some clarity.
My aim is to start to run at the WASM bindings over the next couple of months. I currently use Rapier in my game prototyping stack which is primarily JavaScript and TypeScript based, so having a WASM build that is easy to use would mean I could try swapping Rapier for Box2D as well as releasing it to the community.
… On Nov 30, 2024, at 11:50 AM, Doug Binks ***@***.***> wrote:
@erincatto I'm afraid I know very little about WASM. If it supports threads and atomics (as it seems to from @aleph1's tests ) then there is little reason why it would not give a similar performance boost to enkiTS on a native platform.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
|
If you are able to build the unit tests on WASM, you should also be able to build the benchmark application. This allows you to specify a thread count and generates csv results. |
|
I have managed to get the benchmarks compiling as WASM, however, there are two distinct issues:
Log from Chrome Version 131.0.6778.86 (Official Build) (arm64) on MacOS 12.7.4 (21H1123): Starting Box2D benchmarks Log from Firefox 133.0 (aarch64) on MacOS 12.7.4 (21H1123): Starting Box2D benchmarks benchmark.js:2224:16
|
|
I'm guessing b2Timer is not getting implemented for emscripten. You could try changing this line to: Line 79 in 41e067c Similar for |
|
Thanks! I will try both of these and report back. |
|
Making that change and modifying the following to include Lines 118 to 120 in 2c939c2 Are these changes something you would be willing to commit if I update the pull request or would I need to maintain a fork? |
|
I can add your changes to cmake, but I will leave a note that they are not supported until I have an emscripten build in GitHub actions. There current build script is here: https://github.com/erincatto/box2d/blob/main/.github/workflows/build.yml Did you run the benchmarks again? Are you seen performance improve with threading? |
|
I am getting improved results. I temporarily hardcoded a value for The following are is the log from Chrome with the WASM compiled with support for 4 threads running on my M1 MacBook Pro. The only result that seems strange is the benchmark for smash, as it still reports 0 for ms and inf for fps. Starting Box2D benchmarks |
|
Ah, it thinks you are running debug. Can you make a release build? #ifdef NDEBUG
int stepCount = benchmarks[benchmarkIndex].totalStepCount;
#else
int stepCount = 10;
#endifThis also affects the benchmark content: int columns = BENCHMARK_DEBUG ? 20 : 120;
int rows = BENCHMARK_DEBUG ? 10 : 80;Maybe this will fix the Smash benchmark. |
|
I managed to compile a release build, but in order to do so I had to add another compiler setting for Emscripten -s ALLOW_MEMORY_GROWTH, otherwise the an error is thrown while attempting to run the first test, abortOnCannotGrowMemory. My CMakeLists.txt now includes this for Emscripten: There is a significant improvement in performance when more than a single thread is used. The following log is from Chrome vision 131.0.6778.86 running on an M1 MacBook Pro (17,1) with 8 cores (4 performance and 4 efficiency) and 16GB or RAM: Starting Box2D benchmarks |
|
Very exciting! I definitely want to support this. For comparison here are the M2 results. |
|
Great! Would you prefer I start a separate repo for creating WASM bindings to try to get better performance when calling Box2D from external javascript? Please let me know if there is anything else I can do to help with Emscripten.
… On Dec 4, 2024, at 3:43 PM, Erin Catto ***@***.***> wrote:
Very exciting! I definitely want to support this. For comparison here are the M2 results.
image.png (view on web)
https://box2d.org/files/benchmark_results.html
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
|
Yeah, something to help people get going with WASM would be great. I will add your changes to the CMake files. I played around with emscripten a while back, but it is kind of painful to work with on Windows. |
I opened a pull request [#841] before seeing the note regarding the preference for issues.
The current CMakeLists.txt doesn’t include the required flags for Emscripten’s pthread support, which causes the built .wasm file to fail the MultithreadingTest when run in a browser (tested in Chrome, Safari, and Firefox for MacOS). Adding these flags results in all tests passing when running in a browser environment.
The text was updated successfully, but these errors were encountered: