{{ message }}
Repository navigation
Do not allocate MemorySegments for memory.data(i32, i32?) - #2831
Open
CountBleck wants to merge 1 commit into
Open
CountBleck wants to merge 1 commit into
CountBleck wants to merge 1 commit into
Conversation
CountBleck
commented
Mar 25, 2024
Member
Member
Author
Oh, that's...a much better idea. Thanks and sorry about that. |
CountBleck
force-pushed
the
fix-large-memory-crash
branch
from
March 25, 2024 01:49
373166c to
6a89a09
Compare
Member
Author
@dcodeIO hmm, would this conflict when |
CountBleck
marked this pull request as ready for review
March 25, 2024 03:00
HerrCai0907
approved these changes
Mar 28, 2024
CountBleck
force-pushed
the
fix-large-memory-crash
branch
3 times, most recently
from
April 3, 2024 01:46
ed50aef to
4465767
Compare
MemorySegments can now hold null buffers, which signifies that memory is in fact used in the compiled program, for programs that only reserve zero-filled memories. Using null buffers avoids passing zero-filled buffers to Binaryen, which isn't necessary since Wasm memories are initialized with zeroes when a (data ...) segment isn't present. This change is breaking in that modules using imported memories that contain non-zero values in such memory ranges at the time of instantiation will retain those values, as opposed to being filled. This shouldn't affect most users, however, and it could possibly be desirable. Fixes AssemblyScript#2827.
CountBleck
force-pushed
the
fix-large-memory-crash
branch
from
April 3, 2024 01:47
4465767 to
163a631
Compare
HerrCai0907
approved these changes
Apr 3, 2024
Member
Author
|
@HerrCai0907 I changed the commit message; this is technically a breaking change. |
Contributor
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2827.
Changes proposed in this pull request:
⯈ Bump
Compiler#memoryOffsetinstead of allocating zero-filled segments.There is an issue with the
initialPagescalculation ininitDefaultMemory, however. The existing condition to run that calculation depends onmemorySegments.length, which is now zero if onlymemory.data(n: i32)is called, as in the memory-config-errors test.Making that calculation occur regardless of the number of memory segments would cause a separate issue: due to the default memory offsets of
options.memoryBase,1024, or8, modules that previously compiled with an initial memory of 0 pages would compile with a minimum of 1 page.