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

Optimize OperatorSkipLastTimed #1065

Merged
merged 1 commit into from
Apr 24, 2014

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Apr 22, 2014

Changed OperatorSkipLastTimed to only cache the latest items in the specified time window.

@cloudbees-pull-request-builder

RxJava-pull-requests #981 SUCCESS
This pull request looks good

}

@Override
public void onError(Throwable e) {
buffer = Collections.emptyList();
subscriber.onError(e);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we need to call emitItemsOutOfWindow in onError. Which one is more reasonable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing buffer now and they all emit buffered data in case of onError.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in Rx.Net.

            var o = Observable.Create<int>(observer => {
                observer.OnNext(2);
                Thread.Sleep(2000);
                observer.OnError(new Exception("test"));
                return Disposable.Empty;
            });
            o.SkipLast(TimeSpan.FromMilliseconds(100)).ObserveOn(Scheduler.NewThread).Subscribe(
                next=>Console.WriteLine(next),
                e => Console.WriteLine(e),
                () => Console.WriteLine("onCompleted")
                );
            Console.ReadLine();

The above codes only output

System.Exception: test

So Rx.Net only emits onError and drops the buffer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When onError occurs it immediately emits and does not work any further work.

We had this discussion a while back when debating delay I think.

Rx Design Guideline 6.6

6.6. OnError messages should have abort semantics

As normal control flow in .NET uses abort semantics for exceptions (the stack is unwound, current code path is interrupted), Rx mimics this behavior. To ensure this behavior, no messages should be sent out by an operator once one of it sources has an error message or an exception is thrown within the operator.

...

In this sample, a buffering operator will abandon the observable sequence as soon as the subscription to source encounters an error. The current buffer is not sent to any subscribers, maintain abort semantics.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for clarification. Then this PR should be ready to merge.

benjchristensen added a commit that referenced this pull request Apr 24, 2014
@benjchristensen benjchristensen merged commit 3904af5 into ReactiveX:master Apr 24, 2014
@zsxwing zsxwing deleted the skip-last-timed branch April 27, 2014 03:08
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.

4 participants