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

Design feedback request: IWaitStrategy refactoring #75

Open
ocoanet opened this issue Jul 3, 2024 · 0 comments
Open

Design feedback request: IWaitStrategy refactoring #75

ocoanet opened this issue Jul 3, 2024 · 0 comments

Comments

@ocoanet
Copy link
Collaborator

ocoanet commented Jul 3, 2024

I am considering refactoring IWaitStrategy. It would be a breaking change, so I am exposing the design here to get feedback before moving forward.

Current API:

public interface IWaitStrategy
{
    SequenceWaitResult WaitFor(long sequence, DependentSequenceGroup dependentSequences, CancellationToken cancellationToken);
    void SignalAllWhenBlocking();
}

New API:

public interface IWaitStrategy
{
    ISequenceWaiter GetSequenceWaiter(IEventHandler eventHandler);
    void SignalAllWhenBlocking();
}

// NEW: interface used by sequence barriers to wait for dependent sequences
public interface ISequenceWaiter
{
    SequenceWaitResult WaitFor(long sequence, DependentSequenceGroup dependentSequences, CancellationToken cancellationToken);
    void Abort();
}

// NEW: marker interface for event handler types (IEventHandler<T>, IValueEventHandler<T>, ...)
public interface IEventHandler
{
}

A similar change would be applied to the IAsyncWaitStrategy interface.

The new design implies that dedicated sequence waiters would be created per event handler, which also implies that sequence barriers would be created per event handler. I do not think it would be an issue. Sequence barriers are already created per event handler for async handlers, and I think that sharing sequence barriers is not required (there have been an open issue for two years in the LMAX Disruptor with this exact question).

The goal of the change is to support creating stateful waiters, which would be valuable for:

  • adding a clean way to support per handler logic in the wait strategies.
  • removing allocations in AsyncWaitStrategy by using preallocated IValueTaskSource instances (*).
  • opening new wait strategy implementation options.

(*): I experimented implementing no-allocation async wait strategies in a branch. It requires adding per handler state in wait strategies. It is possible to pass custom state to the wait strategy in WaitFor, but it is not a clean design (because the wait strategy state is leaking into the sequence barrier), and it is a breaking change of the IAsyncWaitStrategy anyway.

Notes:

  • It might be possible to indirectly add per handler state in wait strategies by adding a mutable property WaitStrategyState in DependentSequenceGroup. However, it would feel somewhat hacky and would force DependentSequenceGroup to be instanciated per handler.
  • It might be possible to avoid adding the IEventHandler interface and instead add GetSequenceWaiter overloads for all event handler types.

What do you think?

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

No branches or pull requests

1 participant