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

Add proxy_wasm::main macro. #141

Merged
merged 6 commits into from Apr 7, 2022
Merged

Conversation

PiotrSikora
Copy link
Contributor

This is primarly a mechanism to deliver a workaround for a breaking
change in Rust v1.56.0 (which updated LLVM to v13.0), which changed
existing reactors into multi-entry commands, leading to performance
degradation and/or unusable Proxy-Wasm plugins.

See: WebAssembly/WASI#471

This is delivered as a macro to make it somehow compatible with the
unstable "-Z wasi-exec-model=reactor" feature.

Signed-off-by: Piotr Sikora piotrsikora@google.com

This is primarly a mechanism to deliver a workaround for a breaking
change in Rust v1.56.0 (which updated LLVM to v13.0), which changed
existing reactors into multi-entry commands, leading to performance
degradation and/or unusable Proxy-Wasm plugins.

See: WebAssembly/WASI#471

This is delivered as a macro to make it somehow compatible with the
unstable "-Z wasi-exec-model=reactor" feature.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora marked this pull request as ready for review April 4, 2022 23:29
Copy link

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

intereseting

@PiotrSikora PiotrSikora added this to the v0.2.0 milestone Apr 5, 2022
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
.github/workflows/rust.yml Outdated Show resolved Hide resolved

- Fixed performance degradation with `wasm32-wasi` target in Rust v1.56.0
or newer by adding `proxy_wasm::main` macro that should be used instead
of custom `_start`, `_initialize` and/or `main` exports.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Swaagie @mathetake updated text to make it more clear that it's a fix and not simply a new feature. PTAL.

Copy link
Contributor

@Swaagie Swaagie left a comment

Choose a reason for hiding this comment

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

LGTM

@PiotrSikora PiotrSikora merged commit 4a71cb1 into proxy-wasm:master Apr 7, 2022
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

3 participants