Skip to content

Commit cb0471e

Browse files
committed
HeatMap was not thread safe. Fixes #267.
1 parent a35ddbb commit cb0471e

File tree

3 files changed

+66
-9
lines changed

3 files changed

+66
-9
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package org.baderlab.csplugins.enrichmentmap.util;
2+
3+
import java.util.Map;
4+
import java.util.WeakHashMap;
5+
6+
import javax.swing.Timer;
7+
8+
public class CoalesceTimerStore<T> {
9+
10+
public static final int DEFAULT_DELAY = 120;
11+
12+
// Note: I think this code can leak Timer objects.
13+
// Probably need to explicitly manage a ReferenceQueue.
14+
15+
private final Map<T,Timer> store = new WeakHashMap<>();
16+
private final int delay;
17+
18+
public CoalesceTimerStore(int delay) {
19+
this.delay = delay;
20+
}
21+
22+
public CoalesceTimerStore() {
23+
this(DEFAULT_DELAY);
24+
}
25+
26+
/**
27+
* The Java standard library doesn't have a map with both concurrency and weak keys.
28+
* Also the default implementation of compute isn't atomic. Therefore we will synchronize explicitly.
29+
*/
30+
public synchronized void coalesce(T key, Runnable runnable) {
31+
store.compute(key, (k,t) -> {
32+
if(t == null) {
33+
//System.out.println("start timer");
34+
t = new Timer(0, evt -> {
35+
//System.out.println("fire timer");
36+
runnable.run();
37+
});
38+
t.setRepeats(false);
39+
t.setCoalesce(true);
40+
} else {
41+
//System.out.println("reset timer");
42+
t.stop();
43+
}
44+
45+
t.setInitialDelay(delay);
46+
t.start();
47+
return t;
48+
});
49+
}
50+
51+
public synchronized void remove(T key) {
52+
Timer timer = store.remove(key);
53+
if(timer != null)
54+
timer.stop();
55+
}
56+
57+
}

EnrichmentMapPlugin/src/main/java/org/baderlab/csplugins/enrichmentmap/view/heatmap/HeatMapMediator.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.baderlab.csplugins.enrichmentmap.model.EnrichmentMapManager;
1717
import org.baderlab.csplugins.enrichmentmap.model.Ranking;
1818
import org.baderlab.csplugins.enrichmentmap.style.EMStyleBuilder;
19+
import org.baderlab.csplugins.enrichmentmap.util.CoalesceTimerStore;
1920
import org.baderlab.csplugins.enrichmentmap.view.heatmap.HeatMapParams.Compress;
2021
import org.baderlab.csplugins.enrichmentmap.view.heatmap.HeatMapParams.Operator;
2122
import org.cytoscape.application.CyApplicationManager;
@@ -56,6 +57,7 @@ public class HeatMapMediator implements RowsSetListener, SetCurrentNetworkViewLi
5657
@Inject private CySwingApplication swingApplication;
5758
@Inject private CyApplicationManager applicationManager;
5859

60+
private final CoalesceTimerStore<HeatMapMediator> selectionEventTimer = new CoalesceTimerStore<>(60);
5961
private HeatMapParentPanel heatMapPanel = null;
6062
private boolean onlyEdges;
6163

@@ -89,15 +91,13 @@ private void bringToFront() {
8991
public void handleEvent(RowsSetEvent e) {
9092
if(heatMapPanel == null)
9193
return;
92-
// MKTODO If this has bad performance then add a reconciler timer delay.
93-
// Cytoscape selection events can come in sets of 1-4 events.
9494
if(e.containsColumn(CyNetwork.SELECTED)) {
9595
CyNetworkView networkView = applicationManager.getCurrentNetworkView();
9696
if(networkView != null) {
9797
CyNetwork network = networkView.getModel();
9898
// only handle event if it is a selected node
9999
if(e.getSource() == network.getDefaultEdgeTable() || e.getSource() == network.getDefaultNodeTable()) {
100-
updateHeatMap(networkView);
100+
selectionEventTimer.coalesce(this, () -> updateHeatMap(networkView));
101101
}
102102
}
103103
}

EnrichmentMapPlugin/src/main/java/org/baderlab/csplugins/enrichmentmap/view/heatmap/HeatMapParentPanel.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ public void CreateContents() {
5252
showEmptyView();
5353
}
5454

55-
public void selectGenes(EnrichmentMap map, HeatMapParams params, List<RankingOption> moreRankOptions, Set<String> union, Set<String> intersection) {
55+
public HeatMapMediator getMediator() {
56+
return mediator;
57+
}
58+
59+
public synchronized void selectGenes(EnrichmentMap map, HeatMapParams params, List<RankingOption> moreRankOptions, Set<String> union, Set<String> intersection) {
5660
if(mainPanel == null) {
5761
removeAll();
5862
mainPanel = mainPanelFactory.create(this);
@@ -61,11 +65,7 @@ public void selectGenes(EnrichmentMap map, HeatMapParams params, List<RankingOpt
6165
mainPanel.reset(map, params, moreRankOptions, union, intersection);
6266
}
6367

64-
public HeatMapMediator getMediator() {
65-
return mediator;
66-
}
67-
68-
public void showEmptyView() {
68+
public synchronized void showEmptyView() {
6969
removeAll();
7070
mainPanel = null;
7171
add(new NullViewPanel(), BorderLayout.CENTER);

0 commit comments

Comments
 (0)