Skip to content

2.x: first step switching to the reduced-allocation architecture #4030

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

Merged
merged 1 commit into from
Jun 19, 2016

Conversation

akarnokd
Copy link
Member

This is the first step towards the new architecture where operator implementations are themselves of the base type instead of a 2-3 layer indirection as in 1.x.

This PR removes the instance field onSubscribe from the base reactive classes and introduces subscribeActual to be overridden with the custom operator logic now on. Existing operators are still working through the classical OnSubscribe/Lift indirection and will be updated in subsequent PRs.

Naming and existence of subscribeActual is up for (post-merge) discussion. To recap, the method is needed so that the default subscribe method can be made final and ensure plugin hooks can be called (once available). The method is protected and only affects operator implementors that chose to extend the base reactive classes directly.

The alternative is to leave subscribe abstract and add a hook to every operator method (for example, see this and this.

@akarnokd akarnokd added this to the 2.0 RC 1 milestone Jun 18, 2016
@@ -33,6 +33,7 @@
/** The cache and replay state. */
private CacheState<T> state;

private AtomicBoolean once;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: final

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Updated.

@JakeWharton
Copy link
Contributor

lgtm 👍

@akarnokd akarnokd force-pushed the ArchitectureChange branch from ff57e69 to eb0c28e Compare June 19, 2016 06:18
@akarnokd akarnokd merged commit e222162 into ReactiveX:2.x Jun 19, 2016
@akarnokd akarnokd deleted the ArchitectureChange branch June 19, 2016 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants