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

ReplaySubject with time and size bounds #1192

Closed
wants to merge 4 commits into from

Conversation

akarnokd
Copy link
Member

I've implemented the bounded ReplaySubject variants and removed the old OperatorReplay.CustomReplaySubject so the various Observable.replay() now use this version.
Note that this is based on the current SubjectSubscriptionManager and not my manager from PR #1185. I've copied the ReplaySubjectConcurrencyTest to check the concurrency properties.

The bounded buffering is implemented via a doubly-linked list where the back reference is a weak reference. This allows keeping a reference to the latest Node for each replayState and once a Node is no longer indexed, it will be removed by GC.

A concurrency review would be much appreciated.

@cloudbees-pull-request-builder

RxJava-pull-requests #1104 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #1105 FAILURE
Looks like there's a problem with this pull request

@benjchristensen
Copy link
Member

Nice, people will be happy to have all the functionality on ReplaySubject

Should we wait until after #1185 to rebase and merge this?

I have not yet reviewed this.

@akarnokd
Copy link
Member Author

I'd review and merge this first and go back to #1185 as it surely needs some rebase anyway.

* The weak reference back to the previous node. If that node is not
* hard-referenced, it will be eventually GCd.
*/
final WeakReference<Node<T>> previous;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unnecessary as noone is walking backwards. A singly-linked list is enough.

@akarnokd
Copy link
Member Author

Closing this as the master merge got messed up.

@akarnokd akarnokd closed this May 19, 2014
@cloudbees-pull-request-builder

RxJava-pull-requests #1126 FAILURE
Looks like there's a problem with this pull request

@akarnokd akarnokd deleted the BoundedReplaySubject512 branch May 20, 2014 16:30
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