How often are listeners added and removed? It might make more sense to
make them copy-on-write instead...?
...jim
On 1/14/16 4:52 AM, Doswald Michael wrote:
While profiling a JavaFX application that runs on embedded hardware, I have
found that the Toolkit.firePulse method creates more garbage than necessary. In
an application that simply animates an object without doing much else, I see
that the firePulse method allocates a fair amount of objects (compared to the
overall number of allocated objects). I have written a small patch that
decreases the allocation rate in said method. Since I'm not sure if such tweaks
are desired in the JavaFX codebase, I didn't open a JIRA issue. I'm happy to do
so if they are.
The following small application animates a single circle on screen. The
allocation rate of the 'JavaFX Application Thread' is as follows (measured with
jvisualvm):
openjfx 8u HEAD: ~ 65 KB/s after start, ~ 49 KB/s after JIT compilation
kicked in
with patch: ~ 25 KB/s, doesn't seem to change when JIT kicks
in
public class AllocateApp extends Application {
public static void main(String... args) {
Application.launch(args);
}
@Override
public void start(Stage primaryStage) throws Exception {
Circle circle = new Circle(10, 25, 10);
Timeline timeline = new Timeline(new KeyFrame(Duration.seconds(2), new
KeyValue(circle.centerXProperty(), 490)));
timeline.setCycleCount(Timeline.INDEFINITE);
timeline.setAutoReverse(true);
timeline.play();
primaryStage.setScene(new Scene(new Pane(circle), 500, 50));
primaryStage.show();
}
}
The patch below only creates a single List object instead of three WeakHashMaps
to make a local copy of the TKPulseListeners. It also uses the number of
listeners in the maps to estimate the size of the list. The patch uses
WeakReference objects in the list, which I doubt is necessary, but it emulates
the previous behaviour more accurately. I believe it would be possible to
change that to a strong reference for the following reasons:
a) The list is local and the strong references would only be there for the time
the firePulse method is run
b) The code would become more readable (less generics parameters, null-guard
not necessary in loop)
c) Getting rid of the WeakReference objects would decrease garbage generation
even more (down to ~ 21 KB/s)
Regards
Michael
diff -r f1c3eb85af4d
modules/graphics/src/main/java/com/sun/javafx/tk/Toolkit.java
--- a/modules/graphics/src/main/java/com/sun/javafx/tk/Toolkit.java Fri Jan
08 08:11:51 2016 -0800
+++ b/modules/graphics/src/main/java/com/sun/javafx/tk/Toolkit.java Wed Jan
13 10:43:27 2016 +0100
@@ -51,7 +51,6 @@
import javafx.scene.shape.StrokeType;
import javafx.stage.FileChooser.ExtensionFilter;
import javafx.stage.Modality;
-import javafx.stage.Stage;
import javafx.stage.StageStyle;
import javafx.stage.Window;
import java.io.File;
@@ -94,6 +93,8 @@
import com.sun.scenario.effect.Color4f;
import com.sun.scenario.effect.FilterContext;
import com.sun.scenario.effect.Filterable;
+import java.lang.ref.WeakReference;
+import javafx.util.Pair;
public abstract class Toolkit {
@@ -362,32 +363,36 @@
// and those changes propogated to scene before it gets its pulse to
update
// Copy of listener map
- final Map<TKPulseListener,AccessControlContext> stagePulseList =
- new WeakHashMap<TKPulseListener,AccessControlContext>();
- final Map<TKPulseListener,AccessControlContext> scenePulseList =
- new WeakHashMap<TKPulseListener,AccessControlContext>();
- final Map<TKPulseListener,AccessControlContext> postScenePulseList =
- new WeakHashMap<TKPulseListener,AccessControlContext>();
+ final List<Pair<WeakReference<TKPulseListener>,AccessControlContext>>
listenersList;
synchronized (this) {
- stagePulseList.putAll(stagePulseListeners);
- scenePulseList.putAll(scenePulseListeners);
- postScenePulseList.putAll(postScenePulseListeners);
+ listenersList = new
ArrayList<>(stagePulseListeners.size()+scenePulseListeners.size()+postScenePulseListeners.size());
+ copyListeners(stagePulseListeners, listenersList);
+ copyListeners(scenePulseListeners, listenersList);
+ copyListeners(postScenePulseListeners, listenersList);
}
- for (Map.Entry<TKPulseListener,AccessControlContext> entry :
stagePulseList.entrySet()) {
- runPulse(entry.getKey(), entry.getValue());
- }
- for (Map.Entry<TKPulseListener,AccessControlContext> entry :
scenePulseList.entrySet()) {
- runPulse(entry.getKey(), entry.getValue());
- }
- for (Map.Entry<TKPulseListener,AccessControlContext> entry :
postScenePulseList.entrySet()) {
- runPulse(entry.getKey(), entry.getValue());
+
+ for (int idx = 0, max = listenersList.size(); idx < max; idx++) {
+ Pair<WeakReference<TKPulseListener>, AccessControlContext>
listenerEntry = listenersList.get(idx);
+ TKPulseListener pulseListener = listenerEntry.getKey().get();
+ if (pulseListener != null) {
+ runPulse(pulseListener, listenerEntry.getValue());
+ }
}
if (lastTkPulseListener != null) {
runPulse(lastTkPulseListener, lastTkPulseAcc);
}
}
+
+ private void copyListeners(Map<TKPulseListener, AccessControlContext> listenerMap,
List<Pair<WeakReference<TKPulseListener>,AccessControlContext>> listenersList) {
+ if (!listenerMap.isEmpty()) {
+ for (Map.Entry<TKPulseListener,AccessControlContext> entry :
listenerMap.entrySet()) {
+ listenersList.add(new Pair<>(new
WeakReference<>(entry.getKey()), entry.getValue()));
+ }
+ }
+ }
+
public void addStageTkPulseListener(TKPulseListener listener) {
if (listener == null) {
return;
