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

4.x: Make TailRecursiveSink lock-free and have less allocations #499

Merged
merged 1 commit into from
May 26, 2018

Conversation

akarnokd
Copy link
Collaborator

@akarnokd akarnokd commented May 9, 2018

This PR changes TailRecursiveSink to a lock-free algorithm as well as reduces the allocations in the class by inlining various IDisposable fields.

The aim of the class is to make sure OnComplete or OnError in Concat, Catch and various other operators don't trigger an infinite recursion but stay on the level.

This could have been solved with a straightforward trampoline like this Concat implementation, however, there is a small complication. The recursion may be not only on the termination side, but on the existence of the inner IObservables.

For example, to avoid yet another level of recursion due to graphs like Concat(Concat(a, b), Concat(c, d)), the Extract method can get another level of IEnumerable<IObservable<TSource>> on the current level, and the concatenation should resume on the inner Concat.

The Stack is there to allow returning to an outer enumeration and resume it. I don't know what the length stack was supposed to do; the algorithm doesn't have to know how long each IEnumerator is. Was it some kind of optimization for Lists and arrays avoiding MoveNext() == false and thus IEnumerator.Dispose()? The former should be as costly as an index comparison and the latter should be no-op anyway.

@clairernovotny clairernovotny merged commit 10a44ad into dotnet:master May 26, 2018
@akarnokd akarnokd deleted the TailRecursiveSinkOpt branch May 26, 2018 14:54
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

Successfully merging this pull request may close these issues.

2 participants