-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
refactor(deno/core): Move buffer_ops to deno core & merge logic with json_ops #9457
Conversation
Used by #9323. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inteon there a lot going on in this PR. In general I think these changes are desirable, but to be honest it's quite hard to approve this PR with confidence as it touches as lot of critical infrastructure. Could you maybe split it into two smaller PRs? First which reorganizes file by moving existing implementation of "minimal ops" to "core/", and second which unifies implementation between "json" and "buffer" ops.
@bartlomieju I'll see what I can do. I plan to work on this tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@inteon looks great, could you rewrite core/examples/http_bench_bin_ops.rs
to use this new dispatch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks you @inteon, very nice cleanup. As with previous PRs I will be monitoring benchmarks after landing and if there's a noticeable impact I will revert to look for bottlenecks
This PR is an effort to allow for more general js <-> rs message passing. It fixes #8173, by moving minimal ops to deno core.
This new system can be used to efficiently send and receive array-buffer messages (like minimal ops), but allso allows to return messages of arbitrary length (can be used for worker communication).