From 20545e5cbf6d8a749dac27eeb23d0a4012375684 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Fri, 11 Oct 2024 20:32:55 +0000 Subject: [PATCH 1/4] Update and document Emscripten link-time options Set INITIAL_HEAP=1MB and ALLOW_MEMORY_GROWTH=1 to reduce starting memory usage of plugins, but allow the heap to grow. Explicitly set STACK_SIZE (previously named TOTAL_STACK) to 1MiB. This is a middle ground between Emscripten's default value prior to 3.1.27 of 5MiB and its default value since then of 64KiB. Also add comments documenting the purpose of each link-time option. Signed-off-by: Michael Warres --- bazel/defs.bzl | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/bazel/defs.bzl b/bazel/defs.bzl index 2e20a86..5ac25a8 100644 --- a/bazel/defs.bzl +++ b/bazel/defs.bzl @@ -89,10 +89,25 @@ def proxy_wasm_cc_binary( "@proxy_wasm_cpp_sdk//:proxy_wasm_intrinsics_js", ], linkopts = linkopts + [ + # Setting to indicate module is a "reactor library" without a main() entry point: + # https://emscripten.org/docs/tools_reference/settings_reference.html#standalone-wasm "--no-entry", + # File listing additional functions that Emscripten should expect to be implemented by the host: + # https://emscripten.org/docs/porting/connecting_cpp_and_javascript/Interacting-with-code.html#implement-c-in-javascript "--js-library=$(location @proxy_wasm_cpp_sdk//:proxy_wasm_intrinsics_js)", + # Emit Wasm module that can run without JavaScript "-sSTANDALONE_WASM", + # Give host code access to Emscripten's _malloc() function "-sEXPORTED_FUNCTIONS=_malloc", + # Allow allocating memory past initial heap size + "-sALLOW_MEMORY_GROWTH=1", + # Total stack size (fixed). Emscripten default stack size changed from 5MiB to 64KiB in + # 3.1.27: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md, + # https://github.com/emscripten-core/emscripten/pull/18191. We pick 1MiB somewhat + # arbitrarily, since it is gives a little more room and is easy to remember. + "-sSTACK_SIZE=1MB", + # Initial amount of heap memory + "-sINITIAL_HEAP=1MB", ], tags = tags + [ "manual", From a14a906924277beaf2d77c83cee57efb37919ea7 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 3 Mar 2025 05:59:20 +0000 Subject: [PATCH 2/4] Address review comments: - Reduce STACK_SIZE to 256KB - Update Makefile to be in sync with Bazel build rule Signed-off-by: Michael Warres --- Makefile | 14 ++++++++++---- bazel/defs.bzl | 7 ++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 165cf42..e927b7f 100644 --- a/Makefile +++ b/Makefile @@ -29,17 +29,23 @@ PKG_CONFIG_PATH = ${EMSDK}/upstream/emscripten/cache/sysroot/lib/pkgconfig WASM_LIBS = $(shell $(PKG_CONFIG) $(WASM_DEPS) $(PROTO_DEPS) \ --with-path=$(PKG_CONFIG_PATH) --libs | sed -e 's/-pthread //g') +# See proxy_wasm_cc_binary build rule definition in bazel/defs.bzl for +# explanation of emscripten link options. +EMSCRIPTEN_LINK_OPTS := --no-entry \ + --js-library ${PROXY_WASM_CPP_SDK}/proxy_wasm_intrinsics.js \ + -sSTANDALONE_WASM -sEXPORTED_FUNCTIONS=_malloc \ + -sALLOW_MEMORY_GROWTH=1 -sSTACK_SIZE=256KB -sINITIAL_HEAP=1MB + + debug-deps: # WASM_DEPS : ${WASM_DEPS} # WASM_LIBS : ${WASM_LIBS} # PROTO_DEPS: ${PROTO_DEPS} # PROTO_OPTS: ${PROTO_OPTS} -# TODO(mpwarres): Add Emscripten stack/heap size params in PR#174. %.wasm %.wat: %.cc - em++ --no-entry -sSTANDALONE_WASM -sEXPORTED_FUNCTIONS=_malloc \ - --std=c++17 -O3 -flto \ - --js-library ${PROXY_WASM_CPP_SDK}/proxy_wasm_intrinsics.js \ + em++ --std=c++17 -O3 -flto \ + ${EMSCRIPTEN_LINK_OPTS} \ -I${PROXY_WASM_CPP_SDK} \ ${CPP_CONTEXT_LIB} \ ${PROTO_OPTS} \ diff --git a/bazel/defs.bzl b/bazel/defs.bzl index 169b3da..27e9198 100644 --- a/bazel/defs.bzl +++ b/bazel/defs.bzl @@ -104,9 +104,10 @@ def proxy_wasm_cc_binary( "-sALLOW_MEMORY_GROWTH=1", # Total stack size (fixed). Emscripten default stack size changed from 5MiB to 64KiB in # 3.1.27: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md, - # https://github.com/emscripten-core/emscripten/pull/18191. We pick 1MiB somewhat - # arbitrarily, since it is gives a little more room and is easy to remember. - "-sSTACK_SIZE=1MB", + # https://github.com/emscripten-core/emscripten/pull/18191. We pick 256KB as a balance + # between reducing memory size and providing more headroom in case of deeper call + # stacks. For comparison, the Rust SDK uses 1MB stack by default. + "-sSTACK_SIZE=256KB", # Initial amount of heap memory "-sINITIAL_HEAP=1MB", ], From afab57791e03895f88e66113470f9bf765f86a96 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 3 Mar 2025 20:52:33 +0000 Subject: [PATCH 3/4] Further changes from code review - Remove STACK_SIZE override, and instead use Emscripten default of 64KB - Change INITIAL_HEAP to 64KB Signed-off-by: Michael Warres --- Makefile | 2 +- bazel/defs.bzl | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index e927b7f..2fcdafe 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ WASM_LIBS = $(shell $(PKG_CONFIG) $(WASM_DEPS) $(PROTO_DEPS) \ EMSCRIPTEN_LINK_OPTS := --no-entry \ --js-library ${PROXY_WASM_CPP_SDK}/proxy_wasm_intrinsics.js \ -sSTANDALONE_WASM -sEXPORTED_FUNCTIONS=_malloc \ - -sALLOW_MEMORY_GROWTH=1 -sSTACK_SIZE=256KB -sINITIAL_HEAP=1MB + -sALLOW_MEMORY_GROWTH=1 -sINITIAL_HEAP=64KB debug-deps: diff --git a/bazel/defs.bzl b/bazel/defs.bzl index 27e9198..87727c0 100644 --- a/bazel/defs.bzl +++ b/bazel/defs.bzl @@ -102,14 +102,8 @@ def proxy_wasm_cc_binary( "-sEXPORTED_FUNCTIONS=_malloc", # Allow allocating memory past initial heap size "-sALLOW_MEMORY_GROWTH=1", - # Total stack size (fixed). Emscripten default stack size changed from 5MiB to 64KiB in - # 3.1.27: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md, - # https://github.com/emscripten-core/emscripten/pull/18191. We pick 256KB as a balance - # between reducing memory size and providing more headroom in case of deeper call - # stacks. For comparison, the Rust SDK uses 1MB stack by default. - "-sSTACK_SIZE=256KB", - # Initial amount of heap memory - "-sINITIAL_HEAP=1MB", + # Initial amount of heap memory. 64KB matches Rust SDK starting heap size. + "-sINITIAL_HEAP=64KB", ], tags = tags + [ "manual", From 1e4863dac2e975af2ece23fc0bfe6ef4997b47d0 Mon Sep 17 00:00:00 2001 From: Michael Warres Date: Mon, 3 Mar 2025 21:02:47 +0000 Subject: [PATCH 4/4] Update build action to use actions/cache@v4 actions/cache@v1 is no longer supported: https://github.blog/changelog/2024-12-05-notice-of-upcoming-releases-and-breaking-changes-for-github-actions/#actions-cache-v1-v2-and-actions-toolkit-cache-package-closing-down Signed-off-by: Michael Warres --- .github/workflows/cpp.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 49dc761..0dc6daf 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -67,7 +67,7 @@ jobs: - uses: actions/checkout@v1 - name: Mount bazel cache - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: "/home/runner/.cache/bazel" key: bazel