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

wasm-tbb: increase stack size and add CI #1079

Merged
merged 12 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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