Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes memory-leak: WeakHashMap -> SoftHashMap<…TickMark> - based cache #291

Merged
merged 1 commit into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 { // NOPMD -nomen est omen
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 { // NOPMD -nomen est omen
public static void main(final String[] args) {
Application.launch(MemoryLeakTestDefaultAxis.class);
}
}