From cf6d703c18a5e6161db87f959e7f1a343f6ef194 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 25 Aug 2016 17:46:47 +0200 Subject: [PATCH 01/11] Fixes metrics-on-canvas updating. - We were being a bit overzealous w/ our layout caching. --- client/app/scripts/charts/nodes-chart.js | 6 ++---- client/app/scripts/utils/topology-utils.js | 7 ------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index e42e239a0..95ba8a467 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -12,8 +12,7 @@ import { MIN_NODE_SIZE, DETAILS_PANEL_WIDTH, MAX_NODE_SIZE } from '../constants/ import Logo from '../components/logo'; import { doLayout } from './nodes-layout'; import NodesChartElements from './nodes-chart-elements'; -import { getActiveTopologyOptions, getAdjacentNodes, - isSameTopology } from '../utils/topology-utils'; +import { getActiveTopologyOptions, getAdjacentNodes } from '../utils/topology-utils'; const log = debug('scope:nodes-chart'); @@ -82,8 +81,7 @@ class NodesChart extends React.Component { state.height = nextProps.forceRelayout ? nextProps.height : (state.height || nextProps.height); state.width = nextProps.forceRelayout ? nextProps.width : (state.width || nextProps.width); - // _.assign(state, this.updateGraphState(nextProps, state)); - if (nextProps.forceRelayout || !isSameTopology(nextProps.nodes, this.props.nodes)) { + if (nextProps.forceRelayout || nextProps.nodes !== this.props.nodes) { _.assign(state, this.updateGraphState(nextProps, state)); } diff --git a/client/app/scripts/utils/topology-utils.js b/client/app/scripts/utils/topology-utils.js index ae22afaba..3e90c6e36 100644 --- a/client/app/scripts/utils/topology-utils.js +++ b/client/app/scripts/utils/topology-utils.js @@ -171,13 +171,6 @@ export function getCurrentTopologyUrl(state) { return state.getIn(['currentTopology', 'url']); } -export function isSameTopology(nodes, nextNodes) { - const mapper = node => makeMap({id: node.get('id'), adjacency: node.get('adjacency')}); - const topology = nodes.map(mapper); - const nextTopology = nextNodes.map(mapper); - return isDeepEqual(topology, nextTopology); -} - export function isNodeMatchingQuery(node, query) { return node.get('label').includes(query) || node.get('subLabel').includes(query); } From b3f05ae97b8f32508832c7449ac8ecb2e1f80152 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 25 Aug 2016 17:49:38 +0200 Subject: [PATCH 02/11] Tidy up unused imports --- client/app/scripts/utils/topology-utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/app/scripts/utils/topology-utils.js b/client/app/scripts/utils/topology-utils.js index 3e90c6e36..f58b1e8c1 100644 --- a/client/app/scripts/utils/topology-utils.js +++ b/client/app/scripts/utils/topology-utils.js @@ -1,5 +1,5 @@ import _ from 'lodash'; -import { is as isDeepEqual, Map as makeMap, Set as makeSet, List as makeList } from 'immutable'; +import { Map as makeMap, Set as makeSet, List as makeList } from 'immutable'; // From 9ce399607a87db5353c48924276876f00b76a9b6 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Mon, 5 Sep 2016 12:14:09 +0200 Subject: [PATCH 03/11] This works! Look into getting redux in there next --- client/app/scripts/charts/nodes-chart.js | 281 +++++++++++++---------- 1 file changed, 164 insertions(+), 117 deletions(-) diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index 95ba8a467..fdce656aa 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -3,7 +3,7 @@ import d3 from 'd3'; import debug from 'debug'; import React from 'react'; import { connect } from 'react-redux'; -import { Map as makeMap, fromJS, is as isDeepEqual } from 'immutable'; +import { Map as makeMap, fromJS, is } from 'immutable'; import timely from 'timely'; import { clickBackground } from '../actions/app-actions'; @@ -23,6 +23,125 @@ const radiusDensity = d3.scale.threshold() .domain([3, 6]) .range([2.5, 3.5, 3]); + +function toNodes(nodes) { + return nodes.map((node, id) => makeMap({ + id, + label: node.get('label'), + pseudo: node.get('pseudo'), + subLabel: node.get('label_minor'), + nodeCount: node.get('node_count'), + metrics: node.get('metrics'), + rank: node.get('rank'), + shape: node.get('shape'), + stack: node.get('stack'), + networks: node.get('networks'), + })); +} + + +function identityPresevingMerge(a, b) { + // + // merge two maps, if the values are equal return the old value to preserve (a === a') + // + // Note: mergeDeep keeps identity but we can't always use that. E.g. if nodes have been removed in + // b but still exist in a. they will still exist in the result. + // + return a.mergeWith((v1, v2) => is(v1, v2) ? v1 : v2, a, b); +} + + +function mergeDeepIfExists(mapA, mapB) { + // + // Does a deep merge on any key that exists in the first map + // + return mapA.map((v, k) => v.mergeDeep(mapB.get(k))); +} + + +function getLayoutNodes(nodes) { + return nodes.map(n => makeMap({ + id: n.get('id'), + adjacency: n.get('adjacency'), + })); +} + + +function initEdges(nodes) { + let edges = makeMap(); + + nodes.forEach((node, nodeId) => { + const adjacency = node.get('adjacency'); + if (adjacency) { + adjacency.forEach(adjacent => { + const edge = [nodeId, adjacent]; + const edgeId = edge.join(EDGE_ID_SEPARATOR); + + if (!edges.has(edgeId)) { + const source = edge[0]; + const target = edge[1]; + if (nodes.has(source) && nodes.has(target)) { + edges = edges.set(edgeId, makeMap({ + id: edgeId, + value: 1, + source, + target + })); + } + } + }); + } + }); + + return edges; +} + + +function getNodeScale(nodes, width, height) { + const expanse = Math.min(height, width); + const nodeSize = expanse / 3; // single node should fill a third of the screen + const maxNodeSize = Math.min(MAX_NODE_SIZE, expanse / 10); + const normalizedNodeSize = Math.max(MIN_NODE_SIZE, + Math.min(nodeSize / Math.sqrt(nodes.size), maxNodeSize)); + + return d3.scale.linear().range([0, normalizedNodeSize]); +} + + +function updateLayout({width, height, margins, topologyId, topologyOptions, forceRelayout, + nodes }) { + const nodeScale = getNodeScale(nodes, width, height); + const edges = initEdges(nodes); + + const options = { + width, + height, + margins: margins.toJS(), + forceRelayout, + topologyId, + topologyOptions: topologyOptions.toJS(), + scale: nodeScale, + }; + + const timedLayouter = timely(doLayout); + const graph = timedLayouter(nodes, edges, options); + + log(`graph layout took ${timedLayouter.time}ms`); + + // extract coords and save for restore + const layoutNodes = graph.nodes.map(node => makeMap({ + x: node.get('x'), + px: node.get('x'), + y: node.get('y'), + py: node.get('y') + })); + + const layoutEdges = graph.edges + .map(edge => edge.set('ppoints', edge.get('points'))); + + return { layoutNodes, layoutEdges, graph, nodeScale }; +} + class NodesChart extends React.Component { constructor(props, context) { @@ -42,7 +161,10 @@ class NodesChart extends React.Component { hasZoomed: false, height: props.height || 0, width: props.width || 0, - zoomCache: {} + zoomCache: {}, + + layoutInput: makeMap(), + layoutNodes: makeMap(), }; } @@ -81,9 +203,7 @@ class NodesChart extends React.Component { state.height = nextProps.forceRelayout ? nextProps.height : (state.height || nextProps.height); state.width = nextProps.forceRelayout ? nextProps.width : (state.width || nextProps.width); - if (nextProps.forceRelayout || nextProps.nodes !== this.props.nodes) { - _.assign(state, this.updateGraphState(nextProps, state)); - } + _.assign(state, this.updateGraphState(nextProps, state)); if (this.props.selectedNodeId !== nextProps.selectedNodeId) { _.assign(state, this.restoreLayout(state)); @@ -132,8 +252,12 @@ class NodesChart extends React.Component { - @@ -149,64 +273,6 @@ class NodesChart extends React.Component { } } - initNodes(topology, stateNodes) { - let nextStateNodes = stateNodes; - - // remove nodes that have disappeared - stateNodes.forEach((node, id) => { - if (!topology.has(id)) { - nextStateNodes = nextStateNodes.delete(id); - } - }); - - // copy relevant fields to state nodes - topology.forEach((node, id) => { - nextStateNodes = nextStateNodes.mergeIn([id], makeMap({ - id, - label: node.get('label'), - pseudo: node.get('pseudo'), - subLabel: node.get('label_minor'), - nodeCount: node.get('node_count'), - metrics: node.get('metrics'), - rank: node.get('rank'), - shape: node.get('shape'), - stack: node.get('stack'), - networks: node.get('networks'), - })); - }); - - return nextStateNodes; - } - - initEdges(topology, stateNodes) { - let edges = makeMap(); - - topology.forEach((node, nodeId) => { - const adjacency = node.get('adjacency'); - if (adjacency) { - adjacency.forEach(adjacent => { - const edge = [nodeId, adjacent]; - const edgeId = edge.join(EDGE_ID_SEPARATOR); - - if (!edges.has(edgeId)) { - const source = edge[0]; - const target = edge[1]; - if (stateNodes.has(source) && stateNodes.has(target)) { - edges = edges.set(edgeId, makeMap({ - id: edgeId, - value: 1, - source, - target - })); - } - } - }); - } - }); - - return edges; - } - centerSelectedNode(props, state) { let stateNodes = state.nodes; let stateEdges = state.edges; @@ -272,7 +338,7 @@ class NodesChart extends React.Component { }); // auto-scale node size for selected nodes - const selectedNodeScale = this.getNodeScale(adjacentNodes, state.width, state.height); + const selectedNodeScale = getNodeScale(adjacentNodes, state.width, state.height); return { selectedNodeScale, @@ -309,71 +375,52 @@ class NodesChart extends React.Component { }; } - const stateNodes = this.initNodes(props.nodes, state.nodes); - const stateEdges = this.initEdges(props.nodes, stateNodes); - const nodeScale = this.getNodeScale(props.nodes, state.width, state.height); - const nextState = { nodeScale }; + const nextState = { nodes: toNodes(props.nodes) }; - const options = { + // + // pull this out into redux. + // + const layoutInput = identityPresevingMerge(state.layoutInput, { width: state.width, height: state.height, - scale: nodeScale, - margins: props.margins, + nodes: getLayoutNodes(props.nodes), + margins: fromJS(props.margins), + topologyId: props.topologyId, + topologyOptions: fromJS(props.topologyOptions), forceRelayout: props.forceRelayout, - topologyId: this.props.topologyId, - topologyOptions: this.props.topologyOptions - }; + }); - const timedLayouter = timely(doLayout); - const graph = timedLayouter(stateNodes, stateEdges, options); + // layout input hasn't changed. + // TODO: move this out into reselect + if (state.layoutInput !== layoutInput) { + nextState.layoutInput = layoutInput; - log(`graph layout took ${timedLayouter.time}ms`); + const { layoutNodes, layoutEdges, graph, nodeScale } = updateLayout(layoutInput.toObject()); + // + // adjust layout based on viewport + const xFactor = (state.width - props.margins.left - props.margins.right) / graph.width; + const yFactor = state.height / graph.height; + const zoomFactor = Math.min(xFactor, yFactor); + let zoomScale = state.scale; - // extract coords and save for restore - const graphNodes = graph.nodes.map(node => makeMap({ - x: node.get('x'), - px: node.get('x'), - y: node.get('y'), - py: node.get('y') - })); + if (this.zoom && !state.hasZoomed && zoomFactor > 0 && zoomFactor < 1) { + zoomScale = zoomFactor; + // saving in d3's behavior cache + this.zoom.scale(zoomFactor); + } - const layoutNodes = stateNodes.mergeDeep(graphNodes); - - const layoutEdges = graph.edges - .map(edge => edge.set('ppoints', edge.get('points'))); - - // adjust layout based on viewport - const xFactor = (state.width - props.margins.left - props.margins.right) / graph.width; - const yFactor = state.height / graph.height; - const zoomFactor = Math.min(xFactor, yFactor); - let zoomScale = this.state.scale; - - if (this.zoom && !state.hasZoomed && zoomFactor > 0 && zoomFactor < 1) { - zoomScale = zoomFactor; - // saving in d3's behavior cache - this.zoom.scale(zoomFactor); - } - - nextState.scale = zoomScale; - if (!isDeepEqual(layoutNodes, state.nodes)) { - nextState.nodes = layoutNodes; - } - if (!isDeepEqual(layoutEdges, state.edges)) { + nextState.scale = zoomScale; nextState.edges = layoutEdges; + nextState.nodeScale = nodeScale; + nextState.layoutNodes = layoutNodes; } + nextState.nodes = mergeDeepIfExists(nextState.nodes, + (nextState.layoutNodes || state.layoutNodes)); + return nextState; } - getNodeScale(nodes, width, height) { - const expanse = Math.min(height, width); - const nodeSize = expanse / 3; // single node should fill a third of the screen - const maxNodeSize = Math.min(MAX_NODE_SIZE, expanse / 10); - const normalizedNodeSize = Math.max(MIN_NODE_SIZE, - Math.min(nodeSize / Math.sqrt(nodes.size), maxNodeSize)); - return this.state.nodeScale.copy().range([0, normalizedNodeSize]); - } - zoomed() { // debug('zoomed', d3.event.scale, d3.event.translate); this.isZooming = true; From 4b7471b1b01fd7f6fbef5c92c687c95d1dfd8f00 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 6 Sep 2016 12:31:01 +0200 Subject: [PATCH 04/11] things working again, on the way to reselect! --- .../scripts/charts/nodes-chart-elements.js | 19 +++- client/app/scripts/charts/nodes-chart.js | 98 +++++++------------ client/app/scripts/components/nodes.js | 5 +- .../app/scripts/selectors/chartSelectors.js | 61 ++++++++++++ client/package.json | 1 + 5 files changed, 117 insertions(+), 67 deletions(-) create mode 100644 client/app/scripts/selectors/chartSelectors.js diff --git a/client/app/scripts/charts/nodes-chart-elements.js b/client/app/scripts/charts/nodes-chart-elements.js index 7d921f0a8..19b1aa996 100644 --- a/client/app/scripts/charts/nodes-chart-elements.js +++ b/client/app/scripts/charts/nodes-chart-elements.js @@ -1,6 +1,7 @@ import React from 'react'; import { connect } from 'react-redux'; +import { completeNodesSelector } from '../selectors/chartSelectors'; import NodesChartEdges from './nodes-chart-edges'; import NodesChartNodes from './nodes-chart-nodes'; @@ -9,14 +10,24 @@ class NodesChartElements extends React.Component { const props = this.props; return ( - - ); } } -export default connect()(NodesChartElements); +function mapStateToProps(state, props) { + return { + completeNodes: completeNodesSelector(state, props) + }; +} + +export default connect(mapStateToProps)(NodesChartElements); diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index fdce656aa..ca36943af 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -24,22 +24,6 @@ const radiusDensity = d3.scale.threshold() .range([2.5, 3.5, 3]); -function toNodes(nodes) { - return nodes.map((node, id) => makeMap({ - id, - label: node.get('label'), - pseudo: node.get('pseudo'), - subLabel: node.get('label_minor'), - nodeCount: node.get('node_count'), - metrics: node.get('metrics'), - rank: node.get('rank'), - shape: node.get('shape'), - stack: node.get('stack'), - networks: node.get('networks'), - })); -} - - function identityPresevingMerge(a, b) { // // merge two maps, if the values are equal return the old value to preserve (a === a') @@ -51,14 +35,6 @@ function identityPresevingMerge(a, b) { } -function mergeDeepIfExists(mapA, mapB) { - // - // Does a deep merge on any key that exists in the first map - // - return mapA.map((v, k) => v.mergeDeep(mapB.get(k))); -} - - function getLayoutNodes(nodes) { return nodes.map(n => makeMap({ id: n.get('id'), @@ -97,12 +73,13 @@ function initEdges(nodes) { } -function getNodeScale(nodes, width, height) { +function getNodeScale(nodesCount, width, height) { + console.log(nodesCount, width, height); const expanse = Math.min(height, width); const nodeSize = expanse / 3; // single node should fill a third of the screen const maxNodeSize = Math.min(MAX_NODE_SIZE, expanse / 10); const normalizedNodeSize = Math.max(MIN_NODE_SIZE, - Math.min(nodeSize / Math.sqrt(nodes.size), maxNodeSize)); + Math.min(nodeSize / Math.sqrt(nodesCount), maxNodeSize)); return d3.scale.linear().range([0, normalizedNodeSize]); } @@ -110,7 +87,7 @@ function getNodeScale(nodes, width, height) { function updateLayout({width, height, margins, topologyId, topologyOptions, forceRelayout, nodes }) { - const nodeScale = getNodeScale(nodes, width, height); + const nodeScale = getNodeScale(nodes.size, width, height); const edges = initEdges(nodes); const options = { @@ -119,7 +96,7 @@ function updateLayout({width, height, margins, topologyId, topologyOptions, forc margins: margins.toJS(), forceRelayout, topologyId, - topologyOptions: topologyOptions.toJS(), + topologyOptions: (topologyOptions && topologyOptions.toJS()), scale: nodeScale, }; @@ -128,20 +105,21 @@ function updateLayout({width, height, margins, topologyId, topologyOptions, forc log(`graph layout took ${timedLayouter.time}ms`); - // extract coords and save for restore const layoutNodes = graph.nodes.map(node => makeMap({ x: node.get('x'), - px: node.get('x'), y: node.get('y'), + // extract coords and save for restore + px: node.get('x'), py: node.get('y') })); const layoutEdges = graph.edges .map(edge => edge.set('ppoints', edge.get('points'))); - return { layoutNodes, layoutEdges, graph, nodeScale }; + return { layoutNodes, layoutEdges, width: graph.width, height: graph.height }; } + class NodesChart extends React.Component { constructor(props, context) { @@ -244,6 +222,7 @@ class NodesChart extends React.Component { const translate = [panTranslateX, panTranslateY]; const transform = `translate(${translate}) scale(${scale})`; const svgClassNames = this.props.isEmpty ? 'hide' : ''; + console.log('nodes-chart.render'); return (
@@ -338,7 +317,7 @@ class NodesChart extends React.Component { }); // auto-scale node size for selected nodes - const selectedNodeScale = getNodeScale(adjacentNodes, state.width, state.height); + const selectedNodeScale = getNodeScale(adjacentNodes.size, state.width, state.height); return { selectedNodeScale, @@ -375,11 +354,6 @@ class NodesChart extends React.Component { }; } - const nextState = { nodes: toNodes(props.nodes) }; - - // - // pull this out into redux. - // const layoutInput = identityPresevingMerge(state.layoutInput, { width: state.width, height: state.height, @@ -391,34 +365,32 @@ class NodesChart extends React.Component { }); // layout input hasn't changed. - // TODO: move this out into reselect - if (state.layoutInput !== layoutInput) { - nextState.layoutInput = layoutInput; - - const { layoutNodes, layoutEdges, graph, nodeScale } = updateLayout(layoutInput.toObject()); - // - // adjust layout based on viewport - const xFactor = (state.width - props.margins.left - props.margins.right) / graph.width; - const yFactor = state.height / graph.height; - const zoomFactor = Math.min(xFactor, yFactor); - let zoomScale = state.scale; - - if (this.zoom && !state.hasZoomed && zoomFactor > 0 && zoomFactor < 1) { - zoomScale = zoomFactor; - // saving in d3's behavior cache - this.zoom.scale(zoomFactor); - } - - nextState.scale = zoomScale; - nextState.edges = layoutEdges; - nextState.nodeScale = nodeScale; - nextState.layoutNodes = layoutNodes; + // TODO: move this out into reselect (relies on `state` a bit right now which makes it tricky) + if (state.layoutInput === layoutInput) { + return {}; } - nextState.nodes = mergeDeepIfExists(nextState.nodes, - (nextState.layoutNodes || state.layoutNodes)); + const { layoutNodes, layoutEdges, width, height } = updateLayout(layoutInput.toObject()); + // + // adjust layout based on viewport + const xFactor = (state.width - props.margins.left - props.margins.right) / width; + const yFactor = state.height / height; + const zoomFactor = Math.min(xFactor, yFactor); + let zoomScale = state.scale; - return nextState; + if (this.zoom && !state.hasZoomed && zoomFactor > 0 && zoomFactor < 1) { + zoomScale = zoomFactor; + // saving in d3's behavior cache + this.zoom.scale(zoomFactor); + } + + return { + layoutInput, + scale: zoomScale, + nodes: layoutNodes, + edges: layoutEdges, + nodeScale: getNodeScale(props.nodes.size, state.width, state.height), + }; } zoomed() { @@ -436,6 +408,7 @@ class NodesChart extends React.Component { } } + function mapStateToProps(state) { return { adjacentNodes: getAdjacentNodes(state), @@ -446,6 +419,7 @@ function mapStateToProps(state) { }; } + export default connect( mapStateToProps, { clickBackground } diff --git a/client/app/scripts/components/nodes.js b/client/app/scripts/components/nodes.js index e9632af3c..c96a9f22e 100644 --- a/client/app/scripts/components/nodes.js +++ b/client/app/scripts/components/nodes.js @@ -8,6 +8,7 @@ import { DelayedShow } from '../utils/delayed-show'; import { Loading, getNodeType } from './loading'; import { isTopologyEmpty } from '../utils/topology-utils'; import { CANVAS_MARGINS } from '../constants/styles'; +import { nodesSelector } from '../selectors/chartSelectors'; const navbarHeight = 194; const marginTop = 0; @@ -71,6 +72,8 @@ class Nodes extends React.Component { currentTopology } = this.props; const layoutPrecision = getLayoutPrecision(nodes.size); + console.log('nodes.render'); + return (
@@ -113,7 +116,7 @@ function mapStateToProps(state) { return { currentTopology: state.get('currentTopology'), gridMode: state.get('gridMode'), - nodes: state.get('nodes').filter(node => !node.get('filtered')), + nodes: nodesSelector(state), nodesLoaded: state.get('nodesLoaded'), topologies: state.get('topologies'), topologiesLoaded: state.get('topologiesLoaded'), diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js new file mode 100644 index 000000000..9ca929b53 --- /dev/null +++ b/client/app/scripts/selectors/chartSelectors.js @@ -0,0 +1,61 @@ +import { createSelector } from 'reselect'; +import { Map as makeMap } from 'immutable'; + + +const allNodesSelector = state => state.get('nodes'); + + +export const nodesSelector = createSelector( + allNodesSelector, + (allNodes) => allNodes.filter(node => !node.get('filtered')) +); + + +export const nodeAdjacenciesSelector = createSelector( + nodesSelector, + (nodes) => nodes.map(n => makeMap({ + id: n.get('id'), + adjacency: n.get('adjacency'), + })) +); + + +export const layoutNodesSelector = (_, props) => props.layoutNodes; + + +export const dataNodesSelector = createSelector( + nodesSelector, + (nodes) => nodes.map((node, id) => makeMap({ + id, + label: node.get('label'), + pseudo: node.get('pseudo'), + subLabel: node.get('label_minor'), + nodeCount: node.get('node_count'), + metrics: node.get('metrics'), + rank: node.get('rank'), + shape: node.get('shape'), + stack: node.get('stack'), + networks: node.get('networks'), + })) +); + + +function mergeDeepIfExists(mapA, mapB) { + // + // Does a deep merge on any key that exists in the first map + // + return mapA.map((v, k) => v.mergeDeep(mapB.get(k))); +} + + +export const completeNodesSelector = createSelector( + layoutNodesSelector, + dataNodesSelector, + (layoutNodes, dataNodes) => { + if (!layoutNodes || layoutNodes.size === 0) { + return makeMap(); + } + + return mergeDeepIfExists(dataNodes, layoutNodes); + } +); diff --git a/client/package.json b/client/package.json index f2d2a888e..207f383e1 100644 --- a/client/package.json +++ b/client/package.json @@ -29,6 +29,7 @@ "redux-logger": "2.6.1", "redux-thunk": "2.0.1", "reqwest": "~2.0.5", + "reselect": "^2.5.3", "timely": "0.1.0", "whatwg-fetch": "0.11.0" }, From a1b8e963dc08ef5e1f98d3c2df6c92c776950bd6 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 6 Sep 2016 16:06:19 +0200 Subject: [PATCH 05/11] nodes-chart only re-rendered on prop updates!!! --- client/app/scripts/charts/node-container.js | 2 ++ client/app/scripts/charts/nodes-chart.js | 29 ++++++++++++++-- client/app/scripts/charts/nodes-grid.js | 2 ++ client/app/scripts/components/app.js | 2 ++ client/app/scripts/components/nodes.js | 26 +------------- .../app/scripts/selectors/chartSelectors.js | 34 +++++++++++++++++-- client/app/scripts/utils/topology-utils.js | 1 + 7 files changed, 65 insertions(+), 31 deletions(-) diff --git a/client/app/scripts/charts/node-container.js b/client/app/scripts/charts/node-container.js index 74820303d..9e6b5fb66 100644 --- a/client/app/scripts/charts/node-container.js +++ b/client/app/scripts/charts/node-container.js @@ -13,6 +13,8 @@ class NodeContainer extends React.Component { const scaleFactor = focused ? (1 / zoomScale) : 1; const other = _.omit(this.props, 'dx', 'dy'); + console.log('nodecontainer.render'); + return ( = 50) { + precision = 0; + } else if (nodesCount > 20) { + precision = 1; + } else if (nodesCount > 10) { + precision = 2; + } else { + precision = 3; + } + + return precision; +} + function identityPresevingMerge(a, b) { // @@ -152,6 +172,7 @@ class NodesChart extends React.Component { } componentWillReceiveProps(nextProps) { + console.log('componentWillReceiveProps', diff(nextProps, this.props), nextProps); // gather state, setState should be called only once here const state = _.assign({}, this.state); @@ -224,6 +245,7 @@ class NodesChart extends React.Component { const svgClassNames = this.props.isEmpty ? 'hide' : ''; console.log('nodes-chart.render'); + const layoutPrecision = getLayoutPrecision(nodes.size); return (
+ layoutPrecision={layoutPrecision} />
); @@ -411,7 +433,8 @@ class NodesChart extends React.Component { function mapStateToProps(state) { return { - adjacentNodes: getAdjacentNodes(state), + nodes: nodeAdjacenciesSelector(state), + adjacentNodes: adjacentNodesSelector(state), forceRelayout: state.get('forceRelayout'), selectedNodeId: state.get('selectedNodeId'), topologyId: state.get('currentTopologyId'), diff --git a/client/app/scripts/charts/nodes-grid.js b/client/app/scripts/charts/nodes-grid.js index e05172f7d..6456c0224 100644 --- a/client/app/scripts/charts/nodes-grid.js +++ b/client/app/scripts/charts/nodes-grid.js @@ -5,6 +5,7 @@ import { connect } from 'react-redux'; import { List as makeList, Map as makeMap } from 'immutable'; import NodeDetailsTable from '../components/node-details/node-details-table'; import { clickNode, sortOrderChanged } from '../actions/app-actions'; +import { nodesSelector } from '../selectors/chartSelectors'; import { getNodeColor } from '../utils/color-utils'; @@ -142,6 +143,7 @@ class NodesGrid extends React.Component { function mapStateToProps(state) { return { + nodes: nodesSelector(state), gridSortBy: state.get('gridSortBy'), gridSortedDesc: state.get('gridSortedDesc'), currentTopology: state.get('currentTopology'), diff --git a/client/app/scripts/components/app.js b/client/app/scripts/components/app.js index b324ce059..3621f64e9 100644 --- a/client/app/scripts/components/app.js +++ b/client/app/scripts/components/app.js @@ -142,6 +142,7 @@ class App extends React.Component { } } + function mapStateToProps(state) { return { activeTopologyOptions: getActiveTopologyOptions(state), @@ -158,6 +159,7 @@ function mapStateToProps(state) { }; } + export default connect( mapStateToProps )(App); diff --git a/client/app/scripts/components/nodes.js b/client/app/scripts/components/nodes.js index c96a9f22e..385d64910 100644 --- a/client/app/scripts/components/nodes.js +++ b/client/app/scripts/components/nodes.js @@ -8,30 +8,11 @@ import { DelayedShow } from '../utils/delayed-show'; import { Loading, getNodeType } from './loading'; import { isTopologyEmpty } from '../utils/topology-utils'; import { CANVAS_MARGINS } from '../constants/styles'; -import { nodesSelector } from '../selectors/chartSelectors'; const navbarHeight = 194; const marginTop = 0; -/** - * dynamic coords precision based on topology size - */ -function getLayoutPrecision(nodesCount) { - let precision; - if (nodesCount >= 50) { - precision = 0; - } else if (nodesCount > 20) { - precision = 1; - } else if (nodesCount > 10) { - precision = 2; - } else { - precision = 3; - } - - return precision; -} - class Nodes extends React.Component { constructor(props, context) { super(props, context); @@ -68,9 +49,8 @@ class Nodes extends React.Component { } render() { - const { nodes, topologyEmpty, gridMode, topologiesLoaded, nodesLoaded, topologies, + const { topologyEmpty, gridMode, topologiesLoaded, nodesLoaded, topologies, currentTopology } = this.props; - const layoutPrecision = getLayoutPrecision(nodes.size); console.log('nodes.render'); @@ -87,13 +67,10 @@ class Nodes extends React.Component { {gridMode ? : }
); @@ -116,7 +93,6 @@ function mapStateToProps(state) { return { currentTopology: state.get('currentTopology'), gridMode: state.get('gridMode'), - nodes: nodesSelector(state), nodesLoaded: state.get('nodesLoaded'), topologies: state.get('topologies'), topologiesLoaded: state.get('topologiesLoaded'), diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 9ca929b53..9d730c96a 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -1,5 +1,15 @@ -import { createSelector } from 'reselect'; -import { Map as makeMap } from 'immutable'; +import { createSelector, createSelectorCreator, defaultMemoize } from 'reselect'; +import { Map as makeMap, is } from 'immutable'; + +import { getAdjacentNodes } from '../utils/topology-utils'; + + +// imm createSelector +// +const imCreateSelector = createSelectorCreator( + defaultMemoize, + is +); const allNodesSelector = state => state.get('nodes'); @@ -11,7 +21,7 @@ export const nodesSelector = createSelector( ); -export const nodeAdjacenciesSelector = createSelector( +export const _nodeAdjacenciesSelector = createSelector( nodesSelector, (nodes) => nodes.map(n => makeMap({ id: n.get('id'), @@ -20,6 +30,24 @@ export const nodeAdjacenciesSelector = createSelector( ); +export const nodeAdjacenciesSelector = imCreateSelector( + _nodeAdjacenciesSelector, + (nodes) => nodes +); + + +const _adjacentNodesSelector = createSelector( + getAdjacentNodes, + (ns) => ns +); + + +export const adjacentNodesSelector = imCreateSelector( + _adjacentNodesSelector, + (adjacentNodes) => adjacentNodes +); + + export const layoutNodesSelector = (_, props) => props.layoutNodes; diff --git a/client/app/scripts/utils/topology-utils.js b/client/app/scripts/utils/topology-utils.js index f58b1e8c1..975943598 100644 --- a/client/app/scripts/utils/topology-utils.js +++ b/client/app/scripts/utils/topology-utils.js @@ -143,6 +143,7 @@ export function isTopologyEmpty(state) { && state.get('nodes').size === 0; } + export function getAdjacentNodes(state, originNodeId) { let adjacentNodes = makeSet(); const nodeId = originNodeId || state.get('selectedNodeId'); From fc95e1efa0bc42768508678d1f745516f0598473 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 6 Sep 2016 17:00:24 +0200 Subject: [PATCH 06/11] Fixes selected layout! (was the removal of :id from the node objects) Only show errors in webpack output! Tidying up More tidying more tidying More fiddling around wip wip wip Fixes forceLayout rm console.log --- client/app/scripts/charts/node-container.js | 2 - client/app/scripts/charts/nodes-chart.js | 84 +++++-------------- .../app/scripts/components/debug-toolbar.js | 14 +++- client/app/scripts/components/nodes.js | 2 - .../app/scripts/selectors/chartSelectors.js | 79 +++++++++++------ client/server.js | 2 +- client/test/actions/90-nodes-select.js | 65 +++++++------- client/test/browser-perf/main.js | 2 +- client/test/run-jankie.sh | 14 +++- 9 files changed, 129 insertions(+), 135 deletions(-) diff --git a/client/app/scripts/charts/node-container.js b/client/app/scripts/charts/node-container.js index 9e6b5fb66..74820303d 100644 --- a/client/app/scripts/charts/node-container.js +++ b/client/app/scripts/charts/node-container.js @@ -13,8 +13,6 @@ class NodeContainer extends React.Component { const scaleFactor = focused ? (1 / zoomScale) : 1; const other = _.omit(this.props, 'dx', 'dy'); - console.log('nodecontainer.render'); - return ( is(v1, v2) ? v1 : v2, a, b); -} - - -function getLayoutNodes(nodes) { - return nodes.map(n => makeMap({ - id: n.get('id'), - adjacency: n.get('adjacency'), - })); -} - - function initEdges(nodes) { let edges = makeMap(); @@ -94,7 +74,6 @@ function initEdges(nodes) { function getNodeScale(nodesCount, width, height) { - console.log(nodesCount, width, height); const expanse = Math.min(height, width); const nodeSize = expanse / 3; // single node should fill a third of the screen const maxNodeSize = Math.min(MAX_NODE_SIZE, expanse / 10); @@ -105,20 +84,13 @@ function getNodeScale(nodesCount, width, height) { } -function updateLayout({width, height, margins, topologyId, topologyOptions, forceRelayout, - nodes }) { +function updateLayout(width, height, nodes, baseOptions) { const nodeScale = getNodeScale(nodes.size, width, height); const edges = initEdges(nodes); - const options = { - width, - height, - margins: margins.toJS(), - forceRelayout, - topologyId, - topologyOptions: (topologyOptions && topologyOptions.toJS()), + const options = Object.assign({}, baseOptions, { scale: nodeScale, - }; + }); const timedLayouter = timely(doLayout); const graph = timedLayouter(nodes, edges, options); @@ -136,7 +108,7 @@ function updateLayout({width, height, margins, topologyId, topologyOptions, forc const layoutEdges = graph.edges .map(edge => edge.set('ppoints', edge.get('points'))); - return { layoutNodes, layoutEdges, width: graph.width, height: graph.height }; + return { layoutNodes, layoutEdges, layoutWidth: graph.width, layoutHeight: graph.height }; } @@ -160,9 +132,6 @@ class NodesChart extends React.Component { height: props.height || 0, width: props.width || 0, zoomCache: {}, - - layoutInput: makeMap(), - layoutNodes: makeMap(), }; } @@ -172,7 +141,6 @@ class NodesChart extends React.Component { } componentWillReceiveProps(nextProps) { - console.log('componentWillReceiveProps', diff(nextProps, this.props), nextProps); // gather state, setState should be called only once here const state = _.assign({}, this.state); @@ -202,7 +170,9 @@ class NodesChart extends React.Component { state.height = nextProps.forceRelayout ? nextProps.height : (state.height || nextProps.height); state.width = nextProps.forceRelayout ? nextProps.width : (state.width || nextProps.width); - _.assign(state, this.updateGraphState(nextProps, state)); + if (nextProps.forceRelayout || nextProps.nodes !== this.props.nodes) { + _.assign(state, this.updateGraphState(nextProps, state)); + } if (this.props.selectedNodeId !== nextProps.selectedNodeId) { _.assign(state, this.restoreLayout(state)); @@ -243,7 +213,6 @@ class NodesChart extends React.Component { const translate = [panTranslateX, panTranslateY]; const transform = `translate(${translate}) scale(${scale})`; const svgClassNames = this.props.isEmpty ? 'hide' : ''; - console.log('nodes-chart.render'); const layoutPrecision = getLayoutPrecision(nodes.size); return ( @@ -277,9 +246,7 @@ class NodesChart extends React.Component { centerSelectedNode(props, state) { let stateNodes = state.nodes; let stateEdges = state.edges; - const selectedLayoutNode = stateNodes.get(props.selectedNodeId); - - if (!selectedLayoutNode) { + if (!stateNodes.has(props.selectedNodeId)) { return {}; } @@ -310,8 +277,8 @@ class NodesChart extends React.Component { const radius = Math.min(state.width, state.height) / density / zoomScale; const offsetAngle = Math.PI / 4; - stateNodes = stateNodes.map((node) => { - const index = adjacentLayoutNodeIds.indexOf(node.get('id')); + stateNodes = stateNodes.map((node, nodeId) => { + const index = adjacentLayoutNodeIds.indexOf(nodeId); if (index > -1) { const angle = offsetAngle + Math.PI * 2 * index / adjacentCount; return node.merge({ @@ -324,8 +291,8 @@ class NodesChart extends React.Component { // fix all edges for circular nodes stateEdges = stateEdges.map(edge => { - if (edge.get('source') === selectedLayoutNode.get('id') - || edge.get('target') === selectedLayoutNode.get('id') + if (edge.get('source') === props.selectedNodeId + || edge.get('target') === props.selectedNodeId || _.includes(adjacentLayoutNodeIds, edge.get('source')) || _.includes(adjacentLayoutNodeIds, edge.get('target'))) { const source = stateNodes.get(edge.get('source')); @@ -376,27 +343,21 @@ class NodesChart extends React.Component { }; } - const layoutInput = identityPresevingMerge(state.layoutInput, { + const options = { width: state.width, height: state.height, - nodes: getLayoutNodes(props.nodes), - margins: fromJS(props.margins), - topologyId: props.topologyId, - topologyOptions: fromJS(props.topologyOptions), + margins: props.margins, forceRelayout: props.forceRelayout, - }); + topologyId: props.topologyId, + topologyOptions: props.topologyOptions, + }; - // layout input hasn't changed. - // TODO: move this out into reselect (relies on `state` a bit right now which makes it tricky) - if (state.layoutInput === layoutInput) { - return {}; - } - - const { layoutNodes, layoutEdges, width, height } = updateLayout(layoutInput.toObject()); + const { layoutNodes, layoutEdges, layoutWidth, layoutHeight } = updateLayout( + state.width, state.height, props.nodes, options); // // adjust layout based on viewport - const xFactor = (state.width - props.margins.left - props.margins.right) / width; - const yFactor = state.height / height; + const xFactor = (state.width - props.margins.left - props.margins.right) / layoutWidth; + const yFactor = state.height / layoutHeight; const zoomFactor = Math.min(xFactor, yFactor); let zoomScale = state.scale; @@ -407,7 +368,6 @@ class NodesChart extends React.Component { } return { - layoutInput, scale: zoomScale, nodes: layoutNodes, edges: layoutEdges, diff --git a/client/app/scripts/components/debug-toolbar.js b/client/app/scripts/components/debug-toolbar.js index ae61fff59..e1574574b 100644 --- a/client/app/scripts/components/debug-toolbar.js +++ b/client/app/scripts/components/debug-toolbar.js @@ -178,7 +178,7 @@ class DebugToolbar extends React.Component { } setLoading(loading) { - this.props.setAppState(state => state.set('topologiesLoaded', !loading)); + this.props.dispatch(setAppState(state => state.set('topologiesLoaded', !loading))); } addNodes(n, prefix = 'zing') { @@ -204,6 +204,14 @@ class DebugToolbar extends React.Component { log('added nodes', n); } + removeNode() { + const ns = this.props.nodes; + const nodeNames = ns.keySeq().toJS(); + this.props.dispatch(receiveNodesDelta({ + remove: [nodeNames[_.random(nodeNames.length - 1)]] + })); + } + render() { const { availableCanvasMetrics } = this.props; @@ -220,6 +228,7 @@ class DebugToolbar extends React.Component { Metric Variants +
@@ -289,6 +298,5 @@ function mapStateToProps(state) { export default connect( - mapStateToProps, - {setAppState} + mapStateToProps )(DebugToolbar); diff --git a/client/app/scripts/components/nodes.js b/client/app/scripts/components/nodes.js index 385d64910..20490c321 100644 --- a/client/app/scripts/components/nodes.js +++ b/client/app/scripts/components/nodes.js @@ -52,8 +52,6 @@ class Nodes extends React.Component { const { topologyEmpty, gridMode, topologiesLoaded, nodesLoaded, topologies, currentTopology } = this.props; - console.log('nodes.render'); - return (
diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 9d730c96a..0d5456ae0 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -4,14 +4,18 @@ import { Map as makeMap, is } from 'immutable'; import { getAdjacentNodes } from '../utils/topology-utils'; -// imm createSelector // -const imCreateSelector = createSelectorCreator( +// "immutable" createSelector +// +const createDeepEqualSelector = createSelectorCreator( defaultMemoize, is ); +const identity = v => v; + + const allNodesSelector = state => state.get('nodes'); @@ -21,36 +25,46 @@ export const nodesSelector = createSelector( ); -export const _nodeAdjacenciesSelector = createSelector( - nodesSelector, - (nodes) => nodes.map(n => makeMap({ - id: n.get('id'), - adjacency: n.get('adjacency'), - })) -); - - -export const nodeAdjacenciesSelector = imCreateSelector( - _nodeAdjacenciesSelector, - (nodes) => nodes -); - - -const _adjacentNodesSelector = createSelector( +// +// This is like an === cache... +// +// - getAdjacentNodes is run on every state change and can generate a new immutable object each +// time: +// - v1 = getAdjacentNodes(a) +// - v2 = getAdjacentNodes(a) +// - v1 !== v2 +// - is(v1, v2) === true +// +// - createDeepEqualSelector will wrap those calls with a: is(v1, v2) ? v1 : v2 +// - Thus you can compare consequtive calls to adjacentNodesSelector(state) with === (which is +// what redux is doing with connect() +// +// Note: this feels like the wrong way to be using reselect... +// +export const adjacentNodesSelector = createDeepEqualSelector( getAdjacentNodes, - (ns) => ns + identity ); -export const adjacentNodesSelector = imCreateSelector( - _adjacentNodesSelector, - (adjacentNodes) => adjacentNodes +// +// You what? What is going on here? +// +// We wrap the result of nodes.map in another equality test which discards the new value +// if it was the same as the old one. Again preserving === +// +export const nodeAdjacenciesSelector = createDeepEqualSelector( + createSelector( + nodesSelector, + (nodes) => nodes.map(n => makeMap({ + id: n.get('id'), + adjacency: n.get('adjacency'), + })) + ), + identity ); -export const layoutNodesSelector = (_, props) => props.layoutNodes; - - export const dataNodesSelector = createSelector( nodesSelector, (nodes) => nodes.map((node, id) => makeMap({ @@ -68,6 +82,10 @@ export const dataNodesSelector = createSelector( ); +// FIXME: this is a bit of a hack... +export const layoutNodesSelector = (_, props) => props.layoutNodes || makeMap(); + + function mergeDeepIfExists(mapA, mapB) { // // Does a deep merge on any key that exists in the first map @@ -76,14 +94,21 @@ function mergeDeepIfExists(mapA, mapB) { } -export const completeNodesSelector = createSelector( +const _completeNodesSelector = createSelector( layoutNodesSelector, dataNodesSelector, (layoutNodes, dataNodes) => { - if (!layoutNodes || layoutNodes.size === 0) { + if (layoutNodes.size === 0 || dataNodes.size === 0) { return makeMap(); } + // dataNodes might get updated before layoutNodes when a node is removed from the topo. return mergeDeepIfExists(dataNodes, layoutNodes); } ); + + +export const completeNodesSelector = createDeepEqualSelector( + _completeNodesSelector, + identity +); diff --git a/client/server.js b/client/server.js index a27ac30c7..3b984ba1f 100644 --- a/client/server.js +++ b/client/server.js @@ -66,7 +66,7 @@ if (process.env.NODE_ENV !== 'production') { hot: true, noInfo: true, historyApiFallback: true, - stats: { colors: true } + stats: 'errors-only', }).listen(4041, '0.0.0.0', function (err, result) { if (err) { console.log(err); diff --git a/client/test/actions/90-nodes-select.js b/client/test/actions/90-nodes-select.js index ed7c28313..5693ae01b 100644 --- a/client/test/actions/90-nodes-select.js +++ b/client/test/actions/90-nodes-select.js @@ -18,85 +18,84 @@ function clickIfVisible(list, index) { }); } + +function selectNode(browser) { + debug('select node'); + return browser.elementByCssSelector('.nodes-chart-nodes .node:nth-child(1) > g', function(err, el) { + return el.click(); + }); +} + + +function deselectNode(browser) { + debug('deselect node'); + return browser.elementByCssSelector('.fa-close', function(err, el) { + return el.click(); + }); +} + + module.exports = function(cfg) { - var startUrl = 'http://' + cfg.host + '/debug.html'; - var selectedUrl = 'http://' + cfg.host + '/debug.html#!/state/{"nodeDetails":[{"id":"zing11","label":"zing11","topologyId":"containers"}],"selectedNodeId":"zing11","topologyId":"containers","topologyOptions":{"processes":{"unconnected":"hide"},"processes-by-name":{"unconnected":"hide"},"containers":{"system":"hide","stopped":"hide"},"containers-by-hostname":{"system":"hide","stopped":"hide"},"containers-by-image":{"system":"hide","stopped":"hide"}}}'; - + var startUrl = 'http://' + cfg.host + '/'; // cfg - The configuration object. args, from the example above. return function(browser) { // browser is created using wd.promiseRemote() // More info about wd at https://github.com/admc/wd - return browser.get('http://' + cfg.host + '/debug.html') + return browser.get('http://' + cfg.host + '/') .then(function() { debug('starting run ' + cfg.run); return browser.sleep(2000); }) + .then(function() { + return browser.execute("localStorage.debugToolbar = 1;"); + }) + .then(function() { + return browser.sleep(5000); + }) .then(function() { return browser.elementByCssSelector('.debug-panel button:nth-child(5)'); // return browser.elementByCssSelector('.debug-panel div:nth-child(2) button:nth-child(9)'); }) .then(function(el) { debug('debug-panel found'); - return el.click(function() { - el.click(function() { - el.click(); - }); - }); + return el.click(); }) .then(function() { return browser.sleep(2000); }) - .then(function() { - return browser.sleep(2000); - }) - .then(function() { - debug('select node'); - return browser.get(selectedUrl); + return selectNode(browser); }) .then(function() { return browser.sleep(5000); }) .then(function() { - debug('deselect node'); - return browser.elementByCssSelector('.fa-close', function(err, el) { - return el.click(); - }); + return deselectNode(browser); }) - .then(function() { return browser.sleep(2000); }) .then(function() { - debug('select node'); - return browser.get(selectedUrl); + return selectNode(browser); }) .then(function() { return browser.sleep(5000); }) .then(function() { - debug('deselect node'); - return browser.elementByCssSelector('.fa-close', function(err, el) { - return el.click(); - }); + return deselectNode(browser); }) - .then(function() { return browser.sleep(2000); }) .then(function() { - debug('select node'); - return browser.get(selectedUrl); + return selectNode(browser); }) .then(function() { return browser.sleep(5000); }) .then(function() { - debug('deselect node'); - return browser.elementByCssSelector('.fa-close', function(err, el) { - return el.click(); - }); + return deselectNode(browser); }) .then(function() { diff --git a/client/test/browser-perf/main.js b/client/test/browser-perf/main.js index 039c6d3ad..8c042556e 100644 --- a/client/test/browser-perf/main.js +++ b/client/test/browser-perf/main.js @@ -3,7 +3,7 @@ var options = { selenium: 'http://local.docker:4444/wd/hub', actions: [require('./custom-action.js')()] } -browserPerf('http://local.docker:4040/debug.html', function(err, res){ +browserPerf('http://local.docker:4040/dev.html', function(err, res){ console.error(err); console.log(res); }, options); diff --git a/client/test/run-jankie.sh b/client/test/run-jankie.sh index 2c3f16b2c..005c8444a 100755 --- a/client/test/run-jankie.sh +++ b/client/test/run-jankie.sh @@ -10,6 +10,12 @@ # # perfjankie --only-update-site --couch-server http://local.docker:5984 --couch-database performance # +# Optional: run from localhost which can be a bit fast than rebuilding... +# - ssh -R 0.0.0.0:4042:localhost:4042 docker@local.docker +# - npm run build +# - BACKEND_HOST=local.docker npm start +# - ./run-jankie.sh 192.168.64.3:4042 +# # Usage: # # ./run-jankie.sh 192.168.64.3:4040 @@ -26,9 +32,9 @@ COMMIT=$(git log --format="%h" -1) echo "Testing $COMMIT on $DATE" -../../scope stop -make SUDO= -C ../.. -../../scope launch -sleep 5 +# ../../scope stop +# make SUDO= -C ../.. +# ../../scope launch +# sleep 5 COMMIT="$COMMIT" DATE=$DATE HOST=$HOST DEBUG=scope* node ./perfjankie/main.js From 85e63ac78656c530785244daefe5a5bc5f47cf00 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 8 Sep 2016 20:30:28 +0200 Subject: [PATCH 07/11] Fixes spelling mistake --- client/app/scripts/selectors/chartSelectors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 0d5456ae0..24358b828 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -36,7 +36,7 @@ export const nodesSelector = createSelector( // - is(v1, v2) === true // // - createDeepEqualSelector will wrap those calls with a: is(v1, v2) ? v1 : v2 -// - Thus you can compare consequtive calls to adjacentNodesSelector(state) with === (which is +// - Thus you can compare consecutive calls to adjacentNodesSelector(state) with === (which is // what redux is doing with connect() // // Note: this feels like the wrong way to be using reselect... From eec54415ec174fe52ca3fee4bbd874565e2ec464 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Wed, 14 Sep 2016 11:32:38 +0200 Subject: [PATCH 08/11] Fixes hanging edges caused by a race condition of sorts New nodes were arriving before they had been laid out. --- .../app/scripts/components/debug-toolbar.js | 81 ++++++++++++------- .../app/scripts/selectors/chartSelectors.js | 28 ++++--- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/client/app/scripts/components/debug-toolbar.js b/client/app/scripts/components/debug-toolbar.js index e1574574b..369238d1e 100644 --- a/client/app/scripts/components/debug-toolbar.js +++ b/client/app/scripts/components/debug-toolbar.js @@ -43,9 +43,6 @@ const LABEL_PREFIXES = _.range('A'.charCodeAt(), 'Z'.charCodeAt() + 1) .map(n => String.fromCharCode(n)); -// const randomLetter = () => _.sample(LABEL_PREFIXES); - - const deltaAdd = ( name, adjacency = [], shape = 'circle', stack = false, nodeCount = 1, networks = NETWORKS @@ -71,7 +68,7 @@ function addMetrics(availableMetrics, node, v) { ]); return Object.assign({}, node, { - metrics: metrics.map(m => Object.assign({}, m, {max: 100, value: v})) + metrics: metrics.map(m => Object.assign({}, m, {label: 'zing', max: 100, value: v})).toJS() }); } @@ -94,14 +91,16 @@ function addAllVariants(dispatch) { } -function addAllMetricVariants(availableMetrics, dispatch) { +function addAllMetricVariants(availableMetrics) { const newNodes = _.flattenDeep(METRIC_FILLS.map((v, i) => ( SHAPES.map(s => [addMetrics(availableMetrics, deltaAdd(label(s) + i, [], s), v)]) ))); - dispatch(receiveNodesDelta({ - add: newNodes - })); + return (dispatch) => { + dispatch(receiveNodesDelta({ + add: newNodes + })); + }; } @@ -177,11 +176,28 @@ class DebugToolbar extends React.Component { }); } - setLoading(loading) { - this.props.dispatch(setAppState(state => state.set('topologiesLoaded', !loading))); + asyncDispatch(v) { + setTimeout(() => this.props.dispatch(v), 0); } - addNodes(n, prefix = 'zing') { + setLoading(loading) { + this.asyncDispatch(setAppState(state => state.set('topologiesLoaded', !loading))); + } + + updateAdjacencies() { + const ns = this.props.nodes; + const nodeNames = ns.keySeq().toJS(); + this.asyncDispatch(receiveNodesDelta({ + add: this._addNodes(7), + update: sample(nodeNames).map(n => ({ + id: n, + adjacency: sample(nodeNames), + }), nodeNames.length), + remove: this._removeNode(), + })); + } + + _addNodes(n, prefix = 'zing') { const ns = this.props.nodes; const nodeNames = ns.keySeq().toJS(); const newNodeNames = _.range(ns.size, ns.size + n).map(i => ( @@ -189,26 +205,34 @@ class DebugToolbar extends React.Component { `${prefix}${i}` )); const allNodes = _(nodeNames).concat(newNodeNames).value(); + return newNodeNames.map((name) => deltaAdd( + name, + sample(allNodes), + _.sample(SHAPES), + _.sample(STACK_VARIANTS), + _.sample(NODE_COUNTS), + sample(NETWORKS, 10) + )); + } - this.props.dispatch(receiveNodesDelta({ - add: newNodeNames.map((name) => deltaAdd( - name, - sample(allNodes), - _.sample(SHAPES), - _.sample(STACK_VARIANTS), - _.sample(NODE_COUNTS), - sample(NETWORKS, 10) - )) - })); + addNodes(n, prefix = 'zing') { + setTimeout(() => { + this.asyncDispatch(receiveNodesDelta({ + add: this._addNodes(n, prefix) + })); + log('added nodes', n); + }, 0); + } - log('added nodes', n); + _removeNode() { + const ns = this.props.nodes; + const nodeNames = ns.keySeq().toJS(); + return [nodeNames[_.random(nodeNames.length - 1)]]; } removeNode() { - const ns = this.props.nodes; - const nodeNames = ns.keySeq().toJS(); - this.props.dispatch(receiveNodesDelta({ - remove: [nodeNames[_.random(nodeNames.length - 1)]] + this.asyncDispatch(receiveNodesDelta({ + remove: this._removeNode() })); } @@ -223,12 +247,13 @@ class DebugToolbar extends React.Component { - - + +
diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 24358b828..3a11871e6 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -1,9 +1,13 @@ +import debug from 'debug'; import { createSelector, createSelectorCreator, defaultMemoize } from 'reselect'; -import { Map as makeMap, is } from 'immutable'; +import { Map as makeMap, is, Set } from 'immutable'; import { getAdjacentNodes } from '../utils/topology-utils'; +const log = debug('scope:selectors'); + + // // "immutable" createSelector // @@ -82,15 +86,18 @@ export const dataNodesSelector = createSelector( ); +// // FIXME: this is a bit of a hack... +// export const layoutNodesSelector = (_, props) => props.layoutNodes || makeMap(); -function mergeDeepIfExists(mapA, mapB) { +function mergeDeepKeyIntersection(mapA, mapB) { // - // Does a deep merge on any key that exists in the first map + // Does a deep merge on keys that exists in both maps // - return mapA.map((v, k) => v.mergeDeep(mapB.get(k))); + const commonKeys = Set.fromKeys(mapA).intersect(mapB.keySeq()); + return makeMap(commonKeys.map(k => [k, mapA.get(k).mergeDeep(mapB.get(k))])); } @@ -98,12 +105,15 @@ const _completeNodesSelector = createSelector( layoutNodesSelector, dataNodesSelector, (layoutNodes, dataNodes) => { - if (layoutNodes.size === 0 || dataNodes.size === 0) { - return makeMap(); + // + // There are no guarantees whether this selector will be computed first (when + // node-chart-elements.mapStateToProps is called by store.subscribe before + // nodes-chart.mapStateToProps is called), and component render batching and yadada. + // + if (layoutNodes.size !== dataNodes.size) { + log('Obviously mismatched node data', layoutNodes.size, dataNodes.size); } - - // dataNodes might get updated before layoutNodes when a node is removed from the topo. - return mergeDeepIfExists(dataNodes, layoutNodes); + return mergeDeepKeyIntersection(dataNodes, layoutNodes); } ); From 7b51f97818288404c899ffe6a1f116a5b03d5e8c Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Thu, 15 Sep 2016 11:55:48 +0200 Subject: [PATCH 09/11] Fixes after rebase --- client/app/scripts/utils/topology-utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/app/scripts/utils/topology-utils.js b/client/app/scripts/utils/topology-utils.js index 975943598..f1a9f1b5c 100644 --- a/client/app/scripts/utils/topology-utils.js +++ b/client/app/scripts/utils/topology-utils.js @@ -1,5 +1,5 @@ import _ from 'lodash'; -import { Map as makeMap, Set as makeSet, List as makeList } from 'immutable'; +import { Set as makeSet, List as makeList } from 'immutable'; // From c94e0f95d1e29b53771b53edb8f75760587a3afa Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 20 Sep 2016 18:50:35 +0200 Subject: [PATCH 10/11] Small refactor of selector helpers with more comments --- .../app/scripts/selectors/chartSelectors.js | 102 +++++++++--------- 1 file changed, 48 insertions(+), 54 deletions(-) diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 3a11871e6..06d12413a 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -9,63 +9,72 @@ const log = debug('scope:selectors'); // -// "immutable" createSelector +// `mergeDeepKeyIntersection` does a deep merge on keys that exists in both maps // -const createDeepEqualSelector = createSelectorCreator( - defaultMemoize, - is -); +function mergeDeepKeyIntersection(mapA, mapB) { + const commonKeys = Set.fromKeys(mapA).intersect(mapB.keySeq()); + return makeMap(commonKeys.map(k => [k, mapA.get(k).mergeDeep(mapB.get(k))])); +} -const identity = v => v; +// +// `returnPreviousRefIfEqual` is a helper function that checks the new computed of a selector +// against the previously computed value. If they are deeply equal return the previous result. This +// is important for things like connect() which tests whether componentWillReceiveProps should be +// called by doing a '===' on the values you return from mapStateToProps. +// +// e.g. +// +// const filteredThings = createSelector( +// state => state.things, +// (things) => things.filter(t => t > 2) +// ); +// +// // This will trigger componentWillReceiveProps on every store change: +// connect(s => { things: filteredThings(s) }, ThingComponent); +// +// // But if we wrap it, the result will be === if it `is()` equal and... +// const filteredThingsWrapped = returnPreviousRefIfEqual(filteredThings); +// +// // ...We're safe! +// connect(s => { things: filteredThingsWrapped(s) }, ThingComponent); +// +// Note: This is a slightly stange way to use reselect. Selectors memoize their *arguments* not +// "their results", so use the result of the wrapped selector as the argument to another selector +// here to memoize it and get what we want. +// +const _createDeepEqualSelector = createSelectorCreator(defaultMemoize, is); +const _identity = v => v; +const returnPreviousRefIfEqual = (selector) => _createDeepEqualSelector(selector, _identity); + + +// +// Selectors! +// const allNodesSelector = state => state.get('nodes'); -export const nodesSelector = createSelector( - allNodesSelector, - (allNodes) => allNodes.filter(node => !node.get('filtered')) +export const nodesSelector = returnPreviousRefIfEqual( + createSelector( + allNodesSelector, + (allNodes) => allNodes.filter(node => !node.get('filtered')) + ) ); -// -// This is like an === cache... -// -// - getAdjacentNodes is run on every state change and can generate a new immutable object each -// time: -// - v1 = getAdjacentNodes(a) -// - v2 = getAdjacentNodes(a) -// - v1 !== v2 -// - is(v1, v2) === true -// -// - createDeepEqualSelector will wrap those calls with a: is(v1, v2) ? v1 : v2 -// - Thus you can compare consecutive calls to adjacentNodesSelector(state) with === (which is -// what redux is doing with connect() -// -// Note: this feels like the wrong way to be using reselect... -// -export const adjacentNodesSelector = createDeepEqualSelector( - getAdjacentNodes, - identity -); +export const adjacentNodesSelector = returnPreviousRefIfEqual(getAdjacentNodes); -// -// You what? What is going on here? -// -// We wrap the result of nodes.map in another equality test which discards the new value -// if it was the same as the old one. Again preserving === -// -export const nodeAdjacenciesSelector = createDeepEqualSelector( +export const nodeAdjacenciesSelector = returnPreviousRefIfEqual( createSelector( nodesSelector, (nodes) => nodes.map(n => makeMap({ id: n.get('id'), adjacency: n.get('adjacency'), })) - ), - identity + ) ); @@ -92,16 +101,7 @@ export const dataNodesSelector = createSelector( export const layoutNodesSelector = (_, props) => props.layoutNodes || makeMap(); -function mergeDeepKeyIntersection(mapA, mapB) { - // - // Does a deep merge on keys that exists in both maps - // - const commonKeys = Set.fromKeys(mapA).intersect(mapB.keySeq()); - return makeMap(commonKeys.map(k => [k, mapA.get(k).mergeDeep(mapB.get(k))])); -} - - -const _completeNodesSelector = createSelector( +export const completeNodesSelector = createSelector( layoutNodesSelector, dataNodesSelector, (layoutNodes, dataNodes) => { @@ -116,9 +116,3 @@ const _completeNodesSelector = createSelector( return mergeDeepKeyIntersection(dataNodes, layoutNodes); } ); - - -export const completeNodesSelector = createDeepEqualSelector( - _completeNodesSelector, - identity -); From d48748aafe12927f81b9ff4b6d00b9ebe8c79d12 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Tue, 20 Sep 2016 19:25:16 +0200 Subject: [PATCH 11/11] Fix spelling --- client/app/scripts/selectors/chartSelectors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/app/scripts/selectors/chartSelectors.js b/client/app/scripts/selectors/chartSelectors.js index 06d12413a..5792a56b2 100644 --- a/client/app/scripts/selectors/chartSelectors.js +++ b/client/app/scripts/selectors/chartSelectors.js @@ -39,7 +39,7 @@ function mergeDeepKeyIntersection(mapA, mapB) { // // ...We're safe! // connect(s => { things: filteredThingsWrapped(s) }, ThingComponent); // -// Note: This is a slightly stange way to use reselect. Selectors memoize their *arguments* not +// Note: This is a slightly strange way to use reselect. Selectors memoize their *arguments* not // "their results", so use the result of the wrapped selector as the argument to another selector // here to memoize it and get what we want. //