-
Notifications
You must be signed in to change notification settings - Fork 623
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
Allocate linear memory using mmap #3052
Conversation
ee99a85
to
dc22faa
Compare
product-mini/platforms/linux-sgx/enclave-sample/Enclave/Enclave.config.xml
Outdated
Show resolved
Hide resolved
@loganek thanks a lot for the refactoring, I tried to review the code and added several comments. Since this is a big change which impacts many platforms (per my understanding, it's a breaking change) and hasn't been tested by developers for some time, could we leave it unmerged until WAMR-1.3.2 is released? I am afraid that it may be a little rushed to merge it because we want to release 1.3.2 soon. |
Absolutely, let's not block 1.3.2 and we can pull it for the next release. And thanks a lot for the review, I'll address your comments in the upcoming days. |
ad45e49
to
00c4ba3
Compare
@wenyongh I've replied to all your comments, please let me know if you have any question, and feel free to merge the PR once 1.3.2 is released. |
OK, let's merge it after 1.3.2 and do more tests then. |
7c26320
to
215b7c8
Compare
With this approach we can ommit using memset() for the newly allocated memory therefore the physical pages are not being used unless touched by the program. This also simplifies the implementation.
215b7c8
to
5e9a693
Compare
…)" This reverts commit a27ddec.
With this approach we can omit using memset() for the newly allocated memory therefore the physical pages are not being used unless touched by the program. This also simplifies the implementation.
With this approach we can omit using memset() for the newly allocated memory therefore the physical pages are not being used unless touched by the program.
This also simplifies the implementation. There's a room for further refactoring and unifying aot/wasm runtimes, but we can do it as follow-ups to keep the PR at reasonable size.