From de92495f0470cf41e9521e96724404cf4e6743bf Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Tue, 16 Mar 2021 14:54:53 -0700 Subject: [PATCH] Table optimizations Summary: Performance fine tuning. Did some performance fine-tuning primarily by creating a production build, and verifying the responsiveness of searching, tailing etc in the logs plugin while generating a lot of load, and finetuned based on that. For example stopped using requestAnimationFrame which is too sensitive of starving Flipper under high load, as it doesn't leave room for other events to be processed. Also made scrolling smoother by making an append 'high prio' update while taililng. Also debounced changing the (search) filters, as that is an expensive operation we don't want to trigger on every key press Reviewed By: passy Differential Revision: D27046726 fbshipit-source-id: c3efe59eb26e2d9e518325d85531a0e4a6b245ca --- .../src/ui/datatable/DataSourceRenderer.tsx | 35 +++++++--- .../src/ui/datatable/DataTable.tsx | 68 +++++++++++-------- 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx b/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx index eb1ce667a..705ffafb9 100644 --- a/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/DataSourceRenderer.tsx @@ -21,7 +21,9 @@ import {useVirtual} from 'react-virtual'; import styled from '@emotion/styled'; // how fast we update if updates are low-prio (e.g. out of window and not super significant) -const DEBOUNCE = 500; //ms +const LOW_PRIO_UPDATE = 1000; //ms +const HIGH_PRIO_UPDATE = 40; // 25fps +const SMALL_DATASET = 1000; // what we consider a small dataset, for which we keep all updates snappy enum UpdatePrio { NONE, @@ -115,6 +117,7 @@ export const DataSourceRenderer: ( if (unmounted) { return; } + timeoutHandle = undefined; setForceUpdate((x) => x + 1); }; @@ -135,16 +138,24 @@ export const DataSourceRenderer: ( // already scheduled an update with equal or higher prio return; } - renderPending.current = prio; + renderPending.current = Math.max(renderPending.current, prio); if (prio === UpdatePrio.LOW) { - // TODO: make DEBOUNCE depend on how big the relative change is - timeoutHandle = setTimeout(forceUpdate, DEBOUNCE); + // Possible optimization: make DEBOUNCE depend on how big the relative change is, and how far from the current window + if (!timeoutHandle) { + timeoutHandle = setTimeout(forceUpdate, LOW_PRIO_UPDATE); + } } else { - // High + // High, drop low prio timeout if (timeoutHandle) { clearTimeout(timeoutHandle); + timeoutHandle = undefined; + } + if (lastRender.current < Date.now() - HIGH_PRIO_UPDATE) { + forceUpdate(); // trigger render now + } else { + // debounced + timeoutHandle = setTimeout(forceUpdate, HIGH_PRIO_UPDATE); } - requestAnimationFrame(forceUpdate); } } @@ -154,7 +165,15 @@ export const DataSourceRenderer: ( rerender(UpdatePrio.HIGH, true); break; case 'shift': - if (event.location === 'in') { + if (dataSource.view.size < SMALL_DATASET) { + rerender(UpdatePrio.HIGH, false); + } else if ( + event.location === 'in' || + // to support smooth tailing we want to render on records directly at the end of the window immediately as well + (event.location === 'after' && + event.delta > 0 && + event.index === dataSource.view.windowEnd) + ) { rerender(UpdatePrio.HIGH, false); } else { // optimization: we don't want to listen to every count change, especially after window @@ -221,7 +240,7 @@ export const DataSourceRenderer: ( }, [autoScroll, parentRef]); useLayoutEffect(function scrollToEnd() { - if (followOutput.current) { + if (followOutput.current && autoScroll) { virtualizer.scrollToIndex( dataSource.view.size - 1, /* smooth is not typed by react-virtual, but passed on to the DOM as it should*/ diff --git a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx index 904aa3bd7..00e951733 100644 --- a/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/datatable/DataTable.tsx @@ -17,7 +17,6 @@ import React, { MutableRefObject, CSSProperties, useEffect, - useContext, useReducer, } from 'react'; import {TableRow, DEFAULT_ROW_HEIGHT} from './TableRow'; @@ -46,6 +45,7 @@ import {CoffeeOutlined, SearchOutlined} from '@ant-design/icons'; import {useAssertStableRef} from '../../utils/useAssertStableRef'; import {Formatter} from '../DataFormatter'; import {usePluginInstance} from '../../plugin/PluginContext'; +import {debounce} from 'lodash'; interface DataTableProps { columns: DataTableColumn[]; @@ -105,7 +105,7 @@ export function DataTable( // eslint-disable-next-line const scope = props._testHeight ? "" : usePluginInstance().pluginKey; const virtualizerRef = useRef(); - const [state, dispatch] = useReducer( + const [tableState, dispatch] = useReducer( dataTableManagerReducer as DataTableReducer, undefined, () => @@ -118,15 +118,16 @@ export function DataTable( }), ); - const stateRef = useRef(state); - stateRef.current = state; + const stateRef = useRef(tableState); + stateRef.current = tableState; const lastOffset = useRef(0); + const dragging = useRef(false); const [tableManager] = useState(() => createDataTableManager(dataSource, dispatch, stateRef), ); - const {columns, selection, searchValue, sorting} = state; + const {columns, selection, searchValue, sorting} = tableState; const visibleColumns = useMemo( () => columns.filter((column) => column.visible), @@ -134,18 +135,17 @@ export function DataTable( ); const renderingConfig = useMemo>(() => { - let dragging = false; let startIndex = 0; return { columns: visibleColumns, - onMouseEnter(_e, _item, index) { - if (dragging) { + onMouseEnter(e, _item, index) { + if (dragging.current && e.buttons === 1) { // by computing range we make sure no intermediate items are missed when scrolling fast tableManager.addRangeToSelection(startIndex, index); } }, onMouseDown(e, _item, index) { - if (!dragging) { + if (!dragging.current) { if (e.ctrlKey || e.metaKey) { tableManager.addRangeToSelection(index, index, true); } else if (e.shiftKey) { @@ -154,11 +154,11 @@ export function DataTable( tableManager.selectItem(index); } - dragging = true; + dragging.current = true; startIndex = index; function onStopDragSelecting() { - dragging = false; + dragging.current = false; document.removeEventListener('mouseup', onStopDragSelecting); } @@ -231,6 +231,9 @@ export function DataTable( shiftPressed, ); break; + case 'Escape': + tableManager.clearSelection(); + break; default: handled = false; } @@ -242,48 +245,54 @@ export function DataTable( [dataSource, tableManager], ); + const [debouncedSetFilter] = useState(() => { + // we don't want to trigger filter changes too quickly, as they can be pretty expensive + // and would block the user from entering text in the search bar for example + // (and in the future would really benefit from concurrent mode here :)) + const setFilter = (search: string, columns: DataTableColumn[]) => { + dataSource.view.setFilter(computeDataTableFilter(search, columns)); + }; + return props._testHeight ? setFilter : debounce(setFilter, 250); + }); useEffect( function updateFilter() { - dataSource.view.setFilter( - computeDataTableFilter(state.searchValue, state.columns), - ); + debouncedSetFilter(tableState.searchValue, tableState.columns); }, // Important dep optimization: we don't want to recalc filters if just the width or visibility changes! // We pass entire state.columns to computeDataTableFilter, but only changes in the filter are a valid cause to compute a new filter function // eslint-disable-next-line - [state.searchValue, ...state.columns.map((c) => c.filters)], + [tableState.searchValue, ...tableState.columns.map((c) => c.filters)], ); useEffect( function updateSorting() { - if (state.sorting === undefined) { + if (tableState.sorting === undefined) { dataSource.view.setSortBy(undefined); dataSource.view.setReversed(false); } else { - dataSource.view.setSortBy(state.sorting.key); - dataSource.view.setReversed(state.sorting.direction === 'desc'); + dataSource.view.setSortBy(tableState.sorting.key); + dataSource.view.setReversed(tableState.sorting.direction === 'desc'); } }, - [dataSource, state.sorting], + [dataSource, tableState.sorting], ); useEffect( function triggerSelection() { onSelect?.( - getSelectedItem(dataSource, state.selection), - getSelectedItems(dataSource, state.selection), + getSelectedItem(dataSource, tableState.selection), + getSelectedItems(dataSource, tableState.selection), ); }, - [onSelect, dataSource, state.selection], + [onSelect, dataSource, tableState.selection], ); // The initialScrollPosition is used to both capture the initial px we want to scroll to, // and whether we performed that scrolling already (if so, it will be 0) - // const initialScrollPosition = useRef(scrollOffset.current); useLayoutEffect( function scrollSelectionIntoView() { - if (state.initialOffset) { - virtualizerRef.current?.scrollToOffset(state.initialOffset); + if (tableState.initialOffset) { + virtualizerRef.current?.scrollToOffset(tableState.initialOffset); dispatch({ type: 'appliedInitialScroll', }); @@ -305,7 +314,6 @@ export function DataTable( const onRangeChange = useCallback( (start: number, end: number, total: number, offset) => { - // TODO: figure out if we don't trigger this callback to often hurting perf setRange(`${start} - ${end} / ${total}`); lastOffset.current = offset; clearTimeout(hideRange.current!); @@ -326,10 +334,10 @@ export function DataTable( dataSource, dispatch, selection, - state.columns, + tableState.columns, visibleColumns, ), - [dataSource, dispatch, selection, state.columns, visibleColumns], + [dataSource, dispatch, selection, tableState.columns, visibleColumns], ); useEffect(function initialSetup() { @@ -371,8 +379,8 @@ export function DataTable( > dataSource={dataSource} - autoScroll={props.autoScroll} - useFixedRowHeight={!state.usesWrapping} + autoScroll={props.autoScroll && !dragging.current} + useFixedRowHeight={!tableState.usesWrapping} defaultRowHeight={DEFAULT_ROW_HEIGHT} context={renderingConfig} itemRenderer={itemRenderer}