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

The recv_buffer implementation should be simplified #4818

Open
guhetier opened this issue Feb 14, 2025 · 0 comments
Open

The recv_buffer implementation should be simplified #4818

guhetier opened this issue Feb 14, 2025 · 0 comments
Assignees
Labels
Area: Core Related to the shared, core protocol logic

Comments

@guhetier
Copy link
Contributor

The recv_buffer implementation supports multiple mode, and the number of diverging code paths based on the mode has been increasing.
Some refactoring could consolidate these path, helping with the maintainability of the code.

Potential improvements

  1. Mark "retired" chunks clearly and/or move them out of the chunk list

    • "retired" chunks are the ones that won't be used for any read/write operation anymore and will be freed as soon as they are no longer externally referenced
    • currently, retired buffer staying in the list means that "single" and "circular" mode look at the last chunk for read/write while other modes start by looking at the first chunk
  2. Always treat the first chunk as circular with limited capacity

    • "ReadStart" and "Capacity" are enough to ensure operations won't loop around for modes that don't want it
    • This should allow to consolidate most paths in "Read" and "Write" operations
  3. Refactor the drain logic

    • "Partial drain" and "Full drain" evolved to have different meaning for different buffer modes, making it tricky to modify
  4. The preallocated chunk pointer could be moved to the stream

    • It is allocated and freed by the stream but stored in the recv_buffer, leading to a confusing deinitialization pattern: the recv_buffer must be accessed after being deinitialized to free the chunk
@guhetier guhetier self-assigned this Feb 14, 2025
@guhetier guhetier added the Area: Core Related to the shared, core protocol logic label Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Core Related to the shared, core protocol logic
Projects
Status: No status
Development

No branches or pull requests

1 participant