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

[WASM ABI 1.0] impl __call_reducer__ using bytes_source_read #1609

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Aug 19, 2024

Description of Changes

This adds _bytes_source_read and implements __call_reducer__ with it.
Meanwhile, _buffer_len and _buffer_consume are removed.

cc #1460

@Centril Centril changed the base branch from master to centril/call-reducer-by-val August 19, 2024 14:47
Comment on lines +465 to +467
if args == BytesSource::INVALID {
return logic(&[]);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @cloutiertyler -- slight tweak to the WASM ABI proposal: the host can/will send __call_reducer__ an invalid args which means that the arguments are empty. This is useful as it allows us to avoid a host call as well as avoid allocation. The primary winner for this I imagine are the connect/disconnect reducers.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we're tweaking the proposal, please make sure changes make it back into the proposal.

Base automatically changed from centril/call-reducer-by-val to master August 20, 2024 07:26
@Centril Centril enabled auto-merge August 20, 2024 09:21
@Centril Centril added this pull request to the merge queue Aug 20, 2024
Merged via the queue into master with commit 9ea5a2f Aug 20, 2024
7 checks passed
@Centril Centril deleted the centril/bytes-source-read branch August 20, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants