Skip to content

MeteredExecutor: is seq_cst on the modifyState CAS required, or would relaxed suffice? #2656

Description

@vijay-nagarajan

In folly/executors/MeteredExecutor-inl.h at L66–L70:

} while (!state_.compare_exchange_strong(
    oldState, newState,
    std::memory_order_seq_cst,    // success
    std::memory_order_relaxed));  // failure

The success ordering seq_cst looks stronger than what is required. modifyState has exactly three callers in the Folly tree:

- add() at L83
- resume() at L112
- worker() at L152

In all three sites, every atomic access related to state_ is on the same address (the packed state_ itself; pause() uses fetch_or(kPausedBit, relaxed) on the same word). Cross-address publication is handled by other primitives:

So the state_ CAS appears to be purely a counter update with no cross-address publication role. Single-location coherence + RMW atomicity already linearize the three callers' updates and guarantee no lost increments/decrements.


Is the seq_cst required  for a reason I'm missing? 

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions