Pinned Channels

[futursolo]

Description

This pull request introduces a channel implementation that is !Send.

mpsc channel, 1,000,000 channels, send 10 items each, same task:

Futures Channel: 294ms
Tokio Channel:   340ms
Yew Channel:     120ms

mpsc channel, 1,000,000 channels, send 10 items each, different task:

Futures Channel: 360ms
Tokio Channel:   389ms
Yew Channel:     192ms

oneshot channel, send 10,000,000 items, same task:

Futures Oneshot: 380ms
Tokio Oneshot:   253ms
Yew Oneshot:     133ms

oneshot channel, send 10,000,000 items, different task:
(note: cost of task spawning is substantially more expensive than sending through oneshot channel itself.)

Futures Oneshot: 966ms
Tokio Oneshot:   1065ms
Yew Oneshot:     893ms

Checklist

• I have reviewed my own code
• I have added tests

[github-actions]

Visit the preview URL for this PR (updated for commit 1e73fb4):

https://yew-rs-api--pr2811-pinned-channels-a6yq3teq.web.app

_{(expires Mon, 22 Aug 2022 18:04:28 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

[github-actions]

Benchmark - SSR

Yew Master

Benchmark	Round	Min (ms)	Max (ms)	Mean (ms)	Standard Deviation
Baseline	10	434.703	476.157	446.788	13.359
Hello World	10	1242.081	1394.976	1301.532	59.349
Function Router	10	3262.743	3508.886	3345.685	90.151
Concurrent Task	10	1013.074	1016.943	1014.402	1.505

Pull Request

Benchmark	Round	Min (ms)	Max (ms)	Mean (ms)	Standard Deviation
Baseline	10	435.967	481.270	452.306	16.520
Hello World	10	1202.872	1388.751	1253.956	57.993
Function Router	10	3144.109	3425.877	3266.969	94.842
Concurrent Task	10	1012.602	1016.692	1014.323	1.185

[github-actions]

Size Comparison

examples	master (KB)	pull request (KB)	diff (KB)	diff (%)
boids	169.301	169.310	+0.009	+0.005%
contexts	107.364	107.358	-0.006	-0.005%
counter	84.875	84.875	0	0.000%
counter_functional	85.499	85.500	+0.001	+0.001%
dyn_create_destroy_apps	87.800	87.802	+0.002	+0.002%
file_upload	101.396	101.396	0	0.000%
function_memory_game	162.893	162.881	-0.012	-0.007%
function_router	345.784	345.782	-0.002	-0.001%
function_todomvc	157.918	157.918	0	0.000%
futures	221.961	221.960	-0.001	-0.000%
game_of_life	105.171	105.171	0	0.000%
immutable	180.710	180.712	+0.002	+0.001%
inner_html	81.977	81.976	-0.001	-0.001%
js_callback	110.684	110.687	+0.003	+0.003%
keyed_list	193.473	193.474	+0.001	+0.001%
mount_point	84.660	84.658	-0.002	-0.002%
nested_list	112.274	112.273	-0.001	-0.001%
node_refs	91.818	91.816	-0.002	-0.002%
password_strength	1548.209	1548.209	0	0.000%
portals	95.398	95.398	0	0.000%
router	315.245	315.250	+0.005	+0.002%
simple_ssr	151.344	151.344	0	0.000%
ssr_router	391.298	391.300	+0.002	+0.000%
suspense	108.418	108.417	-0.001	-0.001%
timer	87.644	87.646	+0.002	+0.002%
todomvc	139.266	139.266	0	0.000%
two_apps	85.521	85.521	0	0.000%
web_worker_fib	150.503	150.510	+0.007	+0.005%
webgl	85.656	85.654	-0.002	-0.002%

✅ None of the examples has changed their size significantly.

[futursolo]

With #2824, SSR no longer uses mpsc channels. I have updated this pull request to remove the BufWriter implementation.
It's still beneficial to land this pull request as the pinned oneshot channels can still be used in SSR and mpsc channels are widely used in #2773.

[hamza1311]

Sorry it took me a while to get to this. All the comments are about the same thing: missing safety comment for unsafe. It's very important to have comments explaining why de-referencing the pointers are important here. Those who aren't familiar with the code can have a hard time understanding what's happening, or worse contribute/merge a change that is unsound. That could be someone new, me or even (future) you.

We should have #[deny(clippy::missing_safety_doc)] so unsafe functions are checked for this too. It would be great to run miri in CI. These 2 can be done in separate PRs though.

[WorldSEnder]

I believe the unsafe blocks are now okay, though a few unsafes can be dropped and the phrasing in the SAFETY comments could be improved to:

SAFETY: the ref is not aliased inside this function and dropped before returning. It will be the only ref to the contents.

Being pedantic, a very odd waker implementation could trigger re-entrant code and an aliasing mutable borrow, I believe. The borrow is still active after calling out to arbitrary code a8ee1c0#diff-31d23a5f32d50a4ab7bdff4a7fdc162fc1eaf6597c349f407c4076d83b35377cR54 I'm not really concerned about that, though. I believe it's fine to leave as is.

unsafe now okay

[hamza1311]

Would it be better off if this were a separate library so non-yew users can also use it?

[hamza1311]

This can call try_next and map the Result<Option<_>, _> instead duplicating it's implementation?

[futursolo]

We cannot avoid acquiring the mutable reference to inner as we have to store the waker.

(it’s an equal number of lines to map Result<Option<_>, _> to Poll::* and current implementation.)

[hamza1311]

I was hoping to avoid unsafely obtaining the mutable reference. The lines of code doesn't really matter here

[hamza1311]

What about freeing the inner? Does UnsafeCell takes care of it? I couldn't find anything about drop behavior in UnsafeCell documentation, nor does it implement Drop

[futursolo]

UnsafeCell is a plain struct with an inner of type T. The drop behaviour is handled normally as any other type.

[hamza1311]

Can we run these tests on both targets?

[hamza1311]

Why have these markers?

[futursolo]

This is to mark that there is a hidden information in this struct.

Some other struct have this as we need to expand and add error reason in the future. This is here to keep consistency.

[futursolo]

Would it be better off if this were a separate library so non-yew users can also use it?

I feel yew::platform should be in its own crate. However, this can be done in a future pull request.

[hamza1311]

Looks good.

I agree that yew::platform should be it's own crate. The channels in this PR can be another crate that yew-platform depends upon