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

Broadcast channel capacity is not well-documented #6015

Closed
mdacach opened this issue Sep 18, 2023 · 3 comments · Fixed by #6042
Closed

Broadcast channel capacity is not well-documented #6015

mdacach opened this issue Sep 18, 2023 · 3 comments · Fixed by #6042
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-sync Module: tokio/sync T-docs Topic: documentation

Comments

@mdacach
Copy link

mdacach commented Sep 18, 2023

Description
tokio::sync::broadcast documentation states that the capacity parameter is an upper bound on the number of values the channel may retain at any given time.
But when creating the channel, this capacity is actually rounded up to the nearest power of two - thus the constraint that capacity <= usize::MAX / 2 on channel()

Solution
I guess we want to either

  1. commit to rounding up to the next power of two and update the docs to state that, or
  2. have the docs clarify that the actual channel capacity is >= than capacity, (but then the constraint of capacity <= usize::MAX / 2 is not clear)

For context, I came across this messing around with the example code:
image
image

@mdacach mdacach added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Sep 18, 2023
@mdacach
Copy link
Author

mdacach commented Sep 18, 2023

Follow-up question: why round up the capacity at all?

update: I now see that a bitmask is used. We probably want to commit to the power of two, then. I will open up a PR with suggested writing tomorrow.

@hawkw
Copy link
Member

hawkw commented Sep 19, 2023

I think it would be preferable to change the docs to state that the channel's capacity will be at least capacity, rather than publicly committing to power of two capacities. The bitmask is an internal implementation detail, and if it's possible to document the behavior clearly in a way that does not commit to maintaining that internal implementation detail forever, that's probably better.

@hawkw
Copy link
Member

hawkw commented Sep 19, 2023

For comparison, the documentation for Vec::with_capacity, which also rounds to the next power of two, states:

The vector will be able to hold at least capacity elements without reallocating. This method is allowed to allocate for more elements than capacity. If capacity is 0, the vector will not allocate.

...

If it is important to know the exact allocated capacity of a Vec, always use the capacity method after construction.

The broadcast channel docs could probably use similar language.

@Darksonn Darksonn added the M-sync Module: tokio/sync label Sep 19, 2023
@Darksonn Darksonn added the T-docs Topic: documentation label Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-sync Module: tokio/sync T-docs Topic: documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants