Skip to content
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

Lower requested WASM memory for iOS user agents #26

Merged
merged 2 commits into from
May 30, 2023

Conversation

MartinMinkov
Copy link
Contributor

@MartinMinkov MartinMinkov commented May 29, 2023

Description

🔗 Related Issue: o1-labs/o1js#823

This pull request addresses an issue where SnarkyJS applications encounter an Out of Memory error on iOS devices, specifically iPhones and iPads, while trying to initialize the WASM memory for plonk_wasm.js. This problem seems to be unique to iOS devices as desktop Safari doesn't appear to be affected.

The solution involves modifying the maximum requested WASM memory for iOS user agents. Through testing, it's been found that these devices can handle up to 1GiB of requested memory. Attempting to request 2GB results in the same memory error. However, for all other user agents, the memory request remains unchanged, aiming for the maximum available, which is 4GiB.

…ser agent to improve performance on iOS Safari devices
Copy link
Member

@shimkiv shimkiv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular issue was solved 🎉
Yet, as it was expected it revealed others, starting with inability to compile contract (which causes page reload for some reason and losing all the logs). Probably also related to memory.

Copy link
Member

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one comment below! I don't think this needed a change of the compiled bindings, btw, since it's only changing the JS side.

@MartinMinkov Yeah, a snarkyjs PR should just check out the submodule at the last commit of this PR. Ideally they are merged at the same time

js/web/web-backend.js Outdated Show resolved Hide resolved
Copy link
Member

@Trivo25 Trivo25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I think we should add this to the changelog when we get to it (dont need a seperate PR, just do it whenever)

…memory for iOS devices, as it's not necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants