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

Add possibility for disabling inlining wasm in opencv.js #23344

Merged
merged 1 commit into from Mar 21, 2023

Conversation

anderskiaer
Copy link
Contributor

@anderskiaer anderskiaer commented Mar 11, 2023

Closes #22530 and also related to #15315.

Pull Request Readiness Checklist

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@@ -233,6 +235,7 @@ def build_loader(self):
parser.add_argument('--emscripten_dir', default=emscripten_dir, help="Path to Emscripten to use for build (deprecated in favor of 'emcmake' launcher)")
parser.add_argument('--build_wasm', action="store_true", help="Build OpenCV.js in WebAssembly format")
parser.add_argument('--disable_wasm', action="store_true", help="Build OpenCV.js in Asm.js format")
parser.add_argument('--disable_single_file', action="store_true", help="Do not merge JavaScript and WebAssembly into one single file")
Copy link
Member

Choose a reason for hiding this comment

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

I think it make sense highlight wasm in the flag naming. Something like --disable_wasm_merge or --build_standalone_wasm.

Copy link
Member

Choose a reason for hiding this comment

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

However, origin option is SINGLE_FILE so it's also logical to keep --disable_single_file 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, pros and cons, no strong opinion from my side on argument name 🙂 If we are to move away from single_file in the argument name I think I personally like your second suggestion the best, i.e. --build_standalone_wasm (we then avoid the word disable, which maybe makes the argument name slightly easier to interpret by removing a negation).

Happy to modify the PR if you decide on some other name instead of --disable_single_file.

@@ -83,6 +83,9 @@ Building OpenCV.js from Source
It requires `python` and `cmake` installed in your development environment.

-# The build script builds asm.js version by default. To build WebAssembly version, append `--build_wasm` switch.
By default everything is bundled into one JavaScript file by `base64` encoding the WebAssembly code. For production
builds you can add `--disable_single_file` which will reduce total size by writing the WebAssembly code
to a dedicated `.wasm` file which the generated JavaScript file will automatically load.
Copy link
Member

Choose a reason for hiding this comment

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

Can this approach fix this issue as well? #21431. Which size reduction do you observe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Single file: 8.74 MB .js
  • Separate .wasm file: 170 kB .js + 6.43 MB .wasm

I.e. bundling the generated webassembly code with base64 is ~32-33 % larger in total compared with separate .wasm file (consistent with typical base64 encoding size increases).

@asmorkalov asmorkalov added this to the 4.8.0 milestone Mar 21, 2023
@asmorkalov asmorkalov merged commit 0d082ce into opencv:4.x Mar 21, 2023
17 of 18 checks passed
@asmorkalov asmorkalov mentioned this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WASM modules should not be stored as Base64
3 participants