Skip to content

fix: disable wasm highlighter on riscv64#691

Open
kxxt wants to merge 3 commits intonodejs:mainfrom
kxxt:rv64
Open

fix: disable wasm highlighter on riscv64#691
kxxt wants to merge 3 commits intonodejs:mainfrom
kxxt:rv64

Conversation

@kxxt
Copy link

@kxxt kxxt commented Mar 21, 2026

Description

riscv64 with sv39 has limited virtual memory space, where creating too many (>=20) wasm memory instances fails.
See nodejs/node#60591 for more details.

This PR fixes the following error encountered during make test-only on riscv64:

[07:02:55.797] ERROR: WebAssembly.instantiate(): Out of memory: Cannot allocate Wasm memory for new instance
RangeError: WebAssembly.instantiate(): Out of memory: Cannot allocate Wasm memory for new instancemake[1]: *** [Makefile:392: test/addons/.docbuildstamp] Error 1
make: *** [Makefile:352: test-only] Error 2

I also limited the number of threads to 1 for riscv64 for the same reason, as @minify-html/wasm is used in several generators.

While at it, re-enable wasm highlighter for s390x because the corresponding bug has been fixed.

Validation

Patch tools/doc/node_modules/@nodejs/doc-kit/src/utils/highlighter.mjs and run make test-only again on riscv64 linux.

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run node --run test and all tests passed.
  • I have check code formatting with node --run format & node --run lint.
  • I've covered new added functionality with unit tests if necessary.

riscv64 with sv39 has limited virtual memory space, where creating
too many (>=20) wasm memory instances fails.
See nodejs/node#60591 for more details.

This PR fixes the following error encountered during `make test-only`
on riscv64:

    [07:02:55.797] ERROR: WebAssembly.instantiate(): Out of memory: Cannot allocate Wasm memory for new instance
    RangeError: WebAssembly.instantiate(): Out of memory: Cannot allocate Wasm memory for new instancemake[1]: *** [Makefile:392: test/addons/.docbuildstamp] Error 1
    make: *** [Makefile:352: test-only] Error 2

Signed-off-by: Levi Zim <rsworktech@outlook.com>
@kxxt kxxt requested a review from a team as a code owner March 21, 2026 07:26
@vercel
Copy link

vercel bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api-docs-tooling Ready Ready Preview Mar 21, 2026 3:20pm

Request Review

@codecov
Copy link

codecov bot commented Mar 21, 2026

Codecov Report

❌ Patch coverage is 55.55556% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.43%. Comparing base (9122dbe) to head (2ce69e3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/utils/configuration/index.mjs 42.85% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #691      +/-   ##
==========================================
- Coverage   75.84%   75.43%   -0.42%     
==========================================
  Files         149      148       -1     
  Lines       13682    13604      -78     
  Branches     1040     1028      -12     
==========================================
- Hits        10377    10262     -115     
- Misses       3299     3336      +37     
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The bug for s390x has been fixed.
@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

I get another instance of the error from @minify-html/wasm when running the tests.

[13:07:39.764] ERROR: WebAssembly.Instance(): Out of memory: Cannot allocate Wasm memory for new instance                                                                                                                                   
RangeError: WebAssembly.Instance(): Out of memory: Cannot allocate Wasm memory for new instance                                                                                                                                             
    at node:internal/modules/esm/translators:581:25                                                                                                                                                                                         
    at meta.done (node:internal/modules/esm/create_dynamic_module:84:9)                                                                                                                                                                     
    at file:///build/nodejs/src/node/tools/doc/node_modules/@minify-html/wasm/index_bg.wasm:40:13                                                                                                                                           
    at ModuleJob.run (node:internal/modules/esm/module_job:430:25)                                                                                                                                                                          
    at async node:internal/modules/esm/loader:639:26                                                                                                                                                                                        
    at async generate (file:///build/nodejs/src/node/tools/doc/node_modules/@nodejs/doc-kit/src/utils/generators.mjs:190:35)                                                                                                                
    at async file:///build/nodejs/src/node/tools/doc/node_modules/@nodejs/doc-kit/src/generators.mjs:80:22                                                                                                                                  
    at async getDependencyInput (file:///build/nodejs/src/node/tools/doc/node_modules/@nodejs/doc-kit/src/generators.mjs:36:20)                                                                                                             
    at async file:///build/nodejs/src/node/tools/doc/node_modules/@nodejs/doc-kit/src/generators.mjs:71:31                                                                                                                                  
    at async file:///build/nodejs/src/node/tools/doc/node_modules/@nodejs/doc-kit/src/generators.mjs:116:20make: *** [Makefile:845: out/doc/api/addons.html] Error 1     

I think I need to replace @minify-html/wasm with @minify-html/node for riscv64 as well.

@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

I think I need to replace @minify-html/wasm with @minify-html/node for riscv64 as well.

Unfortunately @minify-html/node uses native node add-on and there's no binary package for linux-riscv64 yet.

@avivkeller
Copy link
Member

Or we just don't support machines that don't support WASM?

@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

Or we just don't support machines that don't support WASM?

WASM is supported on riscv64. But the number of wasm memory instances is severely limited (to 20) on the most popular sv39 configuration, where only 256GiB of virtual address space is assigned to userspace thus the theoretical limit is about 256/8 = 32 instances.

@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

But in this specific case, it appears to be caused by virtual address space fragmentation:

cat trace | grep 8589996032                                                                                                                                                                                         
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = 0x3d677f0000
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 ENOMEM (Cannot allocate memory)
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 ENOMEM (Cannot allocate memory)
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 ENOMEM (Cannot allocate memory)
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 ENOMEM (Cannot allocate memory)
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 ENOMEM (Cannot allocate memory)
8433  mmap(NULL, 8589996032, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = -1 ENOMEM (Cannot allocate memory)

Only a single wasm memory instance is successfully created.

@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

Only a single wasm memory instance is successfully created.

Based on this, I limited the threads to 1 for riscv64 in the default configuration.

Now make doc-only passes for me on riscv64.

@avivkeller
Copy link
Member

Based on this, I limited the threads to 1 for riscv64 in the default configuration.

Sweet! Can you change this PR to do that, if it solves the issue? You can warn on RISC-V and override threads.

cc @nodejs/platform-riscv64 FYI

@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

Sweet! Can you change this PR to do that, if it solves the issue?

Yes, I have already updated this PR.

You can warn on RISC-V and override threads.

I think changing only the default should be fine for make doc-only because the makefile is invoking it without providing the threads parameter.

But I could implement an override if you want.

@avivkeller
Copy link
Member

What would:

Change the default, and warn if the user overrides the default with the CLI?

(Also thank you!)

@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

What would:

Change the default, and warn if the user overrides the default with the CLI?

Thanks for the suggestion! I will implement that.

And warn user if they set it to a larger value.
@kxxt
Copy link
Author

kxxt commented Mar 21, 2026

What would:
Change the default, and warn if the user overrides the default with the CLI?

Thanks for the suggestion! I will implement that.

Implemented in 2ce69e3

@joyeecheung
Copy link
Member

joyeecheung commented Mar 22, 2026

FWIW I think it's weird that tools/doc/node_modules/@nodejs/doc-kit/src/utils/highlighter.mjs is needed at all to run make test-only. The whole point of make test-only is that there's nothing about docs & style checks that it needs to run (maybe except addon tests extracted from docs - but why do we need a doc-kit + WASM monolith for that when it can just be done with a few regexes), as you are just testing the Node.js binary you built is working correctly (for an embedder or distributor).

@joyeecheung
Copy link
Member

I opened nodejs/node#62385

@kxxt
Copy link
Author

kxxt commented Mar 22, 2026

FWIW I think it's weird that tools/doc/node_modules/@nodejs/doc-kit/src/utils/highlighter.mjs is needed at all to run make test-only. The whole point of make test-only is that there's nothing about docs & style checks that it needs to run (maybe except addon tests extracted from docs - but why do we need a doc-kit + WASM monolith for that when it can just be done with a few regexes), as you are just testing the Node.js binary you built is working correctly (for an embedder or distributor).

Yeah, that makes sense. We should remove it from the dependencies of test-only. Thank you for raising this point.

This PR still fixes the make doc-only target on riscv64 if it is removed from test-only.
I see this PR has meet the requirement for merging. But I am not a commiter yet. Could someone else merge it? Thanks!

@joyeecheung
Copy link
Member

joyeecheung commented Mar 22, 2026

I opened nodejs/node#62388 to just make the process dependency free (I don't think that block this PR. It just makes PRs like this no longer necessary at all in the future).

@avivkeller
Copy link
Member

why do we need a doc-kit + WASM monolith for that when it can just be done with a few regexes?

Once this repo is a monorepo, we won't

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants