From 75da199b0694c88f7a79d889821a14af29076bc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 28 Jan 2019 23:28:38 +0100 Subject: [PATCH] Hunt down some redundant events --- .../fxdesigner/MainDesignerController.java | 17 +++++--------- .../fxdesigner/NodeInfoPanelController.java | 23 ++++++++++++------- .../util/fxdesigner/XPathPanelController.java | 18 +++++++-------- .../app/CompositeSelectionSource.java | 5 ++++ .../pmd/util/fxdesigner/app/EventLogger.java | 6 ++--- .../fxdesigner/app/NodeSelectionSource.java | 7 +++++- .../fxdesigner/util/controls/AstTreeView.java | 20 ++++++++-------- 7 files changed, 53 insertions(+), 43 deletions(-) diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java index e6dd37281a..af573beade 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/MainDesignerController.java @@ -8,7 +8,6 @@ import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; -import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -23,14 +22,14 @@ import org.reactfx.value.Val; import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; -import net.sourceforge.pmd.util.fxdesigner.app.DesignerRoot; -import net.sourceforge.pmd.util.fxdesigner.model.XPathEvaluationException; -import net.sourceforge.pmd.util.fxdesigner.popups.EventLogController; import net.sourceforge.pmd.util.fxdesigner.app.AbstractController; import net.sourceforge.pmd.util.fxdesigner.app.CompositeSelectionSource; +import net.sourceforge.pmd.util.fxdesigner.app.DesignerRoot; +import net.sourceforge.pmd.util.fxdesigner.app.NodeSelectionSource; +import net.sourceforge.pmd.util.fxdesigner.model.XPathEvaluationException; +import net.sourceforge.pmd.util.fxdesigner.popups.EventLogController; import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil; import net.sourceforge.pmd.util.fxdesigner.util.LimitedSizeStack; -import net.sourceforge.pmd.util.fxdesigner.app.NodeSelectionSource; import net.sourceforge.pmd.util.fxdesigner.util.SoftReferenceCache; import net.sourceforge.pmd.util.fxdesigner.util.TextAwareNodeWrapper; import net.sourceforge.pmd.util.fxdesigner.util.beans.SettingsPersistenceUtil; @@ -148,12 +147,8 @@ public class MainDesignerController extends AbstractController CompositeSelectionSource.super.bubbleDown(n)); + // this is the only place where getSelectionEvents is called + getSelectionEvents().distinct().subscribe(n -> CompositeSelectionSource.super.bubbleDown(n)); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java index ac2b33ae73..58d00c2d3d 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/NodeInfoPanelController.java @@ -19,6 +19,7 @@ import java.util.stream.Collectors; import org.reactfx.EventStream; import org.reactfx.EventStreams; +import org.reactfx.SuspendableEventStream; import org.reactfx.value.Var; import net.sourceforge.pmd.internal.util.IteratorUtil; @@ -29,9 +30,9 @@ import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.lang.symboltable.Scope; import net.sourceforge.pmd.lang.symboltable.ScopedNode; -import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.app.AbstractController; import net.sourceforge.pmd.util.fxdesigner.app.NodeSelectionSource; +import net.sourceforge.pmd.util.fxdesigner.model.MetricResult; import net.sourceforge.pmd.util.fxdesigner.util.beans.SettingsPersistenceUtil.PersistentProperty; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeCell; import net.sourceforge.pmd.util.fxdesigner.util.controls.ScopeHierarchyTreeItem; @@ -83,6 +84,9 @@ public class NodeInfoPanelController extends AbstractController> myScopeItemSelectionEvents; + + public NodeInfoPanelController(MainDesignerController mainController) { super(mainController); } @@ -101,17 +105,19 @@ public class NodeInfoPanelController extends AbstractController displayAttributes(selectedNode)); + // suppress as early as possible in the pipeline + myScopeItemSelectionEvents = EventStreams.valuesOf(scopeHierarchyTreeView.getSelectionModel().selectedItemProperty()).suppressible(); + } @Override public EventStream getSelectionEvents() { - return EventStreams.valuesOf(scopeHierarchyTreeView.getSelectionModel().selectedItemProperty()) - .filter(Objects::nonNull) - .map(TreeItem::getValue) - .filterMap(o -> o instanceof NameDeclaration, o -> (NameDeclaration) o) - .map(NameDeclaration::getNode) - .map(n -> new NodeSelectionEvent(n, this)); + return myScopeItemSelectionEvents.filter(Objects::nonNull) + .map(TreeItem::getValue) + .filterMap(o -> o instanceof NameDeclaration, o -> (NameDeclaration) o) + .map(NameDeclaration::getNode) + .map(n -> new NodeSelectionEvent(n, this)); } @@ -213,7 +219,8 @@ public class NodeInfoPanelController extends AbstractController myScopeItemSelectionEvents.suspendWhile(() -> scopeHierarchyTreeView.getSelectionModel().select(item))); } } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java index 5b14cfc860..ec4300c5be 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/XPathPanelController.java @@ -19,6 +19,7 @@ import org.controlsfx.validation.ValidationSupport; import org.controlsfx.validation.Validator; import org.reactfx.EventStream; import org.reactfx.EventStreams; +import org.reactfx.SuspendableEventStream; import org.reactfx.collection.LiveArrayList; import org.reactfx.value.Val; import org.reactfx.value.Var; @@ -27,14 +28,14 @@ import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.XPathRule; import net.sourceforge.pmd.lang.rule.xpath.XPathRuleQuery; +import net.sourceforge.pmd.util.fxdesigner.app.AbstractController; import net.sourceforge.pmd.util.fxdesigner.app.LogEntry.Category; +import net.sourceforge.pmd.util.fxdesigner.app.NodeSelectionSource; import net.sourceforge.pmd.util.fxdesigner.model.ObservableXPathRuleBuilder; import net.sourceforge.pmd.util.fxdesigner.model.XPathEvaluationException; import net.sourceforge.pmd.util.fxdesigner.model.XPathEvaluator; import net.sourceforge.pmd.util.fxdesigner.popups.ExportXPathWizardController; -import net.sourceforge.pmd.util.fxdesigner.app.AbstractController; import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil; -import net.sourceforge.pmd.util.fxdesigner.app.NodeSelectionSource; import net.sourceforge.pmd.util.fxdesigner.util.TextAwareNodeWrapper; import net.sourceforge.pmd.util.fxdesigner.util.autocomplete.CompletionResultSource; import net.sourceforge.pmd.util.fxdesigner.util.autocomplete.XPathAutocompleteProvider; @@ -100,6 +101,7 @@ public class XPathPanelController extends AbstractController xpathVersionUIProperty = Var.newSimpleVar(XPathRuleQuery.XPATH_2_0); + private SuspendableEventStream selectionEvents; public XPathPanelController(MainDesignerController mainController) { super(mainController); @@ -127,7 +129,7 @@ public class XPathPanelController extends AbstractController parent.refreshXPathResults()); - + selectionEvents = EventStreams.valuesOf(xpathResultListView.getSelectionModel().selectedItemProperty()).suppressible(); } @@ -219,11 +221,9 @@ public class XPathPanelController extends AbstractController getSelectionEvents() { - return EventStreams.valuesOf(xpathResultListView.getSelectionModel().selectedItemProperty()) - .conditionOn(xpathResultListView.focusedProperty()) - .filter(Objects::nonNull) - .map(TextAwareNodeWrapper::getNode) - .map(n -> new NodeSelectionEvent(n, this)); + return selectionEvents.filter(Objects::nonNull) + .map(TextAwareNodeWrapper::getNode) + .map(n -> new NodeSelectionEvent(n, this)); } @@ -232,7 +232,7 @@ public class XPathPanelController extends AbstractController wrapper.getNode().equals(node)) .findFirst() - .ifPresent(xpathResultListView.getSelectionModel()::select); + .ifPresent(item -> selectionEvents.suspendWhile(() -> xpathResultListView.getSelectionModel().select(item))); } diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/CompositeSelectionSource.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/CompositeSelectionSource.java index 56fd0c7ce0..a621642701 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/CompositeSelectionSource.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/CompositeSelectionSource.java @@ -30,6 +30,11 @@ public interface CompositeSelectionSource extends NodeSelectionSource { } + default boolean isRoot() { + return false; + } + + @Override default void setFocusNode(Node node) { // by default do nothing, diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/EventLogger.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/EventLogger.java index 93a1580162..4be2c5714c 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/EventLogger.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/EventLogger.java @@ -23,13 +23,10 @@ import org.reactfx.collection.LiveArrayList; import org.reactfx.collection.LiveList; import org.reactfx.value.Val; -import net.sourceforge.pmd.util.fxdesigner.app.DesignerRoot; -import net.sourceforge.pmd.util.fxdesigner.app.LogEntry; import net.sourceforge.pmd.util.fxdesigner.app.LogEntry.Category; import net.sourceforge.pmd.util.fxdesigner.app.LogEntry.LogEntryWithData; -import net.sourceforge.pmd.util.fxdesigner.app.ApplicationComponent; -import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil; import net.sourceforge.pmd.util.fxdesigner.app.NodeSelectionSource.NodeSelectionEvent; +import net.sourceforge.pmd.util.fxdesigner.util.DesignerUtil; /** @@ -55,6 +52,7 @@ public class EventLogger implements ApplicationComponent { this.designerRoot = designerRoot; // we have to be careful with initialization order here EventStream> eventTraces = + // none of this is done if developer mode isn't enabled because then those events aren't even pushed in the first place reduceEntangledIfPossible(filterOnCategory(latestEvent, false, SELECTION_EVENT_TRACING).map(t -> (LogEntryWithData) t), // the user data for those is the event // if they're the same event we reduce them together diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/NodeSelectionSource.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/NodeSelectionSource.java index 8ec615df95..76db3bf0ac 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/NodeSelectionSource.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/app/NodeSelectionSource.java @@ -33,7 +33,12 @@ public interface NodeSelectionSource extends ApplicationComponent { /** * Returns a stream of events that should push an event each time * this source or one of its sub components records a change in node - * selection. + * selection. This one needs to be implemented in sub classes. + * + *

You can't trust that this method will return the same stream + * when called several times. In fact it's just called one time. + * That's why you can't abstract the suppressible behaviour here. + * You'd need Scala traits. */ EventStream getSelectionEvents(); diff --git a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/AstTreeView.java b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/AstTreeView.java index e9b16262df..5b777a62a8 100644 --- a/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/AstTreeView.java +++ b/pmd-ui/src/main/java/net/sourceforge/pmd/util/fxdesigner/util/controls/AstTreeView.java @@ -12,7 +12,6 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; import org.reactfx.EventSource; -import org.reactfx.EventStream; import org.reactfx.EventStreams; import org.reactfx.SuspendableEventStream; import org.reactfx.value.Var; @@ -37,25 +36,25 @@ public class AstTreeView extends TreeView implements NodeSelectionSource { private final TreeViewWrapper myWrapper = new TreeViewWrapper<>(this); private ASTTreeItem selectedTreeItem; - private final SuspendableEventStream pausableEvents; + private final SuspendableEventStream selectionEvents; private DesignerRoot designerRoot; public AstTreeView() { - EventSource selectionEvents = new EventSource<>(); - pausableEvents = selectionEvents.pausable(); + EventSource eventSink = new EventSource<>(); + selectionEvents = eventSink.suppressible(); // push a node selection event whenever... // * The selection changes EventStreams.valuesOf(getSelectionModel().selectedItemProperty()) .filterMap(Objects::nonNull, TreeItem::getValue) - .subscribe(selectionEvents::push); + .subscribe(eventSink::push); - // * a cell is explicitly clicked. This catches the case where the cell was already selected + // * the currently selected cell is explicitly clicked setCellFactory(tv -> new ASTTreeCell(n -> { // only push an event if the node was already selected if (selectedTreeItem != null && selectedTreeItem.getValue() != null && selectedTreeItem.getValue().equals(n)) { - selectionEvents.push(n); + eventSink.push(n); } })); @@ -63,8 +62,8 @@ public class AstTreeView extends TreeView implements NodeSelectionSource { @Override - public EventStream getSelectionEvents() { - return pausableEvents.map(n -> new NodeSelectionEvent(n, this)); + public SuspendableEventStream getSelectionEvents() { + return selectionEvents.map(n -> new NodeSelectionEvent(n, this)).suppressible(); } @@ -82,7 +81,8 @@ public class AstTreeView extends TreeView implements NodeSelectionSource { ASTTreeItem found = ((ASTTreeItem) getRoot()).findItem(node); if (found != null) { - pausableEvents.suspendWhile(() -> selectionModel.select(found)); + // don't fire any selection event while itself setting the selected item + selectionEvents.suspendWhile(() -> selectionModel.select(found)); } highlightFocusNodeParents(selectedTreeItem, found);