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

Operator Window and other changes #1138

Merged
merged 2 commits into from
May 5, 2014

Conversation

akarnokd
Copy link
Member

Operator Window with no-first-loss functionality.

Issue #1060

(I had to leave my computer so the detailed description will come within a few hours.)

@cloudbees-pull-request-builder

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

@akarnokd akarnokd mentioned this pull request Apr 30, 2014
@akarnokd
Copy link
Member Author

Changes in respect to OperatorWindow:

  • Theoriginal buffered all events and emitted them at the end of windows as a single hot Observable. This version, similar to groupBy, uses a subject that buffers values until a client subscribes to a window, replays the buffered content, then relays the rest of the events as they appear from the source. This behavior allows code expecting the old buffer-all mode to continue functioning, but such unrevokable behavior might not be desired in the library.
  • Fix for OperatorBufferWithTime: unsubscribe on OnCompleted to stop the associated periodic task.
  • The rewritten exact operators (i.e., count == skip, timespan == timeshift) make extra efforts to ensure values end up in a window even if a window change happens concurrently. A sideeffect is that emission may hop threads, but are still ensured to apear serially.
  • Expanded BufferUntilSubscriber to behave as a regular ReplaySubject for the second, etc. Subscribers instead of either failing with ClassCastException or stealing/overtaking the first client's events.

@benjchristensen
Copy link
Member

BufferUntilSubscriber should not support multicasting, we're just recreating the "time gap" or memory leak problems it was intended to solve.

If it behaves as a PublishSubject then subsequent subscribers will be missing events.
If it behaves as a ReplaySubject then it's a memory leak.

If someone wants to multicast they need to ask for it and deal with ConnectableObservable.

@akarnokd
Copy link
Member Author

I won't be able to change this PR until next monday. If the other behaviors of the new windows are okay, I'd suggest a partial merge leaving my BufferUntilSubscriber out and fixing its ClassCastException and stealing/overtaking behavior in a separate PR.

@benjchristensen
Copy link
Member

suggest a partial merge leaving my BufferUntilSubscriber out and fixing its ClassCastException and stealing/overtaking behavior in a separate PR.

Agreed. I can fix the ClassCastException and just make it onError if someone tries to subscribe twice.

@DavidMGross
Copy link
Collaborator

Under what circumstances (if any) is buffer/window supposed to emit an
empty buffer/Observable -- in other words, if nothing is emitted by the
source Observable during a span of time when any emissions would be
allocated to a new buffer/Observable, will buffer/window emit nothing or
will it emit an empty buffer/Observable? I made some guesss when I came up
with my marble diagrams, but I'm not sure if those guesses still hold.

https://github.com/Netflix/RxJava/wiki/Transforming-Observables#buffer
https://github.com/Netflix/RxJava/wiki/Transforming-Observables#window

On Wed, Apr 30, 2014 at 9:37 AM, Ben Christensen
notifications@github.comwrote:

suggest a partial merge leaving my BufferUntilSubscriber out and fixing
its ClassCastException and stealing/overtaking behavior in a separate PR.

Agreed. I can fix the ClassCastException and just make it onError if
someone tries to subscribe twice.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1138#issuecomment-41818932
.

David M. Gross
PLP Consulting

@akarnokd
Copy link
Member Author

Every buffer and window operator will emit empty lists and observables, except the buffer with size == skip variant. I kept this oddity as there was a test case for it.

@cloudbees-pull-request-builder

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

@akarnokd akarnokd mentioned this pull request May 5, 2014
57 tasks
benjchristensen added a commit that referenced this pull request May 5, 2014
Operator Window and other changes
@benjchristensen benjchristensen merged commit b794705 into ReactiveX:master May 5, 2014
@akarnokd akarnokd deleted the OperatorWindow430 branch May 6, 2014 13:35
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.

None yet

4 participants