-
Notifications
You must be signed in to change notification settings - Fork 10
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
Use mmap, mprotect instead of calloc on POSIX system #171
Conversation
ksh8281
commented
Sep 18, 2023
- Fix default memory maximium size bug
@@ -33,29 +38,76 @@ Memory* Memory::createMemory(Store* store, uint64_t initialSizeInByte, uint64_t | |||
Memory::Memory(uint64_t initialSizeInByte, uint64_t maximumSizeInByte) | |||
: m_sizeInByte(initialSizeInByte) | |||
, m_maximumSizeInByte(maximumSizeInByte) | |||
, m_buffer(reinterpret_cast<uint8_t*>(calloc(1, initialSizeInByte))) |
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.
Would you add initialization of members m_buffer, m_reservedSizeInByte
here?
This will be required by code analysis tools 😢
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.
Okay!
src/runtime/Memory.cpp
Outdated
#if defined(WALRUS_32) | ||
(1024) * 1024 * 64; | ||
#else | ||
(1024) * 1024 * 512; | ||
#endif |
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.
Any specific reason for this initial reserved size?
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.
In 32-bit, we can use only 3GB address. 64MB is practical size.
if we use a lots of address, another code in process like malloc will stop to work.
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 see
* Fix default memory maximium size bug Signed-off-by: Seonghyun Kim <[email protected]>
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.
LGTM