feat: add threadman worker for Node.js multi-threading support#184
Open
Kolezhniuk wants to merge 10 commits into
Open
feat: add threadman worker for Node.js multi-threading support#184Kolezhniuk wants to merge 10 commits into
Kolezhniuk wants to merge 10 commits into
Conversation
- Introduced `threadman_worker.js` to handle worker tasks using workerpool. - Updated `utils.js` to remove unnecessary global BigInt comment. - Refactored algebra tests to use Vitest framework, improving test structure and readability. - Converted all test files to use Vitest for assertions and lifecycle methods. - Added Vite configuration for building both Node.js and browser-compatible versions. - Implemented clean-up plugin in Vite to remove stale build artifacts before each build. - Enhanced test cases for various fields (Zq, Rat, Scalar) to ensure correctness and consistency.
- Deleted `threadman_thread.cjs` and `threadman_worker.cjs` as part of the transition to ESM. - Updated `threadman.node.js` to use a build-time injected worker path instead of relying on `__dirname`. - Modified `vite.config.js` to inject `__BUILD_WORKER_PATH__` during the build process, ensuring correct worker path resolution.
… in engine_fft.js - Fixed the assignment of `pr` in `bn128.js` to use the correct property from `bn128wasmPrebuilt`. - Enhanced the FFT implementation in `engine_fft.js` to utilize a fast WASM path when input format matches the native stride, falling back to a JS implementation otherwise. - Updated `wasm_curve.js` to return a sliced copy of the input array in `toJacobian` when the byte length matches the expected size, ensuring immutability.
- Implemented thread management logic in threadman_thread.cjs, including WebAssembly support for task execution. - Created thread function to handle memory allocation and task execution in a multi-threaded environment. - Added worker entry-point in threadman_worker.cjs to integrate with workerpool for Node.js. - Enabled task registration and execution within worker threads, ensuring compatibility with WebAssembly tasks.
- Updated @emnapi/core and @emnapi/runtime to version 1.9.2 - Updated @oxc-project/types to version 0.124.0 - Updated @rolldown/binding packages to version 1.0.0-rc.15 - Updated vitest and related packages to version 4.1.4 - Updated globals to version 17.5.0 - Updated lru-cache to version 11.3.3 - Updated vite to version 8.0.8
There was a problem hiding this comment.
Pull request overview
This PR introduces a new workerpool-based threading implementation (“threadman worker”) to support multi-threaded execution across Node.js worker_threads and browser Web Workers, while also migrating the project’s build pipeline to Vite and the test suite to Vitest (including a browser test project via Playwright).
Changes:
- Added platform adapters and a dedicated worker entrypoint to run threadman tasks via
workerpoolin Node and browsers. - Replaced Rollup configs with a unified
vite.config.jsthat builds Node CJS plus browser ESM/IIFE bundles, and added ESLint flat config. - Migrated tests from Mocha/Chai to Vitest (with both Node and browser test projects) and updated CI accordingly.
Reviewed changes
Copilot reviewed 30 out of 37 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
vite.config.js |
Adds Vite build modes (node/browser/browser-iife), test projects, and build plugins for worker script inlining and cleanup. |
eslint.config.js |
Introduces ESLint flat config and project-wide formatting rules. |
package.json |
Updates exports/imports mapping for platform adapters; switches scripts to Vite/Vitest; changes dependencies. |
package-lock.json |
Reflects dependency and tooling migration (Vite/Vitest/Playwright/workerpool, etc.). |
src/threadman.js |
Refactors thread manager to use workerpool + platform adapter hooks and eager worker initialization. |
src/threadman.node.js |
Adds Node platform adapter (concurrency, worker type, worker source resolution). |
src/threadman.browser.js |
Adds browser platform adapter (Blob worker source using inlined worker script). |
src/threadman_worker.js |
New worker_threads entrypoint registering runTask with workerpool. |
src/threadman_thread.js |
Refactors worker task logic into a factory returning runTask for workerpool integration. |
src/bn128.js |
Updates wasmcurves import and gzip decode/decompression approach for prebuilt bn128 wasm. |
src/random.js |
Refactors RNG source selection (WebCrypto vs Node crypto fallback). |
src/engine_fft.js |
Adjusts reverse-permutation path selection (WASM vs JS) depending on element stride. |
src/engine_multiexp.js |
Formatting and small structural cleanups within multiexp task building. |
src/wasm_curve.js |
Changes toJacobian to return a copy when already in Jacobian form. |
src/utils.js |
Removes /* global BigInt */ comment. |
src/scalar.js |
Removes /* global BigInt */ comment. |
src/f1field.js |
Removes /* global BigInt */ comment. |
src/polfield.js |
Minor comment indentation/formatting adjustment. |
test/algebra.js |
Migrates to Vitest APIs and updates lifecycle hooks (beforeAll/afterAll). |
test/bn128.js |
Migrates to Vitest and updates lifecycle hooks and formatting. |
test/pols.js |
Migrates to Vitest and removes Mocha-specific timeout usage. |
test/ratzqfield.js |
Migrates to Vitest and applies formatting cleanups. |
test/scalar.js |
Migrates to Vitest and updates suite/test naming. |
test/sqrt.js |
Migrates to Vitest lifecycle hooks and formatting cleanups. |
test/utils.js |
Migrates to Vitest and applies minor formatting fixes. |
test/zqfield.js |
Migrates to Vitest import style. |
.github/workflows/ci.yml |
Updates CI to use Node LTS on Ubuntu and installs Playwright for browser tests. |
build/threadman_worker.cjs |
Adds compiled CJS worker output (generated by Vite/Rolldown). |
build/threadman_thread.cjs |
Adds compiled CJS chunk for thread logic (generated by Vite/Rolldown). |
rollup.cjs.config.js |
Removes legacy Rollup config (Node build). |
rollup.browser.esm.config.js |
Removes legacy Rollup config (browser ESM build). |
.eslintrc.cjs |
Removes legacy ESLint config in favor of eslint.config.js. |
Comments suppressed due to low confidence (1)
src/bn128.js:44
- This decompression path requires
atob,Blob,DecompressionStream, andResponse.bytes(). Those Web APIs are not available in all supported Node.js runtimes, sobuildBn128()may fail in Node. Add feature detection with a Node fallback (e.g., base64 decode via Buffer and gzip inflate vianode:zlib) or keep the previous Node-compatible path.
const compressedCode = Uint8Array.from(atob(bn128wasmPrebuilt.gzipCode), c => c.charCodeAt(0));
const blob = new Blob([compressedCode]);
const ds = new DecompressionStream("gzip");
const decompressedStream = blob.stream().pipeThrough(ds);
bn128wasm.code = await new Response(decompressedStream).bytes();
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+151
to
+154
| rollupOptions: { | ||
| onwarn: suppressNodeExternalWarnings, | ||
| output: { banner: "console.log('browser iife');" }, | ||
| }, |
Comment on lines
+194
to
+198
| rollupOptions: { | ||
| external: isExternal, | ||
| onwarn: suppressNodeExternalWarnings, | ||
| output: { banner: "console.log('browser esm');" }, | ||
| }, |
Comment on lines
+242
to
+248
| external: isExternal, | ||
| output: { | ||
| banner: "console.log('node cjs');", | ||
| // Stable chunk names so the require() path never changes | ||
| // between rebuilds. | ||
| chunkFileNames: "[name].cjs", | ||
| }, |
Comment on lines
+71
to
+73
| const raw = readFileSync(abs("src/threadman_thread.js"), "utf-8"); | ||
| const fnSrc = raw.slice(raw.indexOf("export default ") + "export default ".length).trim(); | ||
|
|
Comment on lines
+74
to
+77
| // workerpool's pre-built, minified worker runtime (CJS string export). | ||
| const require = createRequire(import.meta.url); | ||
| const embeddedWorker = require("workerpool/src/generated/embeddedWorker"); | ||
|
|
Comment on lines
+2
to
+18
| on: [push, pull_request] | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Test | ||
| runs-on: ${{ matrix.os }} | ||
|
|
||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest, macos-latest, windows-latest] | ||
| node-version: [lts/*, lts/-1, lts/-2] | ||
| timeout-minutes: 15 | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout project | ||
| uses: actions/checkout@v2 | ||
| uses: actions/checkout@v6 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v3 | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: ${{ matrix.node-version }} | ||
| check-latest: true | ||
| cache: 'npm' | ||
| node-version: "lts/*" | ||
| cache: "npm" |
| run: npm ci | ||
|
|
||
| - name: Install Playwright dependencies | ||
| run: npx playwright install |
| "wasmbuilder": "0.0.16", | ||
| "wasmcurves": "file:../wasmcurves", | ||
| "web-worker": "1.5.0" | ||
| "wasmcurves": "https://github.com/iden3/wasmcurves.git#feat/esm-migration", |
| @@ -0,0 +1,22 @@ | |||
| console.log("node cjs"); | |||
| @@ -0,0 +1,153 @@ | |||
| console.log("node cjs"); | |||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
threadman_worker.jsto handle worker tasks using workerpool.utils.jsto remove unnecessary global BigInt comment.