Skip to content

Commit

Permalink
fixed slow memory-leak by replacing WeakHashMap with SoftHashMap<...,…
Browse files Browse the repository at this point in the history
…TickMark> - based cache implementation

See following references for details:
https://ewirch.github.io/2013/12/weakhashmap-memory-leaks.html
https://franke.ms/memoryleak1.wiki
N.B. latter author filed this as a bug at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7145759 which was apparently dropped by Oracle devs as supposedly the intended behaviour for a WeakHashMap-based cache.
  • Loading branch information
RalphSteinhagen committed Oct 30, 2020
1 parent eb7a286 commit 17a4ac1
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import java.util.LinkedList;
import java.util.List;
import java.util.WeakHashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.ReentrantLock;

Expand Down Expand Up @@ -36,6 +36,7 @@
import de.gsi.chart.ui.ResizableCanvas;
import de.gsi.chart.ui.geometry.Side;
import de.gsi.dataset.event.AxisChangeEvent;
import de.gsi.dataset.utils.SoftHashMap;

/**
* @author rstein
Expand Down Expand Up @@ -94,10 +95,10 @@ protected void invalidated() {
};

// cache for major tick marks
protected final transient WeakHashMap<String, TickMark> tickMarkStringCache = new WeakHashMap<>();
protected final transient Map<String, TickMark> tickMarkStringCache = new SoftHashMap<>(MAX_TICK_COUNT);

// cache for minor tick marks (N.B. usually w/o string label)
protected final transient WeakHashMap<Double, TickMark> tickMarkDoubleCache = new WeakHashMap<>();
protected final transient Map<Double, TickMark> tickMarkDoubleCache = new SoftHashMap<>(MAX_TICK_COUNT * DEFAULT_MINOR_TICK_COUNT);

public AbstractAxis() {
super();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
public abstract class AbstractAxisParameter extends Pane implements Axis {
private static final String CHART_CSS = Chart.class.getResource("chart.css").toExternalForm();
private static final CssPropertyFactory<AbstractAxisParameter> CSS = new CssPropertyFactory<>(Region.getClassCssMetaData());
private static final int MAX_TICK_COUNT = 20;
protected static final int MAX_TICK_COUNT = 20;
private static final double DEFAULT_MIN_RANGE = -1.0;
private static final double DEFAULT_MAX_RANGE = +1.0;
private static final double DEFAULT_TICK_UNIT = +5d;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package de.gsi.chart.axes.spi;

import java.util.Map;
import java.util.Timer;
import java.util.TimerTask;

import javafx.application.Application;
import javafx.application.Platform;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import de.gsi.chart.utils.FXUtils;

/**
* Small test to demonstrate that the SoftHashMap TickMark cache sizes are in fact limited, memory-bound and do not leak
* rather than their earlier WeakHashMap-based counterpart that had issues when the weak key was also part of the kept value.
*
* See following references for details:
* https://ewirch.github.io/2013/12/weakhashmap-memory-leaks.html
* https://franke.ms/memoryleak1.wiki
* N.B. latter author filed this as a bug at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7145759 which was apparently
* dropped by Oracle devs as supposedly the intended behaviour for a WeakHashMap-based cache.
*
* effect is best seen with limiting the jvm's max memory: -Xmx20m
*
* @author rstein
*/
public class MemoryLeakTestDefaultAxis extends Application {
private static final Logger LOGGER = LoggerFactory.getLogger(MemoryLeakTestDefaultAxis.class);
@Override
public void start(final Stage primaryStage) {
final TestAxis axis = new TestAxis();
VBox.setMargin(axis, new Insets(5, 50, 5, 50));
VBox.setVgrow(axis, Priority.ALWAYS);

new Timer(true).scheduleAtFixedRate(new TimerTask() {
private int counter;
@Override
public void run() {
FXUtils.runFX(() -> axis.set(now(), now() + 1));
if (counter % 5000 == 0) {
LOGGER.atInfo().addArgument(axis.getTickMarkDoubleCache().size()).addArgument(axis.getTickMarkStringCache().size()).log("cache sizes - Map<Double,TickMark> = {} Map<String,TickMark> = {}");
System.gc(); // NOPMD NOSONAR - yes we need to eliminate the non-deterministic behaviour of the jvm's gc
System.gc(); // NOPMD NOSONAR - yes we need to eliminate the non-deterministic behaviour of the jvm's gc
}
counter++;
}
}, 0, 1);

primaryStage.setTitle(this.getClass().getSimpleName());
primaryStage.setScene(new Scene(new VBox(axis), 1800, 100));
primaryStage.setOnCloseRequest(evt -> Platform.exit());
primaryStage.show();
}

public static void main(final String[] args) {
Application.launch(args);
}

private static double now() {
return 0.001 * System.currentTimeMillis(); // [s]
}

private class TestAxis extends DefaultNumericAxis {
public TestAxis() {
super("test axis", now(), now() + 1, 0.05);
}

public Map<Double, TickMark> getTickMarkDoubleCache() {
return tickMarkDoubleCache;
}

public Map<String, TickMark> getTickMarkStringCache() {
return tickMarkStringCache;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package de.gsi.chart.axes.spi;

import javafx.application.Application;

/**
* Small test to demonstrate that the SoftHashMap TickMark cache sizes are in fact limited, memory-bound and do not leak
* rather than their earlier WeakHashMap-based counterpart that had issues when the weak key was also part of the kept value.
*
* See following references for details:
* https://ewirch.github.io/2013/12/weakhashmap-memory-leaks.html
* https://franke.ms/memoryleak1.wiki
* N.B. latter author filed this as a bug at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7145759 which was apparently
* dropped by Oracle devs as supposedly the intended behaviour for a WeakHashMap-based cache.
*
* effect is best seen with limiting the jvm's max memory: -Xmx20m
*
* @author rstein
*/
public class MemoryLeakTestDefaultAxis_run {
public static void main(final String[] args) {
Application.launch(MemoryLeakTestDefaultAxis.class);
}
}

0 comments on commit 17a4ac1

Please sign in to comment.