Skip to content

[EH] Change Walker::TaskFunc back to function pointer #3899

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

Merged
merged 4 commits into from
May 21, 2021

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented May 20, 2021

Walker::TaskFunc has changed from a function pointer to
std::function in #3494, mainly to make the EH support for CFGWalker
easier. We didn't notice much performance difference then, but it was
recently reported that it creased binaryen.js code size and performance.
This changes Walker::TaskFunc back to a function pointer and does a
little more work to manage catch index in CFGWalker side.

Hopefully fixes #3857.

cc @MaxGraey

`Walker::TaskFunc` has changed from a function pointer to
`std::function` in WebAssembly#3494, mainly to make the EH support for `CFGWalker`
easier. We didn't notice much performance difference then, but it was
recently reported that it creased binaryen.js code size and performance.
This changes `Walker::TaskFunc` back to a function pointer and does a
little more work to manage catch index in `CFGWalker` side.

Hopefully fixes WebAssembly#3857.
@aheejin aheejin requested a review from kripken May 20, 2021 20:19
@aheejin aheejin changed the title [EH] Change Walker::TaskFunc back to function pointer (NFC) [EH] Change Walker::TaskFunc back to function pointer May 21, 2021
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

Nice!

@aheejin aheejin merged commit 24d71aa into WebAssembly:main May 21, 2021
@aheejin aheejin deleted the no_std_function branch May 21, 2021 02:54
@MaxGraey
Copy link
Contributor

@aheejin thanks!

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.

Much slower compilation compare to v98
3 participants