From 8581aa1944b4f2c14fa9deeb9043ab3d09ff54b0 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Fri, 17 Feb 2023 02:45:05 -0800 Subject: [PATCH] Memoise selection of nodes Summary: For the visualiser we use the same trick as with the hover state. We subscribe to selection changes and only render if the prev or new state concerns us. For the tree we change from object identity to the node id + and indent guide are added to the memoisation equal check. Depending on teh change this tree memoisation can vary in effectiveness. If you go from nothing selecting to selecting the top element nothing is memoised since react needs to render every element to draw the indent guide. If you have somethign selected and select a nearby element the memoisation works well. There are ways to improve this more down the road changelog: UIDebugger improve performance of selecting nodes Reviewed By: lblasa Differential Revision: D43305979 fbshipit-source-id: 5d90e806ed7b6a8401e9968be398d4a67ed0c294 --- .../public/ui-debugger/components/Tree.tsx | 24 ++++------- .../components/Visualization2D.tsx | 25 +++++++----- .../public/ui-debugger/components/main.tsx | 16 ++------ .../components/sidebar/Inspector.tsx | 26 ++++++++---- .../ui-debugger/hooks/usefilteredValue.tsx | 40 +++++++++++++++++++ desktop/plugins/public/ui-debugger/index.tsx | 7 ++++ 6 files changed, 91 insertions(+), 47 deletions(-) create mode 100644 desktop/plugins/public/ui-debugger/hooks/usefilteredValue.tsx diff --git a/desktop/plugins/public/ui-debugger/components/Tree.tsx b/desktop/plugins/public/ui-debugger/components/Tree.tsx index 3097042db..afec87fc7 100644 --- a/desktop/plugins/public/ui-debugger/components/Tree.tsx +++ b/desktop/plugins/public/ui-debugger/components/Tree.tsx @@ -29,7 +29,7 @@ import { } from 'flipper-plugin'; import {plugin} from '../index'; import {Glyph} from 'flipper'; -import {groupBy, head, last} from 'lodash'; +import {groupBy, head, isEqual, last} from 'lodash'; import {reverse} from 'lodash/fp'; import {Dropdown, Menu, Typography} from 'antd'; import {UIDebuggerMenuItem} from './util/UIDebuggerMenuItem'; @@ -50,21 +50,12 @@ export type TreeNode = UINode & { indentGuide: NodeIndentGuide | null; }; -export function Tree2({ - nodes, - rootId, - selectedNode, - onSelectNode, -}: { - nodes: Map; - rootId: Id; - selectedNode?: Id; - onSelectNode: (node?: Id) => void; -}) { +export function Tree2({nodes, rootId}: {nodes: Map; rootId: Id}) { const instance = usePlugin(plugin); const focusedNode = useValue(instance.uiState.focusedNode); const expandedNodes = useValue(instance.uiState.expandedNodes); const searchTerm = useValue(instance.uiState.searchTerm); + const selectedNode = useValue(instance.uiState.selectedNode); const isContextMenuOpen = useValue(instance.uiState.isContextMenuOpen); const hoveredNode = head(useValue(instance.uiState.hoveredNodes)); @@ -96,7 +87,7 @@ export function Tree2({ refs, selectedNode, hoveredNode, - onSelectNode, + instance.uiActions.onSelectNode, instance.uiActions.onExpandNode, instance.uiActions.onCollapseNode, isUsingKBToScroll, @@ -140,7 +131,7 @@ export function Tree2({ hoveredNode={hoveredNode} isUsingKBToScroll={isUsingKBToScroll} isContextMenuOpen={isContextMenuOpen} - onSelectNode={onSelectNode} + onSelectNode={instance.uiActions.onSelectNode} onExpandNode={instance.uiActions.onExpandNode} onCollapseNode={instance.uiActions.onCollapseNode} onHoverNode={instance.uiActions.onHoverNode} @@ -157,7 +148,7 @@ const MemoTreeItemContainer = React.memo( (prevProps, nextProps) => { const id = nextProps.treeNode.id; return ( - prevProps.treeNode === nextProps.treeNode && + prevProps.treeNode.id === nextProps.treeNode.id && prevProps.isContextMenuOpen === nextProps.isContextMenuOpen && prevProps.frameworkEvents === nextProps.frameworkEvents && prevProps.highlightedNodes === nextProps.highlightedNodes && @@ -166,7 +157,8 @@ const MemoTreeItemContainer = React.memo( prevProps.hoveredNode !== id && //make sure that prev or next hover/selected node doesnt concern this tree node nextProps.hoveredNode !== id && prevProps.selectedNode !== id && - nextProps.selectedNode !== id + nextProps.selectedNode !== id && + isEqual(prevProps.treeNode.indentGuide, nextProps.treeNode.indentGuide) ); }, ); diff --git a/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx b/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx index 02c8c251c..eff660659 100644 --- a/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx +++ b/desktop/plugins/public/ui-debugger/components/Visualization2D.tsx @@ -7,7 +7,7 @@ * @format */ -import React, {useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {Bounds, Coordinate, Id, NestedNode, Tag, UINode} from '../types'; import {produce, styled, theme, usePlugin, useValue} from 'flipper-plugin'; @@ -15,17 +15,17 @@ import {plugin} from '../index'; import {head, isEqual, throttle} from 'lodash'; import {Dropdown, Menu, Tooltip} from 'antd'; import {UIDebuggerMenuItem} from './util/UIDebuggerMenuItem'; +import {useFilteredValue} from '../hooks/usefilteredValue'; export const Visualization2D: React.FC< { rootId: Id; width: number; nodes: Map; - selectedNode?: Id; onSelectNode: (id?: Id) => void; modifierPressed: boolean; } & React.HTMLAttributes -> = ({rootId, width, nodes, selectedNode, onSelectNode, modifierPressed}) => { +> = ({rootId, width, nodes, onSelectNode, modifierPressed}) => { const rootNodeRef = useRef(); const instance = usePlugin(plugin); @@ -147,7 +147,6 @@ export const Visualization2D: React.FC< )} @@ -161,27 +160,34 @@ const MemoedVisualizationNode2D = React.memo( Visualization2DNode, (prev, next) => { return ( - prev.node === next.node && - prev.modifierPressed === next.modifierPressed && - prev.selectedNode === next.selectedNode + prev.node === next.node && prev.modifierPressed === next.modifierPressed ); }, ); function Visualization2DNode({ node, - selectedNode, onSelectNode, modifierPressed, }: { node: NestedNode; modifierPressed: boolean; - selectedNode?: Id; onSelectNode: (id?: Id) => void; }) { const instance = usePlugin(plugin); + const wasOrIsSelected = useCallback( + (curValue?: Id, prevValue?: Id) => + curValue === node.id || prevValue === node.id, + [node.id], + ); + const selectedNode = useFilteredValue( + instance.uiState.selectedNode, + wasOrIsSelected, + ); + const isSelected = selectedNode === node.id; + const {isHovered, isLongHovered} = useHoverStates(node.id); const ref = useRef(null); let nestedChildren: NestedNode[]; @@ -205,7 +211,6 @@ function Visualization2DNode({ key={child.id} node={child} onSelectNode={onSelectNode} - selectedNode={selectedNode} modifierPressed={modifierPressed} /> )); diff --git a/desktop/plugins/public/ui-debugger/components/main.tsx b/desktop/plugins/public/ui-debugger/components/main.tsx index 07674a695..98d92a856 100644 --- a/desktop/plugins/public/ui-debugger/components/main.tsx +++ b/desktop/plugins/public/ui-debugger/components/main.tsx @@ -35,7 +35,6 @@ export function Component() { const metadata: Map = useValue(instance.metadata); const [showPerfStats, setShowPerfStats] = useState(false); - const [selectedNode, setSelectedNode] = useState(undefined); const [visualiserWidth, setVisualiserWidth] = useState(500); @@ -54,12 +53,7 @@ export function Component() { - + @@ -76,16 +70,12 @@ export function Component() { rootId={rootId} width={visualiserWidth} nodes={nodes} - selectedNode={selectedNode} - onSelectNode={setSelectedNode} + onSelectNode={instance.uiActions.onSelectNode} modifierPressed={ctrlPressed} /> - + diff --git a/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx b/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx index 1df3e280b..038c728bb 100644 --- a/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx +++ b/desktop/plugins/public/ui-debugger/components/sidebar/Inspector.tsx @@ -10,20 +10,26 @@ import React from 'react'; // eslint-disable-next-line rulesdir/no-restricted-imports-clone import {Glyph} from 'flipper'; -import {Layout, Tab, Tabs, theme} from 'flipper-plugin'; -import {Metadata, MetadataId, UINode} from '../../types'; +import {Layout, Tab, Tabs, theme, usePlugin, useValue} from 'flipper-plugin'; +import {Id, Metadata, MetadataId, UINode} from '../../types'; + import {IdentityInspector} from './inspector/IdentityInspector'; import {AttributesInspector} from './inspector/AttributesInspector'; import {Tooltip} from 'antd'; import {NoData} from './inspector/NoData'; +import {plugin} from '../../index'; type Props = { - node?: UINode; + nodes: Map; metadata: Map; }; -export const Inspector: React.FC = ({node, metadata}) => { - if (!node) { +export const Inspector: React.FC = ({nodes, metadata}) => { + const instance = usePlugin(plugin); + const selectedNodeId = useValue(instance.uiState.selectedNode); + + const selectedNode = nodes.get(selectedNodeId!!); + if (!selectedNode) { return ; } return ( @@ -37,7 +43,7 @@ export const Inspector: React.FC = ({node, metadata}) => { }> - + = ({node, metadata}) => { }> @@ -66,7 +72,11 @@ export const Inspector: React.FC = ({node, metadata}) => { }> - + diff --git a/desktop/plugins/public/ui-debugger/hooks/usefilteredValue.tsx b/desktop/plugins/public/ui-debugger/hooks/usefilteredValue.tsx new file mode 100644 index 000000000..5e689fb4b --- /dev/null +++ b/desktop/plugins/public/ui-debugger/hooks/usefilteredValue.tsx @@ -0,0 +1,40 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {Atom} from 'flipper-plugin'; +import {useEffect, useState} from 'react'; + +/** + * A hook similar to useValue that Subscribes to an atom and returns the current value. + * However the value only updates if the predicate passes. + * + * Usefull for skipping expensive react renders if an update doesnt concern you + * @param atom + * @param predicate + */ +export function useFilteredValue( + atom: Atom, + predicate: (newValue: T, prevValue?: T) => boolean, +) { + const [value, setValue] = useState(atom.get()); + + useEffect(() => { + const listener = (newValue: T, prevValue?: T) => { + if (predicate(newValue, prevValue)) { + setValue(newValue); + } + }; + atom.subscribe(listener); + return () => { + atom.unsubscribe(listener); + }; + }, [atom, predicate]); + + return value; +} diff --git a/desktop/plugins/public/ui-debugger/index.tsx b/desktop/plugins/public/ui-debugger/index.tsx index 5e82c7f63..a319f8aa5 100644 --- a/desktop/plugins/public/ui-debugger/index.tsx +++ b/desktop/plugins/public/ui-debugger/index.tsx @@ -39,6 +39,7 @@ type UIState = { searchTerm: Atom; isContextMenuOpen: Atom; hoveredNodes: Atom; + selectedNode: Atom; highlightedNodes: Atom>; focusedNode: Atom; expandedNodes: Atom>; @@ -92,6 +93,7 @@ export function plugin(client: PluginClient) { highlightedNodes, + selectedNode: createState(undefined), //used to indicate whether we will higher the visualizer / tree when a matching event comes in //also whether or not will show running total in the tree frameworkEventMonitoring: createState( @@ -255,6 +257,7 @@ type UIActions = { onHoverNode: (node: Id) => void; onFocusNode: (focused?: Id) => void; onContextMenuOpen: (open: boolean) => void; + onSelectNode: (node?: Id) => void; onExpandNode: (node: Id) => void; onCollapseNode: (node: Id) => void; }; @@ -265,6 +268,9 @@ function uiActions(uiState: UIState): UIActions { draft.add(node); }); }; + const onSelectNode = (node?: Id) => { + uiState.selectedNode.set(node); + }; const onCollapseNode = (node: Id) => { uiState.expandedNodes.update((draft) => { @@ -288,6 +294,7 @@ function uiActions(uiState: UIState): UIActions { onExpandNode, onCollapseNode, onHoverNode, + onSelectNode, onContextMenuOpen, onFocusNode, };