On 1/14/16 8:09 PM, Jim Graham wrote:
> How often are listeners added and removed? It might make more sense to
> make them copy-on-write instead...?
That is a good point. As far as I can see, every window adds a stage listener
when it is shown and removes it if it is closed. Every scene adds/removes a
scene listener. So the listener maps are changed rarely. The post scene pulse
listener seems to be used for the snapshot functionality. Although I can only
see that such a listener is added, but it seems it will never be removed again.
I will try to revise my patch and open an RFE as suggested by Kevin as soon as
I handed in my OCA.
Regards,
Michael
>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;
>>
>>
>>
>>
>>
>>
>>
>>
