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

CompositeSubscription with atomic field updater #1236

Merged

Conversation

akarnokd
Copy link
Member

This PR is aimed at #1204 to reduce unnecessary memory overhead in frequently used classes. This change in CompositeSubscription saves about 24 bytes per instance.

In addition, the field updaters may be faster by about 8-15% for small adds and removals. I've changed the implementation of unsubscribe to use getAndSet which leverages platform intrinsics and is usually compiled to a single instruction: this makes unsubscribing a tiny bit faster and thus reducing latency in benchmarks that only pass on single element around.

@cloudbees-pull-request-builder

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

if (oldState.isUnsubscribed) {
return;
}
// intrinsics may make this a single instruction and may prevent concurrent add/remove faster
Copy link
Member

Choose a reason for hiding this comment

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

For my education can you explain what you mean by this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

On x86 and with Java 8, getAndSet is compiled to LOCK XCHG instead of looping and trying to CAS in the new value. This may be true for other platforms. Since now the swap-in of the terminal state is fully atomic, concurrently running add or remove loops will stop faster. Previously, since everybody looped, it was more likely an add succeded before unsubscribe did. This might not be of concern just I wanted to explain the reason behind the use of getAndSet here.

benjchristensen added a commit that referenced this pull request May 21, 2014
CompositeSubscription with atomic field updater
@benjchristensen benjchristensen merged commit 4ae5333 into ReactiveX:master May 21, 2014
@akarnokd akarnokd deleted the CompositeSubscriptionMemory521 branch May 21, 2014 17:16
@benjchristensen benjchristensen mentioned this pull request Jun 1, 2014
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

3 participants