-
Notifications
You must be signed in to change notification settings - Fork 42
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
Update memory configurations and simplify specification #494
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,40 +28,11 @@ IF (NOT DEFINED ENV{WASM_SRC}) | |
ENDIF() | ||
SET(WASM_SRC "$ENV{WASM_SRC}") | ||
|
||
IF (NOT DEFINED ENV{WASM_MEM_CONFIG}) | ||
MESSAGE(FATAL_ERROR "WASM_MEM_CONFIG environment variable not defined!") | ||
ENDIF() | ||
SET(WASM_MEM_CONFIG "$ENV{WASM_MEM_CONFIG}") | ||
|
||
# this should be set by the WAMR toolchain file | ||
IF (NOT DEFINED WASI_SDK_DIR) | ||
MESSAGE(FATAL_ERROR "WASM_SDK_DIR was not defined, check toolchain defines") | ||
ENDIF() | ||
|
||
# --------------------------------------------- | ||
# Set up the memory configuration | ||
# --------------------------------------------- | ||
|
||
# LINEAR_MEMORY: Maximum size for a WASM module's linear memory (module's | ||
# internal stack + static globals + padding); needs to be multiple of 64KB | ||
|
||
# INTERNAL_STACK_SIZE: Size of a WASM module's internal data stack | ||
# (part of LINEAR_MEMORY) | ||
|
||
IF (WASM_MEM_CONFIG STREQUAL "SMALL") | ||
SET(INTERNAL_STACK_SIZE 24576) | ||
SET(LINEAR_MEMORY 65536) | ||
message(STATUS "Building contracts for SMALL memory configuration") | ||
ELSEIF (WASM_MEM_CONFIG STREQUAL "LARGE") | ||
SET(INTERNAL_STACK_SIZE 98304) | ||
SET(LINEAR_MEMORY 262144) | ||
message(STATUS "Building contracts for LARGE memory configuration") | ||
ELSE() | ||
SET(INTERNAL_STACK_SIZE 49152) | ||
SET(LINEAR_MEMORY 131072) | ||
message(STATUS "Building contracts for MEDIUM memory configuration") | ||
ENDIF () | ||
|
||
# --------------------------------------------- | ||
# Set up the compiler configuration | ||
# --------------------------------------------- | ||
|
@@ -80,11 +51,7 @@ LIST(APPEND WASM_BUILD_OPTIONS "-std=c++11") | |
LIST(APPEND WASM_BUILD_OPTIONS "-DUSE_WASI_SDK=1") | ||
|
||
SET(WASM_LINK_OPTIONS) | ||
LIST(APPEND WASM_LINK_OPTIONS "-Wl,--initial-memory=${LINEAR_MEMORY}") | ||
LIST(APPEND WASM_LINK_OPTIONS "-Wl,--max-memory=${LINEAR_MEMORY}") | ||
LIST(APPEND WASM_LINK_OPTIONS "-z stack-size=${INTERNAL_STACK_SIZE}") | ||
Comment on lines
-83
to
-85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I cannot find much documentation about this (i.e., how memory is handled initially and at runtime). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this was a requirement for earlier versions of clang. this was necessary for the combination of the compiler and the interpreter. the wamr documentation no longer talks about this as a requirement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding the allocation... this effectively has nothing to do with the allocation in the interpreter. The fields in the common/interpreter/wawaka cmake file are the only ones that matter for actual allocation. |
||
LIST(APPEND WASM_LINK_OPTIONS "-Wl,--allow-undefined") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are build/link-time memory options, hence under the control of developers/users. I am wondering if any of these should be checked (somehow) by the eservice before running the related contract for safety or efficiency reasons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the numbers are too big then the interpreter will throw the error. and... in fact... i have a hard time making anything other than the default work with the current version of WAMR. and to be clear... these are contract allocations. any developer can set them to whatever is required. |
||
|
||
LIST(APPEND WASM_LINK_OPTIONS "-Wl,--export=ww_dispatch") | ||
LIST(APPEND WASM_LINK_OPTIONS "-Wl,--export=ww_initialize") | ||
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these test cases being removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mostly because they no longer fail and are redundant with other tests already in place. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,9 +17,10 @@ limitations under the License. | |
<EnclaveConfiguration> | ||
<ProdID>0x90E7</ProdID> | ||
<ISVSVN>1</ISVSVN> | ||
<StackMaxSize>0x80000</StackMaxSize> | ||
<HeapMaxSize>0x2000000</HeapMaxSize> | ||
<ReservedMemMaxSize>0x100000</ReservedMemMaxSize> | ||
<StackMaxSize>${ENCLAVE_STACK_SIZE}</StackMaxSize> | ||
<HeapMaxSize>${ENCLAVE_HEAP_SIZE}</HeapMaxSize> | ||
<ReservedMemMaxSize>${ENCLAVE_RESERVED_SIZE}</ReservedMemMaxSize> | ||
<ReservedMemInitSize>${ENCLAVE_RESERVED_SIZE}</ReservedMemInitSize> | ||
<ReservedMemExecutable>1</ReservedMemExecutable> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we switch this to 0? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think thats a deeper discussion and requires additional research on its impact for different WAMR settings. this has been our setting all along. i would suggest that we add an issue to investigate the implications. |
||
<TCSNum>2</TCSNum> | ||
<TCSPolicy>1</TCSPolicy> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this is where we'd add more tests to push the higher memory size limits?