satoshinm / NetCraft

Web-based fork of fogleman/Craft ⛺
https://satoshinm.github.io/NetCraft/
MIT License
57 stars 13 forks source link

Optimization: warning: emitted code will contain very large numbers of local variables, which is bad for performance (build to JS with -O2 or above to avoid this - make sure to do so both on source files, and during 'linking') #45

Closed satoshinm closed 7 years ago

satoshinm commented 7 years ago

emcc is giving this warning when building with the current options:

cmake -DCMAKE_TOOLCHAIN_FILE=$EMSCRIPTEN/cmake/Modules/Platform/Emscripten.cmake .. make

warning: emitted code will contain very large numbers of local variables, which is bad for performance (build to JS with -O2 or above to avoid this - make sure to do so both on source files, and during 'linking')

satoshinm commented 7 years ago

CMakeLists.txt does add -O3 using add_definitions, but is this only when compiling not linking?

add_definitions(-std=c99 -O3)
satoshinm commented 7 years ago

Adding to LINK_FLAGS, an unscientific comparison with one trial of time make:

no linker optimization flag:

build $ time make
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/admin/games/wasm/NetCraft/build
[  5%] Linking C executable craft.html
warning: emitted code will contain very large numbers of local variables, which is bad for performance (build to JS with -O2 or above to avoid this - make sure to do so both on source files, and during 'linking')
[100%] Built target craft

real    0m4.209s
user    0m3.596s
sys 0m0.502s

-O1 is about the same:

build $ time make
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/admin/games/wasm/NetCraft/build
[  5%] Linking C executable craft.html
warning: emitted code will contain very large numbers of local variables, which is bad for performance (build to JS with -O2 or above to avoid this - make sure to do so both on source files, and during 'linking')
[100%] Built target craft

real    0m4.209s
user    0m3.335s
sys 0m0.518s

-O2 is a few seconds longer:

build $ time make
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/admin/games/wasm/NetCraft/build
[  5%] Linking C executable craft.html
[100%] Built target craft

real    0m6.342s
user    0m11.043s
sys 0m0.945s

-O3 is painfully long for iterative development:

build $ time make
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/admin/games/wasm/NetCraft/build
[  5%] Linking C executable craft.html
[100%] Built target craft

real    0m27.148s
user    0m42.930s
sys 0m1.725s

Going with -O2 during linking, can consider raising higher for release builds or optimizing later. This seems to minify the JavaScript, build/craft.js now only 3.4MB instead of ~12 MB, nice. Should be about 4x faster to download.

satoshinm commented 7 years ago

A side-effect of this change is memory is served from craft.html.mem, Chrome will deny local XHR, so craft.html cannot be played from a local file:/// URL. Not a catastrophe, just serve through a local web server: python -m SimpleHTTPServer then open http://localhost:8000/craft.html.

satoshinm commented 7 years ago

A better way to enable optimization would be for an optimized "release" build, leaving the "development" build optimized for convenience purposes. Need to figure out how to best in cmake represent these alternative builds (release types, like native vs web?). And while we're at it, also add wasm https://github.com/satoshinm/NetCraft/issues/1, so at least these release/build types: