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

Avoid writing length on each IO for async channel #62

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

vstakhov
Copy link
Contributor

This is a simple fine tune for a metered channel to update channel length merely when we need a readout.

There are two other small changes:

  • Implement len method for the futures_channel feature flag
  • Fix inconsistency between documentation and the actual value for the TOF queue size

@vstakhov vstakhov added the enhancement New feature or request label Sep 21, 2023
@drahnr
Copy link
Collaborator

drahnr commented Sep 21, 2023

I think once the root cause for some stalls are resolved, we should get rid of the two underlying implementations, and consolidate to one. Not a fan of the complexity added by sprinkled cfg(feature=..).

@vstakhov
Copy link
Contributor Author

I have a gut feeling that we have deadlocks just because we rely somehow on a wild behaviour of the futures_channel. In fact, all the time we do clone for a Sender we have a free sending slot, so if we do clone+send it is guaranteed to be successful even if a queue is full. I personally would also prefer to get rid of code duplication and to use async_channel only (like Substrate does so far, for example).

@drahnr
Copy link
Collaborator

drahnr commented Sep 21, 2023

I am all in favor of consolidating to async_channel or whatever channel crate gets the job done without much headache.

@vstakhov vstakhov merged commit a5a9330 into master Sep 25, 2023
6 checks passed
@drahnr drahnr deleted the vstakhov-meter-tune branch September 29, 2023 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants