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

Fix send buffer misalignment [No 2] #1332

Merged

Commits on Oct 2, 2024

  1. Enforce active work to be a power of two

    This is the second attempt to mitigate send buffer misalignment. Previous one (private-attribution#1307) didn't handle all the edge cases and was abandoned in favour of this PR.
    
    What I believe makes this change work is the new requirement for active work to be a power of two. With this constraint, it is much easier to align the read size with it. Given that `total_capacity = active * record_size`, we can represent `read_size` as a multiple of `record_size` too:
    `read_size = X * record_size`. If X is a power of two and active_work is a power of two, then they will always be aligned with each other.
    
    For example, if active work is 16, read size is 10 bytes and record size is 3 bytes, then:
    
    ```
    total_capacity = 16*3
    read_size = X*3 (close to 10)
    X = 2 (power of two that satisfies the requirement)
    ```
    
    when picking up the read size, we are rounding down to avoid buffer overflows. In the example above, setting X=3 would make it closer to the desired read size, but it is greater than 10, so we pick 2 instead.
    akoshelev committed Oct 2, 2024
    Configuration menu
    Copy the full SHA
    e259e5b View commit details
    Browse the repository at this point in the history
  2. Add a type that enforces power of two constraint

    While working on changing the gateway and parameters, I ran into
    several issues where the power of two constraint was not enforced
    and breakages were hard to find.
    
    A better model for me is to gate the active work at the config level,
    prohibiting invalid constructions at the caller side.
    akoshelev committed Oct 2, 2024
    Configuration menu
    Copy the full SHA
    d9a29f3 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    46087f9 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    831986c View commit details
    Browse the repository at this point in the history
  5. Fix oneshot bench

    akoshelev committed Oct 2, 2024
    Configuration menu
    Copy the full SHA
    27f65d0 View commit details
    Browse the repository at this point in the history

Commits on Oct 4, 2024

  1. Merge from main

    akoshelev committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    32bf1bb View commit details
    Browse the repository at this point in the history
  2. Fix shuttle tests

    akoshelev committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    52fd077 View commit details
    Browse the repository at this point in the history
  3. Fix flaky send_recv_randomized test

    When using more than one thread, this test was failing if futures were scheduled out of order, because `Notify` couldn't wake up futures scheduled after `notify_all` call.
    
    Using barriers solves the issue
    akoshelev committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    279c306 View commit details
    Browse the repository at this point in the history
  4. Change how we compute the previous power of two.

    Using bitshift turns out to be much easier to understand
    akoshelev committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    5e22824 View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    4aeb686 View commit details
    Browse the repository at this point in the history
  6. Don't run send config tests under Shuttle

    There is no reason to do that.
    akoshelev committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    ac31cbb View commit details
    Browse the repository at this point in the history
  7. Don't run non zero power of two tests under Shuttle

    There is no reason to do that.
    akoshelev committed Oct 4, 2024
    Configuration menu
    Copy the full SHA
    34838fe View commit details
    Browse the repository at this point in the history