Skip to content

Commit

Permalink
Enable figures to retain references to objects they rely on (deephave…
Browse files Browse the repository at this point in the history
…n#4552)

Also turns down log levels when testing/generating figure code.

Fixes deephaven#4540
  • Loading branch information
niloc132 authored Oct 6, 2023
1 parent a3eb75b commit 1c588df
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ private static void generate(final boolean assertNoChange, final String file, fi
out.print(newcode);
out.close();

log.warning(file + " written");
log.info(file + " written");
}
}

Expand All @@ -892,7 +892,7 @@ public static void main(String[] args) throws IOException {
}

log.setLevel(Level.WARNING);
log.warning("Running GenerateAxesPlotMethods assertNoChange=" + assertNoChange);
log.info("Running GenerateAxesPlotMethods assertNoChange=" + assertNoChange);

final String fileIface = devroot + "/Plot/src/main/java/io/deephaven/plot/Axes.java";
final String fileImpl = devroot + "/Plot/src/main/java/io/deephaven/plot/AxesImpl.java";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.util.function.Predicate;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static io.deephaven.plot.util.PlotGeneratorUtils.indent;

Expand All @@ -30,14 +32,14 @@
public class GenerateFigureImmutable {
// See also GroovyStaticImportGenerator

private static Logger log = Logger.getLogger(GenerateFigureImmutable.class.toString());
private static final Logger log = Logger.getLogger(GenerateFigureImmutable.class.toString());

private static final String CLASS_NAME_INTERFACE = "io.deephaven.plot.Figure";
private static final String CLASS_NAME_IMPLEMENTATION = "io.deephaven.plot.FigureImpl";

private final String outputClass;
private final String outputClassNameShort;
private boolean isInterface;
private final boolean isInterface;
private final String[] imports;
private final String[] interfaces;
private final String[] seriesInterfaces;
Expand Down Expand Up @@ -109,7 +111,7 @@ private void addPublicNonStatic(Method m) {
boolean skip = skip(f);

if (skip) {
log.warning("*** Skipping function: " + f);
log.info("*** Skipping function: " + f);
return;
}

Expand Down Expand Up @@ -194,16 +196,10 @@ private String generateImplements() {
final StringBuilder sb = new StringBuilder();

if (isInterface) {
sb.append(" extends ");
sb.append("java.io.Serializable");

for (final String[] ii : new String[][] {interfaces, seriesInterfaces}) {
for (final String iface : ii) {
// final String[] siface = iface.split("[.]");
// final String name = siface[siface.length - 1];
sb.append(", ").append(iface);
}
}
sb.append(" extends ").append(
Stream.concat(
Stream.of(interfaces),
Stream.of(seriesInterfaces)).collect(Collectors.joining(", ")));
} else {
sb.append(" implements " + CLASS_NAME_INTERFACE);
}
Expand Down Expand Up @@ -247,7 +243,7 @@ private String generateCode() {
final boolean skip = skip(f);

if (skip) {
log.warning("*** Skipping function: " + f);
log.info("*** Skipping function: " + f);
continue;
}

Expand All @@ -262,7 +258,7 @@ private String generateCode() {
final boolean skip = skip(f);

if (skip) {
log.warning("*** Skipping function: " + f);
log.info("*** Skipping function: " + f);
continue;
}

Expand Down Expand Up @@ -941,7 +937,7 @@ private static void generateFile(final String devroot, final boolean assertNoCha
throws ClassNotFoundException, IOException {

log.setLevel(Level.WARNING);
log.warning("Running GenerateFigureImmutable assertNoChange=" + assertNoChange);
log.info("Running GenerateFigureImmutable assertNoChange=" + assertNoChange);

final String[] imports = {
"io.deephaven.plot.datasets.DataSeriesInternal",
Expand Down Expand Up @@ -1019,7 +1015,7 @@ private static void generateFile(final String devroot, final boolean assertNoCha
out.print(code);
out.close();

log.warning(gen.outputClass + " written to: " + file);
log.info(gen.outputClass + " written to: " + file);
}
}

Expand Down
2 changes: 1 addition & 1 deletion Plot/src/main/java/io/deephaven/plot/Figure.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

/** An interface for constructing plots. A Figure is immutable, and all function calls return a new immutable Figure instance.*/
@SuppressWarnings({"unused", "RedundantCast", "SameParameterValue"})
public interface Figure extends java.io.Serializable, io.deephaven.plot.BaseFigure, io.deephaven.plot.Chart, io.deephaven.plot.Axes, io.deephaven.plot.Axis, io.deephaven.plot.datasets.DataSeries, io.deephaven.plot.datasets.category.CategoryDataSeries, io.deephaven.plot.datasets.interval.IntervalXYDataSeries, io.deephaven.plot.datasets.ohlc.OHLCDataSeries, io.deephaven.plot.datasets.xy.XYDataSeries, io.deephaven.plot.datasets.multiseries.MultiSeries, io.deephaven.plot.datasets.xy.XYDataSeriesFunction, io.deephaven.plot.datasets.xyerrorbar.XYErrorBarDataSeries, io.deephaven.plot.datasets.categoryerrorbar.CategoryErrorBarDataSeries {
public interface Figure extends io.deephaven.plot.BaseFigure, io.deephaven.plot.Chart, io.deephaven.plot.Axes, io.deephaven.plot.Axis, io.deephaven.plot.datasets.DataSeries, io.deephaven.plot.datasets.category.CategoryDataSeries, io.deephaven.plot.datasets.interval.IntervalXYDataSeries, io.deephaven.plot.datasets.ohlc.OHLCDataSeries, io.deephaven.plot.datasets.xy.XYDataSeries, io.deephaven.plot.datasets.multiseries.MultiSeries, io.deephaven.plot.datasets.xy.XYDataSeriesFunction, io.deephaven.plot.datasets.xyerrorbar.XYErrorBarDataSeries, io.deephaven.plot.datasets.categoryerrorbar.CategoryErrorBarDataSeries {


/**
Expand Down
36 changes: 33 additions & 3 deletions Plot/src/main/java/io/deephaven/plot/FigureWidget.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,55 @@
package io.deephaven.plot;

import io.deephaven.datastructures.util.CollectionUtil;
import io.deephaven.engine.liveness.DelegatingLivenessNode;
import io.deephaven.engine.liveness.LivenessArtifact;
import io.deephaven.engine.liveness.LivenessNode;
import io.deephaven.engine.updategraph.DynamicNode;
import io.deephaven.engine.util.FigureWidgetMarker;
import io.deephaven.engine.util.LiveWidget;
import io.deephaven.engine.util.LiveWidgetVisibilityProvider;
import io.deephaven.plot.util.tables.PartitionedTableHandle;
import io.deephaven.plot.util.tables.TableHandle;
import io.deephaven.util.annotations.ScriptApi;

import java.util.*;

/**
* Displayable version of a Figure.
*/
public class FigureWidget extends FigureImpl implements LiveWidget, LiveWidgetVisibilityProvider, FigureWidgetMarker {
public class FigureWidget extends FigureImpl
implements LiveWidget, LiveWidgetVisibilityProvider, FigureWidgetMarker, DelegatingLivenessNode {

private static final long serialVersionUID = 763409998768966385L;
private String[] validGroups;

@SuppressWarnings("WeakerAccess") // this is used in the python integration
/**
* By making this a non-static inner class, we can do two things:
* <ul>
* <li>we can call the protected constructors</li>
* <li>any hard reference made to the liveness artifact itself will ensure reachability to the FigureWidget (this is
* uncommon, but technically supported)</li>
* </ul>
*/
private final LivenessArtifact livenessImpl = new LivenessArtifact() {};

public FigureWidget(final FigureImpl figure) {
super(figure);
figure.getFigure().consolidatePartitionedTables();
getFigure().consolidatePartitionedTables();

getFigure().getTableHandles().stream()
.map(TableHandle::getTable)
.filter(DynamicNode::notDynamicOrIsRefreshing)
.forEach(this::manage);
getFigure().getPartitionedTableHandles().stream()
.map(PartitionedTableHandle::getPartitionedTable)
.filter(DynamicNode::notDynamicOrIsRefreshing)
.forEach(this::manage);
}

@Override
public LivenessNode asLivenessNode() {
return livenessImpl;
}

@ScriptApi
Expand Down
45 changes: 45 additions & 0 deletions Plot/src/test/java/io/deephaven/plot/TestFigureLiveness.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package io.deephaven.plot;

import io.deephaven.engine.liveness.LivenessReferent;
import io.deephaven.engine.liveness.LivenessScope;
import io.deephaven.engine.liveness.LivenessScopeStack;
import io.deephaven.engine.table.Table;
import io.deephaven.engine.testutil.testcase.RefreshingTableTestCase;
import io.deephaven.engine.util.TableTools;
import io.deephaven.util.SafeCloseable;

public class TestFigureLiveness extends RefreshingTableTestCase {
public void testFigureLiveness() {
// Scope that represents the incoming grpc call
LivenessScope reqScope = new LivenessScope();

// Scope that the work is done in
LivenessScope scope = new LivenessScope();
Table t;
Figure plot;
try (SafeCloseable ignored = LivenessScopeStack.open(scope, false)) {
t = TableTools.timeTable("PT1s").updateView("Y=i", "X=i", "Z=i");
plot = PlottingConvenience.plot("series", t, "X", "Y")
.plotBy("multieseries", t, "X", "Y", "Z")
.show();
reqScope.manage((LivenessReferent) plot);// explicitly manage outside of the scope
}

// Assert that the tables are retained while the scope is still open (then drop the new reference)
assertTrue(t.tryRetainReference());
t.dropReference();

// Drop the inner scope
scope.release();

// Assert that the table reference is retained now by virtue of the plot that holds it
assertTrue(t.tryRetainReference());
t.dropReference();

// Simulate the user releasing their export
reqScope.release();

// Assert that the table reference is no longer held after the figure is released
assertFalse(t.tryRetainReference());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package io.deephaven.engine.liveness;

import org.jetbrains.annotations.NotNull;

import java.lang.ref.WeakReference;
import java.util.stream.Stream;

/**
* Indicates that this class implements LivenessNode via a member rather than implementing it directly. The real
* LivenessNode is exposed via {@link #asLivenessNode()}, all other methods delegate to this instance.
*/
public interface DelegatingLivenessNode extends LivenessNode {
/**
* Returns the "real" {@link LivenessNode} instance. When implementing this, care should be taken to match lifecycle
* of the {@code DelegatingLivenessNode} instance with this instance, as the returned {@code LivenessNode} behaves
* as a proxy for {@code this}.
*
* @return a LivenessNode to use to manage this object's liveness.
*/
LivenessNode asLivenessNode();

@Override
default boolean tryManage(@NotNull LivenessReferent referent) {
return asLivenessNode().tryManage(referent);
}

@Override
default boolean tryUnmanage(@NotNull LivenessReferent referent) {
return asLivenessNode().tryManage(referent);
}

@Override
default boolean tryUnmanage(@NotNull Stream<? extends LivenessReferent> referents) {
return asLivenessNode().tryUnmanage(referents);
}

@Override
default boolean tryRetainReference() {
return asLivenessNode().tryRetainReference();
}

@Override
default void dropReference() {
asLivenessNode().dropReference();
}

@Override
default WeakReference<? extends LivenessReferent> getWeakReference() {
return asLivenessNode().getWeakReference();
}
}

0 comments on commit 1c588df

Please sign in to comment.