Skip to content

Commit a14a906

Browse files
committed
Address review comments:
- Reduce STACK_SIZE to 256KB - Update Makefile to be in sync with Bazel build rule Signed-off-by: Michael Warres <[email protected]>
1 parent 5f4384e commit a14a906

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

Makefile

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,23 @@ PKG_CONFIG_PATH = ${EMSDK}/upstream/emscripten/cache/sysroot/lib/pkgconfig
2929
WASM_LIBS = $(shell $(PKG_CONFIG) $(WASM_DEPS) $(PROTO_DEPS) \
3030
--with-path=$(PKG_CONFIG_PATH) --libs | sed -e 's/-pthread //g')
3131

32+
# See proxy_wasm_cc_binary build rule definition in bazel/defs.bzl for
33+
# explanation of emscripten link options.
34+
EMSCRIPTEN_LINK_OPTS := --no-entry \
35+
--js-library ${PROXY_WASM_CPP_SDK}/proxy_wasm_intrinsics.js \
36+
-sSTANDALONE_WASM -sEXPORTED_FUNCTIONS=_malloc \
37+
-sALLOW_MEMORY_GROWTH=1 -sSTACK_SIZE=256KB -sINITIAL_HEAP=1MB
38+
39+
3240
debug-deps:
3341
# WASM_DEPS : ${WASM_DEPS}
3442
# WASM_LIBS : ${WASM_LIBS}
3543
# PROTO_DEPS: ${PROTO_DEPS}
3644
# PROTO_OPTS: ${PROTO_OPTS}
3745

38-
# TODO(mpwarres): Add Emscripten stack/heap size params in PR#174.
3946
%.wasm %.wat: %.cc
40-
em++ --no-entry -sSTANDALONE_WASM -sEXPORTED_FUNCTIONS=_malloc \
41-
--std=c++17 -O3 -flto \
42-
--js-library ${PROXY_WASM_CPP_SDK}/proxy_wasm_intrinsics.js \
47+
em++ --std=c++17 -O3 -flto \
48+
${EMSCRIPTEN_LINK_OPTS} \
4349
-I${PROXY_WASM_CPP_SDK} \
4450
${CPP_CONTEXT_LIB} \
4551
${PROTO_OPTS} \

bazel/defs.bzl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,10 @@ def proxy_wasm_cc_binary(
104104
"-sALLOW_MEMORY_GROWTH=1",
105105
# Total stack size (fixed). Emscripten default stack size changed from 5MiB to 64KiB in
106106
# 3.1.27: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md,
107-
# https://github.com/emscripten-core/emscripten/pull/18191. We pick 1MiB somewhat
108-
# arbitrarily, since it is gives a little more room and is easy to remember.
109-
"-sSTACK_SIZE=1MB",
107+
# https://github.com/emscripten-core/emscripten/pull/18191. We pick 256KB as a balance
108+
# between reducing memory size and providing more headroom in case of deeper call
109+
# stacks. For comparison, the Rust SDK uses 1MB stack by default.
110+
"-sSTACK_SIZE=256KB",
110111
# Initial amount of heap memory
111112
"-sINITIAL_HEAP=1MB",
112113
],

0 commit comments

Comments
 (0)