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

Improve documentation of internals of bounded implementation #63

Open
fogti opened this issue Mar 30, 2024 · 3 comments
Open

Improve documentation of internals of bounded implementation #63

fogti opened this issue Mar 30, 2024 · 3 comments

Comments

@fogti
Copy link
Member

fogti commented Mar 30, 2024

see also #58 (comment) and a few related comments.

The logic in large parts of the bounded implementation is somewhat non-obvious, and partially stems from other code bases, so it might be a good idea to write down how it works, and that it works correctly, or find a document that already properly does that and link to it. (ad the comments that already exist partially duplicate each other, not necessarily adding to understanding of how all the components interact)

@notgull
Copy link
Member

notgull commented Mar 30, 2024

This post by stjepang might be helpful. I'm relatively sure that the core algorithm hasn't changed much since then.

cc smol-rs/smol#302

@fogti
Copy link
Member Author

fogti commented Mar 31, 2024

I'm aware of that document (at least, I've read it in the past), but it doesn't really document how the book-keeping lines up (e.g. slot, tail, lap), and how/when these are incremented, what the intended flow is (in a blocking case, and a "channel filled enough that no blocking needs to happen" case), what edge cases might exist.

  • The docstring of one_lap sounds like one_lap is actually a constant, and indeed, it is not modified once a bounded channel is created, and it directly depends upon another instance variable mark_bit, so why is it even stored at all? (perf reasons?)
  • one_lap == mark_bit << 1, basically (and prevented from overflowing in release mode by the allocation above, which would fail)

@notgull
Copy link
Member

notgull commented Mar 31, 2024

@taiki-e might be more familiar with the implementation of these channels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants