From 3f2811d9da22f4be0d90a90665d4e3702ea041a4 Mon Sep 17 00:00:00 2001 From: Bruce Martin Date: Fri, 4 Oct 2019 07:59:22 -0700 Subject: [PATCH] rendering performance improvements (#968) * freeze objects * component rendering perf work * use PureComponent where safe * remove obsolete WorldUtil code * make brushable histogram a pure component --- .../util/stateManager/worldUtil.test.js | 62 --------------- .../components/brushableHistogram/index.js | 42 +++++------ .../src/components/categorical/categorical.js | 40 +++++----- client/src/components/graph/graph.js | 2 +- .../src/components/menubar/cellSetButtons.js | 2 +- .../src/components/scatterplot/scatterplot.js | 2 +- client/src/reducers/controls.js | 5 -- client/src/util/dataframe/dataframe.js | 3 + client/src/util/stateManager/index.js | 1 - client/src/util/stateManager/worldUtil.js | 75 ------------------- client/src/util/typedCrossfilter/bitArray.js | 1 + .../src/util/typedCrossfilter/crossfilter.js | 2 + 12 files changed, 47 insertions(+), 190 deletions(-) delete mode 100644 client/__tests__/util/stateManager/worldUtil.test.js delete mode 100644 client/src/util/stateManager/worldUtil.js diff --git a/client/__tests__/util/stateManager/worldUtil.test.js b/client/__tests__/util/stateManager/worldUtil.test.js deleted file mode 100644 index 59ddc974..00000000 --- a/client/__tests__/util/stateManager/worldUtil.test.js +++ /dev/null @@ -1,62 +0,0 @@ -import { - countCategoryValues2D, - clearCaches -} from "../../../src/util/stateManager/worldUtil"; -import * as Dataframe from "../../../src/util/dataframe"; - -describe("WorldUtil cache management", () => { - test("empty", () => { - const count = countCategoryValues2D( - "a", - "b", - new Dataframe.Dataframe([0, 0], []) - ); - expect(count).toMatchObject(new Map()); - expect(count.size).toBe(0); - }); - - test("simple couts", () => { - const df = new Dataframe.Dataframe( - [3, 2], - [[0, 0, 1], [false, true, false]], - null, - new Dataframe.KeyIndex(["a", "b"]) - ); - const count = countCategoryValues2D("a", "b", df); - expect(count).toMatchObject( - new Map([ - [0, new Map([[true, 1], [false, 1]])], - [1, new Map([[false, 1]])] - ]) - ); - }); - - test("memo cache clear", () => { - clearCaches(); - const df1 = new Dataframe.Dataframe([0, 0], []); - const df2 = new Dataframe.Dataframe( - [3, 2], - [[0, 0, 1], [false, true, false]], - null, - new Dataframe.KeyIndex(["a", "b"]) - ); - - const count1 = countCategoryValues2D("a", "b", df1); - const count2 = countCategoryValues2D("a", "b", df1); - const count3 = countCategoryValues2D("a", "b", df1.clone()); - const count4 = countCategoryValues2D("a", "b", df2); - - clearCaches(); - const count10 = countCategoryValues2D("a", "b", df1); - const count11 = countCategoryValues2D("a", "b", df2); - - expect(count1).toEqual(count2); - expect(count1).toEqual(count3); - expect(count1).toEqual(count10); - expect(count1).not.toBe(count3); - expect(count1).not.toBe(count10); - - expect(count4).toEqual(count11); - expect(count4).not.toBe(count11); - }); -}); diff --git a/client/src/components/brushableHistogram/index.js b/client/src/components/brushableHistogram/index.js index 5ef7adb1..58379b20 100644 --- a/client/src/components/brushableHistogram/index.js +++ b/client/src/components/brushableHistogram/index.js @@ -13,15 +13,21 @@ import * as globals from "../../globals"; import actions from "../../actions"; import { makeContinuousDimensionName } from "../../util/nameCreators"; -@connect(state => ({ - world: state.world, - scatterplotXXaccessor: state.controls.scatterplotXXaccessor, - scatterplotYYaccessor: state.controls.scatterplotYYaccessor, - continuousSelection: state.continuousSelection, - differential: state.differential, - colorAccessor: state.colors.colorAccessor -})) -class HistogramBrush extends React.Component { +@connect((state, ownProps) => { + const { isObs, isUserDefined, isDiffExp, field } = ownProps; + const myName = makeContinuousDimensionName( + { isObs, isUserDefined, isDiffExp }, + field + ); + return { + world: state.world, + scatterplotXXaccessor: state.controls.scatterplotXXaccessor, + scatterplotYYaccessor: state.controls.scatterplotYYaccessor, + continuousSelectionRange: state.continuousSelection[myName], + colorAccessor: state.colors.colorAccessor + }; +}) +class HistogramBrush extends React.PureComponent { static getColumn(world, field, clipped = true) { /* Return the underlying Dataframe column for our field. By default, @@ -84,7 +90,7 @@ class HistogramBrush extends React.Component { } componentDidUpdate(prevProps) { - const { field, world, continuousSelection } = this.props; + const { field, world } = this.props; const { x, y, bins, svgRef } = this._histogram; let { brushXselection, brushX } = this.state; let forceBrushUpdate = false; @@ -112,16 +118,8 @@ class HistogramBrush extends React.Component { if the selection has changed, ensure that the brush correctly reflects the underlying selection. */ - if ( - forceBrushUpdate || - continuousSelection !== prevProps.continuousSelection - ) { - const { isObs, isUserDefined, isDiffExp } = this.props; - const myName = makeContinuousDimensionName( - { isObs, isUserDefined, isDiffExp }, - field - ); - const range = continuousSelection[myName]; + const { continuousSelectionRange: range } = this.props; + if (forceBrushUpdate || range !== prevProps.continuousSelectionRange) { if (brushXselection) { const selection = d3.brushSelection(brushXselection.node()); if (!range && selection) { @@ -417,8 +415,8 @@ class HistogramBrush extends React.Component { isDiffExp ? "histogram-diffexp" : isUserDefined - ? "histogram-user-gene" - : "histogram-continuous-metadata" + ? "histogram-user-gene" + : "histogram-continuous-metadata" } style={{ padding: globals.leftSidebarSectionPadding, diff --git a/client/src/components/categorical/categorical.js b/client/src/components/categorical/categorical.js index da658eac..98bb8785 100644 --- a/client/src/components/categorical/categorical.js +++ b/client/src/components/categorical/categorical.js @@ -76,30 +76,26 @@ class Categories extends React.Component { > {/* READ ONLY CATEGORICAL FIELDS */} {/* this is duplicative but flat, could be abstracted */} - {_.map( - allCategoryNames, - catName => - !schema.annotations.obsByName[catName].writable ? ( - - ) : null + {_.map(allCategoryNames, catName => + !schema.annotations.obsByName[catName].writable ? ( + + ) : null )} {/* WRITEABLE FIELDS */} - {_.map( - allCategoryNames, - catName => - schema.annotations.obsByName[catName].writable ? ( - - ) : null + {_.map(allCategoryNames, catName => + schema.annotations.obsByName[catName].writable ? ( + + ) : null )} {writableCategoriesEnabled ? (
diff --git a/client/src/components/graph/graph.js b/client/src/components/graph/graph.js index c3cdbfa5..66f30416 100644 --- a/client/src/components/graph/graph.js +++ b/client/src/components/graph/graph.js @@ -83,7 +83,7 @@ function renderThrottle(callback) { graphInteractionMode: state.controls.graphInteractionMode, colorAccessor: state.colors.colorAccessor })) -class Graph extends React.Component { +class Graph extends React.PureComponent { computePointPositions = memoize((X, Y, modelTF) => { /* compute the model coordinate for each point diff --git a/client/src/components/menubar/cellSetButtons.js b/client/src/components/menubar/cellSetButtons.js index c9b34115..47f9a338 100644 --- a/client/src/components/menubar/cellSetButtons.js +++ b/client/src/components/menubar/cellSetButtons.js @@ -6,7 +6,7 @@ import { World } from "../../util/stateManager"; import { tooltipHoverOpenDelay } from "../../globals"; @connect() -class CellSetButton extends React.Component { +class CellSetButton extends React.PureComponent { set() { const { differential, diff --git a/client/src/components/scatterplot/scatterplot.js b/client/src/components/scatterplot/scatterplot.js index 43ad808d..781aa9b7 100644 --- a/client/src/components/scatterplot/scatterplot.js +++ b/client/src/components/scatterplot/scatterplot.js @@ -57,7 +57,7 @@ function createProjectionTF(viewportWidth, viewportHeight) { responsive: state.responsive }; }) -class Scatterplot extends React.Component { +class Scatterplot extends React.PureComponent { computePointPositions = memoize((X, Y, xScale, yScale) => { const positions = new Float32Array(2 * X.length); for (let i = 0, len = X.length; i < len; i += 1) { diff --git a/client/src/reducers/controls.js b/client/src/reducers/controls.js index e3e3f908..9a9ac531 100644 --- a/client/src/reducers/controls.js +++ b/client/src/reducers/controls.js @@ -2,8 +2,6 @@ import _ from "lodash"; -import { WorldUtil } from "../util/stateManager"; - const Controls = ( state = { // data loading flag @@ -45,7 +43,6 @@ const Controls = ( } case "initial data load complete (universe exists)": { /* first light - create world & other data-driven defaults */ - WorldUtil.clearCaches(); return { ...state, loading: false, @@ -54,14 +51,12 @@ const Controls = ( }; } case "reset World to eq Universe": { - WorldUtil.clearCaches(); return { ...state, resettingInterface: false }; } case "set World to current selection": { - WorldUtil.clearCaches(); return { ...state, loading: false, diff --git a/client/src/util/dataframe/dataframe.js b/client/src/util/dataframe/dataframe.js index c79a7b6d..5e2c444e 100644 --- a/client/src/util/dataframe/dataframe.js +++ b/client/src/util/dataframe/dataframe.js @@ -127,6 +127,7 @@ class Dataframe { this.__id = Dataframe.__getId(); this.__compile(__columnsAccessor); + Object.freeze(this); } static __errorChecks(dims, columnarData, rowIndex, colIndex) { @@ -269,6 +270,7 @@ class Dataframe { get.iget = iget; get.__id = __id; + Object.freeze(get); return get; } @@ -288,6 +290,7 @@ class Dataframe { } return Dataframe.__compileColumn(column, getRowByOffset, getRowByLabel); }); + Object.freeze(this.__columnsAccessor); } clone() { diff --git a/client/src/util/stateManager/index.js b/client/src/util/stateManager/index.js index 345ffbde..c73f9518 100644 --- a/client/src/util/stateManager/index.js +++ b/client/src/util/stateManager/index.js @@ -17,7 +17,6 @@ exists to support those concepts. export * as ColorHelpers from "./colorHelpers"; export * as Universe from "./universe"; export * as World from "./world"; -export * as WorldUtil from "./worldUtil"; export * as ControlsHelpers from "./controlsHelpers"; export * as AnnotationsHelpers from "./annotationsHelpers"; export * as SchemaHelpers from "./schemaHelpers"; diff --git a/client/src/util/stateManager/worldUtil.js b/client/src/util/stateManager/worldUtil.js deleted file mode 100644 index d51eb638..00000000 --- a/client/src/util/stateManager/worldUtil.js +++ /dev/null @@ -1,75 +0,0 @@ -/* eslint-disable import/prefer-default-export */ -import _ from "lodash"; - -/* -Various utility functions operating on World/Universe -*/ - -/* -Count unique category values, binning first by dim1 then by dim2 -Return: - -Map { - dim1_val1: Map { - dim2_val1: number, - dim2_val2: number, - ... - }, - ... -} - -Parameters are: -- dim1: dimension 1 name/label -- dim2: dimension 2 name/label -- df: dataframe containing dim1 and dim2 on the column axis - -*/ -function _countCategoryValues2D(dim1, dim2, df) { - const dimMap = new Map(); - const col1 = df.col(dim1) ? df.col(dim1).asArray() : null; - const col2 = df.col(dim2) ? df.col(dim2).asArray() : null; - if (!col1 || !col2) { - return dimMap; - } - - for (let r = 0, l = df.length; r < l; r += 1) { - const val1 = col1[r]; - const val2 = col2[r]; - let d2Map = dimMap.get(val1); - if (d2Map === undefined) { - d2Map = new Map(); - dimMap.set(val1, d2Map); - } - let curCount = d2Map.get(val2); - if (curCount === undefined) { - curCount = 0; - } - d2Map.set(val2, curCount + 1); - } - return dimMap; -} - -let __worldUtilMemoId__ = 0; -function _memoizedId(x) { - if (!x.__worldUtilMemoId__) { - __worldUtilMemoId__ += 1; - x.__worldUtilMemoId__ = __worldUtilMemoId__; - } - return x.__worldUtilMemoId__; -} -function _countCategoryValues2DResolver(...args) { - const id = args[0] + args[1] + _memoizedId(args[2]); - return id; -} - -export const countCategoryValues2D = _.memoize( - _countCategoryValues2D, - _countCategoryValues2DResolver -); - -/* -Clear any cached data within WorldUtil caches, eg, memoized functions -*/ -export function clearCaches() { - countCategoryValues2D.cache.clear(); -} diff --git a/client/src/util/typedCrossfilter/bitArray.js b/client/src/util/typedCrossfilter/bitArray.js index 51ff809a..3db06c1c 100644 --- a/client/src/util/typedCrossfilter/bitArray.js +++ b/client/src/util/typedCrossfilter/bitArray.js @@ -35,6 +35,7 @@ class BitArray { this.bitmask = new Int32Array(this.width); // dimension allocation mask this.bitarray = new Int32Array(this.width * this.length); + Object.seal(this); } // Return the number of records that are selected, ie, have a one bit in diff --git a/client/src/util/typedCrossfilter/crossfilter.js b/client/src/util/typedCrossfilter/crossfilter.js index 98e9672d..d1961156 100644 --- a/client/src/util/typedCrossfilter/crossfilter.js +++ b/client/src/util/typedCrossfilter/crossfilter.js @@ -47,6 +47,7 @@ export default class ImmutableTypedCrossfilter { this.data = data; this.selectionCache = selectionCache; /* BitArray */ this.dimensions = dimensions; /* name: { id, dim, name, selection } */ + Object.preventExtensions(this); } size() { @@ -94,6 +95,7 @@ export default class ImmutableTypedCrossfilter { } const DimensionType = DimTypes[type]; const dim = new DimensionType(name, data, ...rest); + Object.freeze(dim); const dimensions = { ...this.dimensions, [name]: {