8000 SI-8362: AbstractPromise extends AtomicReference · scala/scala-dev@c201eac · GitHub
[go: up one dir, main page]
Skip to content

Commit c201eac

Browse files
mchvadriaanm
mchv
authored and
adriaanm
committed
SI-8362: AbstractPromise extends AtomicReference
To avoid `sun.misc.Unsafe`, which is not supported on Google App Engine. Deprecate `AbstractPromise` --> extend `j.u.c.atomic.AtomicReference` directly. `AtomicReference.compareAndSet()` should also provide better performance on HotSpot, which compiles it down to the machine's CAS instruction. The binary incompatible change is ok because it's in an internal package. I can't think of any real issue with adding a superclass (which contributes only final methods) to a class in an implementation package (as long as those methods were not introduced in any illicit subclasses of said class). Instead of changing `DefaultPromise`'s super class, let's be more conservative, and do it closest to the source. This is both clearer and more focussed, leaving those subclasses of AbstractPromise we never heard of unaffected. Genesis of the commit: since the work on `Future` performance, `AbstractPromise` is using `Unsafe`, breaking the ability for `Future` to be executed on GAE. At that time, viktorklang suggested to implement a fallback in case `Unsafe` is not available. carey proposed an implementation, and mchv submitted a patch, which was refined by adriaanm.
1 parent a745f06 commit c201eac

File tree

3 files changed

+25
-30
lines changed

3 files changed

+25
-30
lines changed

‎bincompat-backward.whitelist.conf

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,15 @@ filter {
208208
{
209209
matchName="scala.reflect.io.ZipArchive.scala$reflect$io$ZipArchive$$walkIterator"
210210
problemName=MissingMethodProblem
211+
},
212+
// SI-8362: AbstractPromise extends AtomicReference
213+
// It's ok to change a package-protected class in an impl package,
214+
// even though it's not clear why it changed -- bug in generic signature generation?
215+
// -public class scala.concurrent.impl.Promise$DefaultPromise<T> extends scala.concurrent.impl.AbstractPromise implements scala.concurrent.impl.Promise<T>
216+
// +public class scala.concurrent.impl.Promise$DefaultPromise<T extends java.lang.Object> extends scala.concurrent.impl.AbstractPromise implements scala.concurrent.impl.Promise<T>
217+
{
218+
matchName="scala.concurrent.impl.Promise$DefaultPromise"
219+
problemName=MissingTypesProblem
211220
}
212221
]
213222
}

‎bincompat-forward.whitelist.conf

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,15 @@ filter {
372372
{
373373
matchName="scala.util.Sorting.scala$util$Sorting$$booleanSort"
374374
problemName=MissingMethodProblem
375+
},
376+
// SI-8362: AbstractPromise extends AtomicReference
377+
// It's ok to change a package-protected class in an impl package,
378+
// even though it's not clear why it changed -- bug in generic signature generation?
379+
// -public class scala.concurrent.impl.Promise$DefaultPromise<T> extends scala.concurrent.impl.AbstractPromise implements scala.concurrent.impl.Promise<T>
380+
// +public class scala.concurrent.impl.Promise$DefaultPromise<T extends java.lang.Object> extends scala.concurrent.impl.AbstractPromise implements scala.concurrent.impl.Promise<T>
381+
{
382+
matchName="scala.concurrent.impl.Promise$DefaultPromise"
383+
problemName=MissingTypesProblem
375384
}
376385
]
377386
}

‎src/library/scala/concurrent/impl/AbstractPromise.java

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,17 @@
11
/* __ *\
22
** ________ ___ / / ___ Scala API **
3-
** / __/ __// _ | / / / _ | (c) 2003-2013, LAMP/EPFL **
3+
** / __/ __// _ | / / / _ | (c) 2003-2015, LAMP/EPFL **
44
** __\ \/ /__/ __ |/ /__/ __ | http://scala-lang.org/ **
55
** /____/\___/_/ |_/____/_/ | | **
66
** |/ **
77
\* */
88

99
package scala.concurrent.impl;
1010

11+
import java.util.concurrent.atomic.AtomicReference;
1112

12-
import scala.concurrent.util.Unsafe;
13-
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
14-
15-
16-
17-
abstract class AbstractPromise {
18-
private volatile Object _ref;
19-
20-
final static long _refoffset;
21-
22-
static {
23-
try {
24-
_refoffset = Unsafe.instance.objectFieldOffset(AbstractPromise.class.getDeclaredField("_ref"));
25-
} catch (Throwable t) {
26-
throw new ExceptionInInitializerError(t);
27-
}
28-
}
29-
30-
protected final boolean updateState(Object oldState, Object newState) {
31-
return Unsafe.instance.compareAndSwapObject(this, _refoffset, oldState, newState);
32-
}
33-
34-
protected final Object getState() {
35-
return _ref;
36-
}
37-
38-
protected final static AtomicReferenceFieldUpdater<AbstractPromise, Object> updater =
39-
AtomicReferenceFieldUpdater.newUpdater(AbstractPromise.class, Object.class, "_ref");
40-
}
13+
@Deprecated // Since 2.11.8. Extend java.util.concurrent.atomic.AtomicReference instead.
14+
abstract class AbstractPromise extends AtomicReference<Object> {
15+
protected final boolean updateState(Object oldState, Object newState) { return compareAndSet(oldState, newState); }
16+
protected final Object getState() { return get(); }
17+
}

0 commit comments

Comments
 (0)
0