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

timer: Improve memory ordering in Inner's increment #2107

Merged
merged 10 commits into from Mar 26, 2020

Commits on Jan 13, 2020

  1. timer: Improve memory ordering in Inner's increment

    This commit improves the memory ordering in the implementation of
    Inner's increment function. The former code did a sequentially
    consistent load of self.num, then entered a loop with a sequentially
    consistent compare and swap on the same, bailing out with and Err only
    if the loaded value was MAX_TIMEOUTS. The use of SeqCst means that all
    threads must observe all relevant memory operations in the same order,
    implying synchronization between all CPUs.
    
    This commit adjusts the implementation in two key ways. First, the
    initial load of self.num is now down with Relaxed ordering. If two
    threads entered this code simultaneously, formerly, tokio required
    that one proceed before the other, negating their parallelism. Now,
    either thread may proceed without coordination. Second, the SeqCst
    compare_and_swap is changed to a Release, Relaxed
    compare_exchange_weak. The first memory ordering referrs to success:
    if the value is swapped the load of that value for comparison will be
    Relaxed and the store will be Release. The second memory ordering
    referrs to failure: if the value is not swapped the load is
    Relaxed. The _weak variant may spuriously fail but will generate
    better code.
    
    These changes mean that it is possible for more loops to be taken per
    call than strictly necessary but with greater parallelism available on
    this operation, improved energy consumption as CPUs don't have to
    coordinate as much.
    blt committed Jan 13, 2020
    Copy the full SHA
    c73a450 View commit details
    Browse the repository at this point in the history
  2. Avoid an additional load, per @udoprog's review

    This commit avoids an additional, unecessary load performed in my last
    commit by storing the result of the compare_exchange_weak. This is
    done at the cost of one additional loop when curr == MAX_TIMEOUTS but,
    considering how expensive loads are, this is the correct thing to do.
    blt committed Jan 13, 2020
    Copy the full SHA
    58d62b7 View commit details
    Browse the repository at this point in the history

Commits on Jan 17, 2020

  1. Introduce a small 'sanity' test

    I'm still learning enough about tokio internals to not be sure
    I'm taking the right approach here. This commit introduces a small
    test to create an Inner instance -- runnable only with loom flagged
    on, but without actually doing loom tests yet -- and assert that
    no time has elapsed for the Inner. This fails like so:
    
    ```
    tokio > RUSTFLAGS="--cfg loom" cargo test time::driver --lib  --features "full" -- --test-threads=1 --nocapture
       Compiling tokio v0.2.9 (/Users/blt/projects/com/github/tokio/tokio)
        Finished test [unoptimized + debuginfo] target(s) in 5.43s
         Running /Users/blt/projects/com/github/tokio/target/debug/deps/tokio-16630467e4eb52a5
    
    running 1 test
    test time::driver::tests::test_inner::sanity ... thread 'main' panicked at 'cannot access a scoped thread local variable without calling `set` first', /Users/blt/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-0.1.2/src/lib.rs:186:9
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
    FAILED
    
    failures:
    
    failures:
        time::driver::tests::test_inner::sanity
    
    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 65 filtered out
    
    error: test failed, to rerun pass '--lib'
    ```
    
    That it does is a pretty fair indication I don't understand something
    vital about tokio internals.
    blt committed Jan 17, 2020
    Copy the full SHA
    677c829 View commit details
    Browse the repository at this point in the history

Commits on Jan 22, 2020

  1. Introduce a loom test for inner increment/decrement changes

    This commit introduces a new test-only function into Inner that
    returns the value of num. This is used to synchronize two concurrent
    loom threads whose sole	goals are to increment and decrement in
    equal measure. If the atomics of this changeset	are correct we'll
    always hit zero	at the end of the model.
    
    Special	care has to be taken not to decrement unless there's
    previously been	an increment. This is the purpose of the
    'synchronization' on Inner's num. Note that we always assert at the
    end of the model with SeqCst ordering but load in the model relaxed.
    blt committed Jan 22, 2020
    Copy the full SHA
    fa33129 View commit details
    Browse the repository at this point in the history
  2. Address unused import build error

    I neglected to check the last commit to see if it would build in a
    non-loom test environment. It did not.
    blt committed Jan 22, 2020
    Copy the full SHA
    d7f07b9 View commit details
    Browse the repository at this point in the history

Commits on Jan 28, 2020

  1. Merge branch 'master' into blt-inner_atomics

    Brian L. Troutwine committed Jan 28, 2020
    Copy the full SHA
    a58614e View commit details
    Browse the repository at this point in the history
  2. Update tokio/src/time/driver/mod.rs

    Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
    Brian L. Troutwine and hawkw committed Jan 28, 2020
    Copy the full SHA
    ee9fddf View commit details
    Browse the repository at this point in the history

Commits on Jan 29, 2020

  1. Unstack test/loom cfg

    blt committed Jan 29, 2020
    Copy the full SHA
    20d01c6 View commit details
    Browse the repository at this point in the history
  2. Copy the full SHA
    5e1fd00 View commit details
    Browse the repository at this point in the history
  3. Collapse test_inner upward

    blt committed Jan 29, 2020
    Copy the full SHA
    21ef90a View commit details
    Browse the repository at this point in the history