Support large initial memory allocation #305
Conversation
|
I don't understand the windows error. It installs tmate, then a lot of warnings, then Does something changed in the build system? |
|
Oh, it showed the wrong error. So win32 cannot allocate 2GB memory. Maybe I need to lower it. |
Signed-off-by: Zoltan Herczeg [email protected]
| @@ -43,7 +43,7 @@ Memory::Memory(uint64_t initialSizeInByte, uint64_t maximumSizeInByte, bool isSh | |||
| , m_targetBuffers(nullptr) | |||
| , m_isShared(isShared) | |||
| { | |||
| ASSERT(initialSizeInByte <= std::numeric_limits<size_t>::max()); | |||
| RELEASE_ASSERT(initialSizeInByte <= std::numeric_limits<size_t>::max()); | |||
There was a problem hiding this comment.
I have changed this because of 32 bit systems.
There was a problem hiding this comment.
Why did you change this?
ASSERT enabled only in debug mode is not enough?
There was a problem hiding this comment.
The mmap (and calloc) uses size_t length, and that is 32 bit on 32 bit systems. When max size is 4GByte or more (uint64_t value), it allocates an incorrect amount of memory.
Btw, would it be better to throw error on alloc fails?
There was a problem hiding this comment.
It would be better to inform the unvalid size before actually allocated.
BTW is memory limit differently defined for 32bit and 64bit each?
(I cannot find this in spec)
There was a problem hiding this comment.
It is not defined differently, that is the problem. You can allocate more than 4GByte memory for WebAssembly, and 32 bit system can access maximum of 4GByte only. So I think anything requires more than 4GByte maximum memory should fail with allocation failure. This should affect memory.grow as well.
There was a problem hiding this comment.
I see,
then what about using 64bit length instead of size_t to unify both architectures?
We may need to allocate much more memory for complex wasm apps.
There was a problem hiding this comment.
Oh this is because of mmap and calloc, I see.
For now there is no other choice..
let's think about it more
There was a problem hiding this comment.
WebAssembly has 32 bit pointers and 32 bit offsets by default, so they can only access 8GByte memory. However, with 64 bit offsets (an extension), it can asses more memory.
| @@ -43,7 +43,7 @@ Memory::Memory(uint64_t initialSizeInByte, uint64_t maximumSizeInByte, bool isSh | |||
| , m_targetBuffers(nullptr) | |||
| , m_isShared(isShared) | |||
| { | |||
| ASSERT(initialSizeInByte <= std::numeric_limits<size_t>::max()); | |||
| RELEASE_ASSERT(initialSizeInByte <= std::numeric_limits<size_t>::max()); | |||
There was a problem hiding this comment.
Why did you change this?
ASSERT enabled only in debug mode is not enough?
| @@ -58,7 +58,7 @@ Memory::Memory(uint64_t initialSizeInByte, uint64_t maximumSizeInByte, bool isSh | |||
| #else | |||
| WALRUS_64_MEMORY_INITIAL_MMAP_RESERVED_ADDRESS_SIZE; | |||
| #endif | |||
| m_reservedSizeInByte = std::min(initialReservedSize, m_maximumSizeInByte); | |||
| m_reservedSizeInByte = std::min(std::max(initialReservedSize, initialSizeInByte), m_maximumSizeInByte); | |||
No description provided.