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

Build not fully optimized? #3

Open
kripken opened this issue Jan 4, 2018 · 17 comments
Open

Build not fully optimized? #3

kripken opened this issue Jan 4, 2018 · 17 comments

Comments

@kripken
Copy link

kripken commented Jan 4, 2018

clang.wasm is 54315717 bytes. Running binaryen's wasm-opt -O2 shrinks it by around 15%, and -Os by 20%, which suggests maybe it wasn't fully optimized?

I see the build script uses cmake and sets it to release, which should optimize the object files, but I'm not sure how it handles the final link to wasm (in emcc, we need -Os or such during both compile and link). @juj, I think you wrote the emcmake code, what does it do for the final link?

cc @mbebenita

@kripken
Copy link
Author

kripken commented Jan 4, 2018

Oh wait, is this using emscripten with the LLVM wasm backend, and not fastcomp/asm2wasm? Seems so from the list of repos in build.py. If so, then the 15-20% size difference is not that surprising, as the wasm backend tends to emit larger code. Might be worth adding a build step to run binaryen's optimizer, then.

@tbfleming
Copy link
Owner

is this using emscripten with the LLVM wasm backend

Yes.

Might be worth adding a build step to run binaryen's optimizer, then.

Sounds good.

@juj
Copy link

juj commented Jan 4, 2018

If using CMake and e.g. Unix Makefiles generator, then one should pass -DCMAKE_BUILD_TYPE=Release to the build if not otherwise specified, as CMake defaults to building Debug build type, which is unoptimized. emcmake does not reach the linking phase to do any adjustments.

@tbfleming
Copy link
Owner

Here's what it currently uses:

  • -DCMAKE_BUILD_TYPE=Release to build llvm's *.a
  • -DCMAKE_BUILD_TYPE=Debug to build the final executable. Release crashes during build.

tbfleming added a commit that referenced this issue Jan 4, 2018
tbfleming added a commit that referenced this issue Jan 4, 2018
@tbfleming
Copy link
Owner

It's now down to 43M.

@kripken
Copy link
Author

kripken commented Jan 4, 2018

I'm a little confused by

@juj: emcmake does not reach the linking phase to do any adjustments.

@tbfleming -DCMAKE_BUILD_TYPE=Debug to build the final executable.

Is cmake issuing the final emcc command to link and generate the executable? I don't see an emcc command in the build script, so it seems like that's the case, but if I understand you @juj that shouldn't be possible?

Release crashes during build.

Oh, do you have an error message or stack trace from that? EMCC_DEBUG=1 in the env could give more info.

@tbfleming
Copy link
Owner

Is cmake issuing the final emcc command to link and generate the executable?

Yes. Here's the commands which create the app. I'm running it through release to give you a log.

cd /mnt/cib/build/clang-browser-Release/ && emcmake cmake -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DLLVM_BUILD=/mnt/cib/build/llvm-browser-Release/ -DEMSCRIPTEN=on ../../src

cd /mnt/cib/build/clang-browser-Release/ && ninja clang

-DLLVM_BUILD=... and -DEMSCRIPTEN=on are variables in my CMakeLists.txt.

@tbfleming
Copy link
Owner

Oops. Sorry; I remembered wrong. It builds but hits this at runtime:

stackAlloc is not defined

@kripken
Copy link
Author

kripken commented Jan 4, 2018

1: That stackAlloc issue might be fixed by emscripten-core/emscripten#5986 (Please let me know either way if you check.)

2: Is the relevant CMakeLists.txt this one? https://github.com/tbfleming/cib/blob/master/src/CMakeLists.txt#L105

If so then it looks like cmake is picking the -Ox optimization level for the final link stage. (@juj, is that odd?) Some notes on the flags:

@tbfleming
Copy link
Owner

1: I'll try it
2: Yes

I forgot to remove the SAFE_HEAP after debugging.

This reminds me of an issue: NO_EXIT_RUNTIME doesn't stop it from closing stdio. I had to do this instead: https://github.com/tbfleming/cib/blob/master/src/process.js#L137

@tbfleming
Copy link
Owner

emscripten-core/emscripten#5986 fixed the stackAlloc issue. I also removed SAFE_HEAP, ASSERTIONS, DEMANGLE_SUPPORT, and NO_EXIT_RUNTIME.

Step 1 Release build 41M
Step 2 wasm-opt -Os 36M
Step 3 combine data segments 37M

Step 3 is because the builds sometimes produce around 130,000 data segments, which exceed Firefox's and Chrome's 100,000 limit. I combine them into a single segment.

tbfleming added a commit that referenced this issue Jan 4, 2018
tbfleming added a commit that referenced this issue Jan 4, 2018
@kripken
Copy link
Author

kripken commented Jan 7, 2018

Is that stdio issue fixed by emscripten-core/emscripten#6038 ?

About wasm having a 100,000 segment limitation, I wasn't aware of that limit. @lukewagner , is that a temporary thing, or should binaryen work around it?

@tbfleming
Copy link
Owner

emscripten-core/emscripten#6038 fixed it.

@lukewagner
Copy link

It's a permanent limit and should be the same across browsers since it is in the list of constant limits. If there was a hard reason to relax it, it could be relaxed.

@kripken
Copy link
Author

kripken commented Jan 8, 2018

Ok, thanks for the info, I'll handle it in binaryen.

@kripken
Copy link
Author

kripken commented Jan 8, 2018

@tbfleming WebAssembly/binaryen#1350 should work around the 100K segment limit, please verify it works if you can.

@tbfleming
Copy link
Owner

@kripken that fixed it. Nice branch name :)

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

No branches or pull requests

4 participants