Skip to content

1.x: compact MultipleAssignment- and Serial-Subscriptions #4328

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 2 commits into from
Aug 11, 2016

Conversation

akarnokd
Copy link
Member

This PR reduces the allocation in MultipleAssignmentSubscription and SerialSubscription by introducing a shared, compact underlying container SequentialSubscription.

I've updated Worker.schedulePeriodically to use it directly.

In addition, there is a behavior change with MultipleAssignmentSubscription: it no longer retains the last Subscription as it was likely to cause retention problems (and otherwise didn't make sense to me).

The SequentialSubscription feature weak versions of the mutation methods that don't retry if there was a concurrent mutation: for some operations, not winning such races is fine.

/cc @JakeWharton @davidmoten @artem-zinnatullin

@akarnokd akarnokd added this to the 1.2 milestone Aug 10, 2016
* A container of a Subscription that supports operations of SerialSubscription
* and MultipleAssignmentSubscription via methods (update, replace) and extends
* AtomicReference to reduce allocation count (beware the API leak of AtomicReference!).
* @since 1.1.9
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: do we really need this for internal classes?

@JakeWharton
Copy link
Contributor

👍

@codecov-io
Copy link

Current coverage is 84.38% (diff: 56.57%)

Merging #4328 into 1.x will decrease coverage by 0.08%

@@                1.x      #4328   diff @@
==========================================
  Files           268        270     +2   
  Lines         17478      17500    +22   
  Methods           0          0          
  Messages          0          0          
  Branches       2664       2677    +13   
==========================================
+ Hits          14764      14768     +4   
- Misses         1853       1876    +23   
+ Partials        861        856     -5   

Powered by Codecov. Last update b3a0446...cccf0ad

@artem-zinnatullin
Copy link
Contributor

👍

@akarnokd akarnokd merged commit cd0301b into ReactiveX:1.x Aug 11, 2016
@akarnokd akarnokd deleted the SubscriptionLowAlloc branch August 11, 2016 11:34
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.

4 participants