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

Dynamically load ort-wasm*.js according to the EP name #19130

Closed
wants to merge 2 commits into from

Conversation

Honry
Copy link
Contributor

@Honry Honry commented Jan 13, 2024

In wasm-factory.ts file, if the BUILD_DEFS.DISABLE_WEBGPU is false, it will load the ort-wasm*.jsep.js by default, which would cause other ep (e.g. webnn ep) fail in npm test if it is not compiled in the ort-wasm*.jsep.js. This PR is intent to dynamically load the ort-wasm*.js according to the ep name.

In wasm-factory.ts file, if the BUILD_DEFS.DISABLE_WEBGPU is false,
it will laod ort-wasm*.jsep.js by default, which would cause other
ep (e.g. webnn ep) fail in npm test if it is not compiled in the
ort-wasm*.jsep.js. This PR is intent to dynamically load the ort-wasm*.js
ccording to the ep name.
@@ -17,7 +17,7 @@ if (!BUILD_DEFS.DISABLE_TRAINING) {
BUILD_DEFS.DISABLE_WEBGPU ? require('./binding/ort-wasm.js') : require('./binding/ort-wasm-simd.jsep.js');
}

const ortWasmFactoryThreaded: EmscriptenModuleFactory<OrtWasmModule> = !BUILD_DEFS.DISABLE_WASM_THREAD ?
let ortWasmFactoryThreaded: EmscriptenModuleFactory<OrtWasmModule> = !BUILD_DEFS.DISABLE_WASM_THREAD ?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fs-eire, I was trying to move the code of ortWasmFactory and ortWasmFactory initialization to the initializeWebAssembly function, but failed.

Looks like it takes some other ort-web initialization work in somewhere. Is that possible to optimize them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initialization logic is kind of complicated. You can check #18715 for details if you are interested.

@Honry
Copy link
Contributor Author

Honry commented Jan 13, 2024

@guschmue, @fs-eire, @qjia7, PTAL, thanks!

@fs-eire
Copy link
Contributor

fs-eire commented Jan 17, 2024

@Honry I think you are going to use the ort-wasm*.jsep.wasm for webnn soon.

The real reason why we split the web assembly artifacts into 2 versions (ort-wasm*.wasm and ort-wasm*.jsep.wasm) is because in JSEP we use ASYNCIFY, which significantly increased the binary size. We have customers only interested in CPU with tiny models, and they care about the binary size, so we keep the old wasm build without asyncify and add the ".jsep" to the file name for new builds.

I saw that you are working on introducing ASYNCIFY into webNN, so I guess the easiest way is to stick with ort-wasm*.jsep.wasm, unless you have a special requirement to exclude WebGPU from your build. @guschmue is working on enabling webnn on JSEP and I think that is almost done.

And I am a little confused about why it added "BUILD_DEFS.DISABLE_WEBNN" in #18486. The BUILD_DEFS is introduced only for trimming down the generated JS file size (for example, ort.wasm.min.js will not contain webgl/webgpu JS code, so it became much smaller), but we have almost no JS code for WebNN. It does not affect how the corresponding wasm files are built.

@Honry
Copy link
Contributor Author

Honry commented Jan 17, 2024

And I am a little confused about why it added "BUILD_DEFS.DISABLE_WEBNN" in #18486. The BUILD_DEFS is introduced only for trimming down the generated JS file size (for example, ort.wasm.min.js will not contain webgl/webgpu JS code, so it became much smaller), but we have almost no JS code for WebNN. It does not affect how the corresponding wasm files are built.

Oh, really? If binary size is not a problem for WebNN EP, we can remove BUILD_DEFS.DISABLE_WEBNN. Anyway, I will try Asyncify firstly by following your guideline, thanks!

@Honry
Copy link
Contributor Author

Honry commented Jan 25, 2024

Thanks @fs-eire, we've now landed WebNN async #19145. I think we can close this PR now.

@Honry Honry closed this Jan 25, 2024
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.

2 participants