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

New Asyncify #35

Open
wants to merge 2 commits into
base: wasm
Choose a base branch
from
Open

New Asyncify #35

wants to merge 2 commits into from

Conversation

rhysd
Copy link
Owner

@rhysd rhysd commented Aug 6, 2019

This branch is an experiment to use new Asyncify for vim.wasm's event loop.

Implementation Status

  • Replace SharedArrayBuffer and Atomics with simple postMessage
  • Use Asyncify.handleSleep to wait for messages from main thread in blocking form in C
  • Remove browser compatibility check since SharedArrayBuffer and Atomics are no longer used
  • Tweak build configuration
    • Stack size: 20480 seems enough
  • Check browser compatibility
    • Chrome
    • Firefox
    • Safari

Current Issues

  • Does not work on Safari 12.1.2
  • Vim utilizes indirect function calls for running editor commands so SYNCIFY_IGNORE_INDIRECT does not work. Blacklisting may work but the asynchronous functions are called in deep part of call stack so list would be big and hard to maintain
  • Performance seems degraded. Especially CPU usage at initialization is quite high

Resources

@rhysd rhysd self-assigned this Aug 6, 2019
@rhysd
Copy link
Owner Author

rhysd commented Aug 6, 2019

Regarding to Safari issue, warning that running dependencies is taking time is output repeatedly at several seconds interval on starting Vim. Safari continues to allocate memory and finally macOS crashes when it runs out of memory.

スクリーンショット 2019-08-07 1 39 57

@kripken
Copy link

kripken commented Aug 7, 2019

Nice work :)

About Safari, I think a build of this is enough for a bug report for WebKit. It sounds like their JIT is hitting a problem, probably due to the extra overhead of asyncify (the many more locals, which can lead to worse than linear time and memory).

May be worth trying the asyncify whitelist/blacklist feature, btw, it may help with performance.

@rhysd
Copy link
Owner Author

rhysd commented Aug 9, 2019

@kripken

Thanks for the comment.

About Safari, I think a build of this is enough for a bug report for WebKit. It sounds like their JIT is hitting a problem, probably due to the extra overhead of asyncify (the many more locals, which can lead to worse than linear time and memory).

Yeah, I agree that it would be bug of WebKit. In order to clarify it is a bug of WebKit, not Safari, I tried WebKit nightly build r248339. I confirmed it consumed 2GB memory and it finally caused Maximum call stack size exceeded error.

May be worth trying the asyncify whitelist/blacklist feature, btw, it may help with performance.

Yes, I should try :)
My understanding is that when calling function which may unwind stack, all functions on call stack should be instrumented. Since a function to wait for user's input with blocking is called at the bottom of a bit deep call stack, the list would be big. Since vim.wasm must follow the upstream changes, I need to investigate if the list is maintenable.

@ashwinkjoseph
Copy link

ashwinkjoseph commented Aug 28, 2020

hey, is this PR still active? is anyone working on it or do you need a hand in finishing this?
I would like to use this project for an educational website and I would like it to have support for firefox and safari as well. So if possible, I can help you out with this PR

@rhysd
Copy link
Owner Author

rhysd commented Aug 28, 2020

Actually not active. This PR is for experiment and not meant to be merged eventually. The second point of 'Current Issues' in description is hard in terms of maintenance.

@ashwinkjoseph
Copy link

Actually not active. This PR is for experiment and not meant to be merged eventually. The second point of 'Current Issues' in description is hard in terms of maintenance.

Okay, thanks for letting me know.
Let me know if I can help you with this project in any way.

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.

3 participants