Skip to content

Commit

Permalink
wasm-tbb: increase stack size and add CI (#1079)
Browse files Browse the repository at this point in the history
* update dependencies and run wasm-tbb build

* update stack size...

* update

* format

* fix compilation error

* format

* fix

* idk what happened

* change emscripten version back

* don't test for wasm-tbb

* readme changes
  • Loading branch information
pca006132 authored Dec 1, 2024
1 parent 11387fb commit 2162d8d
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 47 deletions.
15 changes: 10 additions & 5 deletions .github/workflows/manifold.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ jobs:
build_wasm:
timeout-minutes: 30
runs-on: ubuntu-22.04
strategy:
matrix:
parallelization: [OFF, ON]
steps:
- name: Install dependencies
run: |
Expand All @@ -126,20 +129,22 @@ jobs:
# setup emscripten
git clone https://github.com/emscripten-core/emsdk.git
cd emsdk
./emsdk install 3.1.61
./emsdk activate 3.1.61
./emsdk install 3.1.64
./emsdk activate 3.1.64
- uses: jwlawson/actions-setup-cmake@v2
- name: Build WASM
- name: Build WASM ${{matrix.parallelization}}
run: |
source ./emsdk/emsdk_env.sh
mkdir build
cd build
emcmake cmake -DCMAKE_BUILD_TYPE=Release .. && emmake make
emcmake cmake -DCMAKE_BUILD_TYPE=MinSizeRel -DMANIFOLD_PAR=${{matrix.parallelization}} .. && emmake make
- name: Test WASM
if: matrix.parallelization == 'OFF'
run: |
cd build/test
node ./manifold_test.js
- name: Test examples
if: matrix.parallelization == 'OFF'
run: |
cd bindings/wasm/examples
npm ci
Expand All @@ -148,7 +153,7 @@ jobs:
cp ../manifold.* ./dist/
- name: Upload WASM files
uses: actions/upload-artifact@v4
if: github.event_name == 'push'
if: github.event_name == 'push' && matrix.parallelization == 'OFF'
with:
name: wasm
path: bindings/wasm/examples/dist/
Expand Down
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,11 @@ if(EMSCRIPTEN)
add_compile_options(-pthread)
# mimalloc is needed for good performance
add_link_options(-sMALLOC=mimalloc)
add_link_options(-sPTHREAD_POOL_SIZE=4)
# The default stack size apparently causes problem when parallelization is
# enabled.
add_link_options(-sSTACK_SIZE=10MB)
add_link_options(-sSTACK_SIZE=30MB)
add_link_options(-sINITIAL_MEMORY=32MB)
endif()
if(MANIFOLD_DEBUG)
list(APPEND MANIFOLD_FLAGS -fexceptions)
Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ The build instructions used by our CI are in [manifold.yml](https://github.com/e

### WASM

> Note: While we support compiling with `MANIFOLD_PAR=ON` in recent emscripten
> versions, this is not recommended as there can potentially be memory
> corruption issues.
To build the JS WASM library, first install NodeJS and set up emscripten:

(on Mac):
Expand All @@ -158,7 +162,7 @@ Then build:
cd manifold
mkdir buildWASM
cd buildWASM
emcmake cmake -DCMAKE_BUILD_TYPE=Release .. && emmake make
emcmake cmake -DCMAKE_BUILD_TYPE=MinSizeRel .. && emmake make
node test/manifold_test.js
```

Expand Down
4 changes: 0 additions & 4 deletions bindings/wasm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ add_custom_target(
add_custom_command(
TARGET js_deps
POST_BUILD
# fix js file
COMMAND
${Python_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/fixup.py
${CMAKE_CURRENT_BINARY_DIR}/manifold.js
# copy WASM build back here for publishing to npm
COMMAND
${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/manifold.*
Expand Down
48 changes: 48 additions & 0 deletions bindings/wasm/examples/vite-fixup-plugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// https://gist.github.com/jamsinclair/6ad148d0590291077a4ce389c2b274ea
import {createFilter} from 'vite';

function isEmscriptenFile(code) {
return /var\s+Module\s*=|WebAssembly\.instantiate/.test(code) &&
/var\s+workerOptions\s*=/.test(code);
}

/**
* Vite plugin that replaces Emscripten workerOptions with static object literal
* to fix error with Vite See project issue:
* https://github.com/emscripten-core/emscripten/issues/22394
*
* Defaults to running for all .js and .ts files. If there are any issues you
* can use the include/exclude options.
*
* @param {Object} options
* @property {string[]} [include] - Glob patterns to include
* @property {string[]} [exclude] - Glob patterns to exclude
* @returns {import('vite').Plugin}
*/
export default function emscriptenStaticWorkerOptions(options = {}) {
const filter = createFilter(options.include || /\.[jt]s$/, options.exclude);

return {
name: 'emscripten-static-worker-options',
enforce: 'pre',
transform(code, id) {
if (!filter(id)) return null;

if (!isEmscriptenFile(code)) return null;

const workerOptionsMatch =
code.match(/var\s+workerOptions\s*=\s*({[^}]+})/);
if (!workerOptionsMatch) return null;

const optionsObjectStr = workerOptionsMatch[1];
const optionsDeclarationStr = workerOptionsMatch[0];

const modifiedCode =
code.replace(optionsDeclarationStr, '')
.replace(
new RegExp('workerOptions(?![\\w$])', 'g'), optionsObjectStr);

return {code: modifiedCode, map: null};
}
};
}
6 changes: 3 additions & 3 deletions bindings/wasm/examples/vite.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
import {resolve} from 'path'
import {defineConfig} from 'vite'

import emscriptenStaticWorkerOptions from './vite-fixup-plugin.js'

export default defineConfig({
test: {testTimeout: 15000},
worker: {
format: 'es',
},
worker: {format: 'es', plugins: [emscriptenStaticWorkerOptions]},
server: {
headers: {
'Cross-Origin-Embedder-Policy': 'require-corp',
Expand Down
19 changes: 0 additions & 19 deletions bindings/wasm/fixup.py

This file was deleted.

9 changes: 3 additions & 6 deletions cmake/manifoldDeps.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ if(MANIFOLD_PAR)
FetchContent_Declare(
TBB
GIT_REPOSITORY https://github.com/oneapi-src/oneTBB.git
GIT_TAG v2021.11.0
GIT_TAG v2022.0.0
GIT_PROGRESS TRUE
EXCLUDE_FROM_ALL
)
Expand Down Expand Up @@ -112,7 +112,8 @@ if(MANIFOLD_CROSS_SECTION)
FetchContent_Declare(
Clipper2
GIT_REPOSITORY https://github.com/AngusJohnson/Clipper2.git
GIT_TAG ff378668baae3570e9d8070aa9eb339bdd5a6aba
# Nov 22, 2024
GIT_TAG a8269cafe92cdbf92572bceda5e9fdacc4684b51
GIT_PROGRESS TRUE
SOURCE_SUBDIR CPP
EXCLUDE_FROM_ALL
Expand Down Expand Up @@ -218,10 +219,6 @@ if(MANIFOLD_TEST)
endif()
endif()

if(EMSCRIPTEN)
find_package(Python REQUIRED)
endif()

if(MANIFOLD_FUZZ)
logmissingdep("fuzztest" , "MANIFOLD_FUZZ")
FetchContent_Declare(
Expand Down
19 changes: 12 additions & 7 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
export EM_CACHE=$(pwd)/.emscriptencache
mkdir build
cd build
emcmake cmake -DCMAKE_BUILD_TYPE=Release \
emcmake cmake -DCMAKE_BUILD_TYPE=MinSizeRel \
-DMANIFOLD_PAR=${if parallel then "ON" else "OFF"} \
-DFETCHCONTENT_SOURCE_DIR_GOOGLETEST=${gtest-src} \
-DFETCHCONTENT_SOURCE_DIR_TBB=${onetbb-src} \
Expand All @@ -134,11 +134,12 @@
buildPhase = ''
emmake make -j''${NIX_BUILD_CORES}
'';
checkPhase = if doCheck then ''
cd test
node manifold_test.js
cd ../
'' else "";
checkPhase =
if doCheck then ''
cd test
node manifold_test.js
cd ../
'' else "";
installPhase = ''
mkdir -p $out
cp bindings/wasm/manifold.* $out/
Expand All @@ -150,7 +151,11 @@
manifold-tbb = manifold { };
manifold-none = manifold { parallel = false; };
manifold-js = manifold-emscripten { };
manifold-js-tbb = manifold-emscripten { parallel = true; };
manifold-js-tbb = manifold-emscripten {
parallel = true;
doCheck =
false;
};
# but how should we make it work with other python versions?
manifold3d = with pkgs.python3Packages; buildPythonPackage {
pname = "manifold3d";
Expand Down
2 changes: 1 addition & 1 deletion test/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "test.h"

#if (MANIFOLD_PAR == 1)
#include <tbb/parallel_for.h>
#include <oneapi/tbb/parallel_for.h>
#endif

// we need to call some tracy API to establish the connection
Expand Down

0 comments on commit 2162d8d

Please sign in to comment.