Skip to content

Commit 48e4db0

Browse files
benjchristensenbenjchristensen
committed
Merge pull request #2762 from akarnokd/SubscriptionOpt
Optimized isUnsubscribed check
2 parents 6c4c9f1 + 56eb4bd commit 48e4db0

File tree

2 files changed

+98
-65
lines changed

2 files changed

+98
-65
lines changed

‎src/main/java/rx/internal/util/SubscriptionList.java

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
public final class SubscriptionList implements Subscription {
3333

3434
private List<Subscription> subscriptions;
35-
private boolean unsubscribed = false;
35+
private volatile boolean unsubscribed;
3636

3737
public SubscriptionList() {
3838
}
@@ -42,7 +42,7 @@ public SubscriptionList(final Subscription... subscriptions) {
4242
}
4343

4444
@Override
45-
public synchronized boolean isUnsubscribed() {
45+
public boolean isUnsubscribed() {
4646
return unsubscribed;
4747
}
4848

@@ -55,21 +55,19 @@ public synchronized boolean isUnsubscribed() {
5555
* the {@link Subscription} to add
5656
*/
5757
public void add(final Subscription s) {
58-
Subscription unsubscribe = null;
59-
synchronized (this) {
60-
if (unsubscribed) {
61-
unsubscribe = s;
62-
} else {
63-
if (subscriptions == null) {
64-
subscriptions = new LinkedList<Subscription>();
58+
if (!unsubscribed) {
59+
synchronized (this) {
60+
if (!unsubscribed) {
61+
if (subscriptions == null) {
62+
subscriptions = new LinkedList<Subscription>();
63+
}
64+
subscriptions.add(s);
65+
return;
6566
}
66-
subscriptions.add(s);
6767
}
6868
}
69-
if (unsubscribe != null) {
70-
// call after leaving the synchronized block so we're not holding a lock while executing this
71-
unsubscribe.unsubscribe();
72-
}
69+
// call after leaving the synchronized block so we're not holding a lock while executing this
70+
s.unsubscribe();
7371
}
7472

7573
/**
@@ -78,17 +76,19 @@ public void add(final Subscription s) {
7876
*/
7977
@Override
8078
public void unsubscribe() {
81-
List<Subscription> list;
82-
synchronized (this) {
83-
if (unsubscribed) {
84-
return;
79+
if (!unsubscribed) {
80+
List<Subscription> list;
81+
synchronized (this) {
82+
if (unsubscribed) {
83+
return;
84+
}
85+
unsubscribed = true;
86+
list = subscriptions;
87+
subscriptions = null;
8588
}
86-
unsubscribed = true;
87-
list = subscriptions;
88-
subscriptions = null;
89+
// we will only get here once
90+
unsubscribeFromAll(list);
8991
}
90-
// we will only get here once
91-
unsubscribeFromAll(list);
9292
}
9393

9494
private static void unsubscribeFromAll(Collection<Subscription> subscriptions) {
@@ -110,11 +110,25 @@ private static void unsubscribeFromAll(Collection<Subscription> subscriptions) {
110110
}
111111
/* perf support */
112112
public void clear() {
113-
List<Subscription> list;
114-
synchronized (this) {
115-
list = subscriptions;
116-
subscriptions = null;
113+
if (!unsubscribed) {
114+
List<Subscription> list;
115+
synchronized (this) {
116+
list = subscriptions;
117+
subscriptions = null;
118+
}
119+
unsubscribeFromAll(list);
120+
}
121+
}
122+
/**
123+
* Returns true if this composite is not unsubscribed and contains subscriptions.
124+
* @return {@code true} if this composite is not unsubscribed and contains subscriptions.
125+
*/
126+
public boolean hasSubscriptions() {
127+
if (!unsubscribed) {
128+
synchronized (this) {
129+
return !unsubscribed && subscriptions != null && !subscriptions.isEmpty();
130+
}
117131
}
118-
unsubscribeFromAll(list);
132+
return false;
119133
}
120134
}

‎src/main/java/rx/subscriptions/CompositeSubscription.java

Lines changed: 56 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
public final class CompositeSubscription implements Subscription {
3232

3333
private Set<Subscription> subscriptions;
34-
private boolean unsubscribed = false;
34+
private volatile boolean unsubscribed;
3535

3636
public CompositeSubscription() {
3737
}
@@ -41,7 +41,7 @@ public CompositeSubscription(final Subscription... subscriptions) {
4141
}
4242

4343
@Override
44-
public synchronized boolean isUnsubscribed() {
44+
public boolean isUnsubscribed() {
4545
return unsubscribed;
4646
}
4747

@@ -58,21 +58,19 @@ public void add(final Subscription s) {
5858
if (s.isUnsubscribed()) {
5959
return;
6060
}
61-
Subscription unsubscribe = null;
62-
synchronized (this) {
63-
if (unsubscribed) {
64-
unsubscribe = s;
65-
} else {
66-
if (subscriptions == null) {
67-
subscriptions = new HashSet<Subscription>(4);
61+
if (!unsubscribed) {
62+
synchronized (this) {
63+
if (!unsubscribed) {
64+
if (subscriptions == null) {
65+
subscriptions = new HashSet<Subscription>(4);
66+
}
67+
subscriptions.add(s);
68+
return;
6869
}
69-
subscriptions.add(s);
7070
}
7171
}
72-
if (unsubscribe != null) {
73-
// call after leaving the synchronized block so we're not holding a lock while executing this
74-
unsubscribe.unsubscribe();
75-
}
72+
// call after leaving the synchronized block so we're not holding a lock while executing this
73+
s.unsubscribe();
7674
}
7775

7876
/**
@@ -83,16 +81,18 @@ public void add(final Subscription s) {
8381
* the {@link Subscription} to remove
8482
*/
8583
public void remove(final Subscription s) {
86-
boolean unsubscribe = false;
87-
synchronized (this) {
88-
if (unsubscribed || subscriptions == null) {
89-
return;
84+
if (!unsubscribed) {
85+
boolean unsubscribe = false;
86+
synchronized (this) {
87+
if (unsubscribed || subscriptions == null) {
88+
return;
89+
}
90+
unsubscribe = subscriptions.remove(s);
91+
}
92+
if (unsubscribe) {
93+
// if we removed successfully we then need to call unsubscribe on it (outside of the lock)
94+
s.unsubscribe();
9095
}
91-
unsubscribe = subscriptions.remove(s);
92-
}
93-
if (unsubscribe) {
94-
// if we removed successfully we then need to call unsubscribe on it (outside of the lock)
95-
s.unsubscribe();
9696
}
9797
}
9898

@@ -102,28 +102,35 @@ public void remove(final Subscription s) {
102102
* an unoperative state.
103103
*/
104104
public void clear() {
105-
Collection<Subscription> unsubscribe = null;
106-
synchronized (this) {
107-
if (unsubscribed || subscriptions == null) {
108-
return;
109-
} else {
110-
unsubscribe = subscriptions;
111-
subscriptions = null;
105+
if (!unsubscribed) {
106+
Collection<Subscription> unsubscribe = null;
107+
synchronized (this) {
108+
if (unsubscribed || subscriptions == null) {
109+
return;
110+
} else {
111+
unsubscribe = subscriptions;
112+
subscriptions = null;
113+
}
112114
}
115+
unsubscribeFromAll(unsubscribe);
113116
}
114-
unsubscribeFromAll(unsubscribe);
115117
}
116118

117119
@Override
118120
public void unsubscribe() {
119-
synchronized (this) {
120-
if (unsubscribed) {
121-
return;
121+
if (!unsubscribed) {
122+
Collection<Subscription> unsubscribe = null;
123+
synchronized (this) {
124+
if (unsubscribed) {
125+
return;
126+
}
127+
unsubscribed = true;
128+
unsubscribe = subscriptions;
129+
subscriptions = null;
122130
}
123-
unsubscribed = true;
131+
// we will only get here once
132+
unsubscribeFromAll(unsubscribe);
124133
}
125-
// we will only get here once
126-
unsubscribeFromAll(subscriptions);
127134
}
128135

129136
private static void unsubscribeFromAll(Collection<Subscription> subscriptions) {
@@ -143,4 +150,16 @@ private static void unsubscribeFromAll(Collection<Subscription> subscriptions) {
143150
}
144151
Exceptions.throwIfAny(es);
145152
}
153+
/**
154+
* Returns true if this composite is not unsubscribed and contains subscriptions.
155+
* @return {@code true} if this composite is not unsubscribed and contains subscriptions.
156+
*/
157+
public boolean hasSubscriptions() {
158+
if (!unsubscribed) {
159+
synchronized (this) {
160+
return !unsubscribed && subscriptions != null && !subscriptions.isEmpty();
161+
}
162+
}
163+
return false;
164+
}
146165
}

0 commit comments

Comments
 (0)