Fix performance issues [1/N]

Summary:
This issue fixes a bunch of performance issues:

* The introduction of a context menu around every _row_ in D18808544 breaks the internal contract between lists and rows that react-window has, causing empty element to appear during scrolling, and some optimizations not working. Fixed by wrapping the context menu at the right level
* Every time a new row is created for the listview, it gets fresh event handlers and column configurations. THis has been fixed by precomputing the column configuration and avoiding the need to close over the row data in the event handlers
* Added some stricter immutable typings to make sure we don't break the immutable contract somewhere
* Fix the introduction of on the fly styling generation, which isn't needed

Reviewed By: passy

Differential Revision: D19853595

fbshipit-source-id: dc82b6586889f4e8c7a437cfdc27a50dc33ba2a2
This commit is contained in:
Michel Weststrate
2020-02-20 04:37:53 -08:00
committed by Facebook Github Bot
parent a05b8d1ba2
commit 2dad8809c4
5 changed files with 100 additions and 76 deletions

View File

@@ -21,13 +21,13 @@ export type LogLevel =
| 'fatal'; | 'fatal';
export type DeviceLogEntry = { export type DeviceLogEntry = {
date: Date; readonly date: Date;
pid: number; readonly pid: number;
tid: number; readonly tid: number;
app?: string; readonly app?: string;
type: LogLevel; readonly type: LogLevel;
tag: string; readonly tag: string;
message: string; readonly message: string;
}; };
export type DeviceShell = { export type DeviceShell = {

View File

@@ -24,20 +24,20 @@ import {
import React from 'react'; import React from 'react';
export type Counter = { export type Counter = {
expression: RegExp; readonly expression: RegExp;
count: number; readonly count: number;
notify: boolean; readonly notify: boolean;
label: string; readonly label: string;
}; };
type Props = { type Props = {
onChange: (counters: Array<Counter>) => void; readonly onChange: (counters: ReadonlyArray<Counter>) => void;
counters: Array<Counter>; readonly counters: ReadonlyArray<Counter>;
}; };
type State = { type State = {
input: string; readonly input: string;
highlightedRow: string | null; readonly highlightedRow: string | null;
}; };
const ColumnSizes = { const ColumnSizes = {

View File

@@ -40,20 +40,20 @@ import {MenuTemplate} from 'src/ui/components/ContextMenu';
const LOG_WATCHER_LOCAL_STORAGE_KEY = 'LOG_WATCHER_LOCAL_STORAGE_KEY'; const LOG_WATCHER_LOCAL_STORAGE_KEY = 'LOG_WATCHER_LOCAL_STORAGE_KEY';
type Entries = Array<{ type Entries = ReadonlyArray<{
row: TableBodyRow; readonly row: TableBodyRow;
entry: DeviceLogEntry; readonly entry: DeviceLogEntry;
}>; }>;
type BaseState = { type BaseState = {
rows: Array<TableBodyRow>; readonly rows: ReadonlyArray<TableBodyRow>;
entries: Entries; readonly entries: Entries;
key2entry: {[key: string]: DeviceLogEntry}; readonly key2entry: {readonly [key: string]: DeviceLogEntry};
}; };
type AdditionalState = { type AdditionalState = {
highlightedRows: Set<string>; readonly highlightedRows: ReadonlySet<string>;
counters: Array<Counter>; readonly counters: ReadonlyArray<Counter>;
}; };
type State = BaseState & AdditionalState; type State = BaseState & AdditionalState;
@@ -95,7 +95,7 @@ const COLUMN_SIZE = {
tag: 120, tag: 120,
app: 200, app: 200,
message: 'flex', message: 'flex',
}; } as const;
const COLUMNS = { const COLUMNS = {
type: { type: {
@@ -119,7 +119,7 @@ const COLUMNS = {
message: { message: {
value: 'Message', value: 'Message',
}, },
}; } as const;
const INITIAL_COLUMN_ORDER = [ const INITIAL_COLUMN_ORDER = [
{ {
@@ -150,7 +150,7 @@ const INITIAL_COLUMN_ORDER = [
key: 'message', key: 'message',
visible: true, visible: true,
}, },
]; ] as const;
const LOG_TYPES: { const LOG_TYPES: {
[level: string]: { [level: string]: {
@@ -260,7 +260,7 @@ export function addEntriesToState(
rows: [], rows: [],
entries: [], entries: [],
key2entry: {}, key2entry: {},
}, } as const,
): BaseState { ): BaseState {
const rows = [...state.rows]; const rows = [...state.rows];
const entries = [...state.entries]; const entries = [...state.entries];
@@ -421,7 +421,7 @@ export default class LogTable extends FlipperDevicePlugin<
calculateHighlightedRows = ( calculateHighlightedRows = (
deepLinkPayload: string | null, deepLinkPayload: string | null,
rows: Array<TableBodyRow>, rows: ReadonlyArray<TableBodyRow>,
): Set<string> => { ): Set<string> => {
const highlightedRows = new Set<string>(); const highlightedRows = new Set<string>();
if (!deepLinkPayload) { if (!deepLinkPayload) {
@@ -459,7 +459,10 @@ export default class LogTable extends FlipperDevicePlugin<
columnOrder: TableColumnOrder; columnOrder: TableColumnOrder;
logListener: Symbol | undefined; logListener: Symbol | undefined;
batch: Entries = []; batch: Array<{
readonly row: TableBodyRow;
readonly entry: DeviceLogEntry;
}> = [];
queued: boolean = false; queued: boolean = false;
counter: number = 0; counter: number = 0;

View File

@@ -34,6 +34,9 @@ import {DEFAULT_ROW_HEIGHT} from './types';
import textContent from '../../../utils/textContent'; import textContent from '../../../utils/textContent';
import {notNull} from '../../../utils/typeUtils'; import {notNull} from '../../../utils/typeUtils';
const EMPTY_OBJECT = {};
Object.freeze(EMPTY_OBJECT);
export type ManagedTableProps = { export type ManagedTableProps = {
/** /**
* Column definitions. * Column definitions.
@@ -140,6 +143,7 @@ type ManagedTableState = {
highlightedRows: Set<string>; highlightedRows: Set<string>;
sortOrder?: TableRowSortOrder; sortOrder?: TableRowSortOrder;
columnOrder: TableColumnOrder; columnOrder: TableColumnOrder;
columnKeys: string[];
columnSizes: TableColumnSizes; columnSizes: TableColumnSizes;
shouldScrollToBottom: boolean; shouldScrollToBottom: boolean;
}; };
@@ -172,20 +176,6 @@ export class ManagedTable extends React.Component<
); );
}; };
state: ManagedTableState = {
columnOrder:
JSON.parse(window.localStorage.getItem(this.getTableKey()) || 'null') ||
this.props.columnOrder ||
Object.keys(this.props.columns).map(key => ({key, visible: true})),
columnSizes:
this.props.tableKey && globalTableState[this.props.tableKey]
? globalTableState[this.props.tableKey]
: this.props.columnSizes || {},
highlightedRows: this.props.highlightedRows || new Set(),
sortOrder: this.props.initialSortOrder || undefined,
shouldScrollToBottom: Boolean(this.props.stickyBottom),
};
tableRef = React.createRef<List>(); tableRef = React.createRef<List>();
scrollRef: { scrollRef: {
@@ -200,6 +190,25 @@ export class ManagedTable extends React.Component<
// trigger actions on the first update instead. // trigger actions on the first update instead.
firstUpdate = true; firstUpdate = true;
constructor(props: ManagedTableProps) {
super(props);
const columnOrder =
JSON.parse(window.localStorage.getItem(this.getTableKey()) || 'null') ||
this.props.columnOrder ||
Object.keys(this.props.columns).map(key => ({key, visible: true}));
this.state = {
columnOrder,
columnKeys: this.computeColumnKeys(columnOrder),
columnSizes:
this.props.tableKey && globalTableState[this.props.tableKey]
? globalTableState[this.props.tableKey]
: this.props.columnSizes || {},
highlightedRows: this.props.highlightedRows || new Set(),
sortOrder: this.props.initialSortOrder || undefined,
shouldScrollToBottom: Boolean(this.props.stickyBottom),
};
}
componentDidMount() { componentDidMount() {
document.addEventListener('keydown', this.onKeyDown); document.addEventListener('keydown', this.onKeyDown);
} }
@@ -233,6 +242,7 @@ export class ManagedTable extends React.Component<
} }
this.setState({ this.setState({
columnOrder: nextProps.columnOrder, columnOrder: nextProps.columnOrder,
columnKeys: this.computeColumnKeys(nextProps.columnOrder),
}); });
} }
@@ -277,6 +287,10 @@ export class ManagedTable extends React.Component<
this.firstUpdate = false; this.firstUpdate = false;
} }
computeColumnKeys(columnOrder: TableColumnOrder) {
return columnOrder.map(k => (k.visible ? k.key : null)).filter(notNull);
}
scrollToHighlightedRows = () => { scrollToHighlightedRows = () => {
const {current} = this.tableRef; const {current} = this.tableRef;
const {highlightedRows} = this.state; const {highlightedRows} = this.state;
@@ -609,34 +623,24 @@ export class ManagedTable extends React.Component<
getRow = ({index, style}: {index: number; style: React.CSSProperties}) => { getRow = ({index, style}: {index: number; style: React.CSSProperties}) => {
const {onAddFilter, multiline, zebra, rows} = this.props; const {onAddFilter, multiline, zebra, rows} = this.props;
const {columnOrder, columnSizes, highlightedRows} = this.state; const {columnKeys, columnSizes, highlightedRows} = this.state;
const columnKeys: Array<string> = columnOrder
.map(k => (k.visible ? k.key : null))
.filter(notNull);
return ( return (
<ContextMenu
buildItems={
this.props.buildContextMenuItems || this.buildContextMenuItems
}>
<TableRow <TableRow
key={rows[index].key} key={rows[index].key}
columnSizes={columnSizes} columnSizes={columnSizes}
columnKeys={columnKeys} columnKeys={columnKeys}
onMouseDown={e => this.onHighlight(e, rows[index], index)} onMouseDown={this.onHighlight}
onMouseEnter={e => this.onMouseEnterRow(e, rows[index], index)} onMouseEnter={this.onMouseEnterRow}
multiline={multiline} multiline={multiline}
rowLineHeight={24} rowLineHeight={24}
highlighted={highlightedRows.has(rows[index].key)} highlighted={highlightedRows.has(rows[index].key)}
row={rows[index]} row={rows[index]}
index={index} index={index}
style={ style={style}
rows[index].height ? {...style, height: rows[index].height} : style
}
onAddFilter={onAddFilter} onAddFilter={onAddFilter}
zebra={zebra} zebra={zebra}
/> />
</ContextMenu>
); );
}; };
@@ -686,7 +690,14 @@ export class ManagedTable extends React.Component<
)} )}
<Container> <Container>
{this.props.autoHeight ? ( {this.props.autoHeight ? (
this.props.rows.map((_, index) => this.getRow({index, style: {}})) <ContextMenu
buildItems={
this.props.buildContextMenuItems || this.buildContextMenuItems
}>
{this.props.rows.map((_, index) =>
this.getRow({index, style: EMPTY_OBJECT}),
)}
</ContextMenu>
) : ( ) : (
<AutoSizer> <AutoSizer>
{({width, height}) => ( {({width, height}) => (

View File

@@ -115,8 +115,12 @@ TableBodyColumnContainer.displayName = 'TableRow:TableBodyColumnContainer';
type Props = { type Props = {
columnSizes: TableColumnSizes; columnSizes: TableColumnSizes;
columnKeys: TableColumnKeys; columnKeys: TableColumnKeys;
onMouseDown: (e: React.MouseEvent) => any; onMouseDown: (e: React.MouseEvent, row: TableBodyRow, index: number) => void;
onMouseEnter?: (e: React.MouseEvent) => void; onMouseEnter?: (
e: React.MouseEvent,
row: TableBodyRow,
index: number,
) => void;
multiline?: boolean; multiline?: boolean;
rowLineHeight: number; rowLineHeight: number;
highlighted: boolean; highlighted: boolean;
@@ -132,6 +136,14 @@ export default class TableRow extends React.PureComponent<Props> {
zebra: true, zebra: true,
}; };
handleMouseDown = (e: React.MouseEvent) => {
this.props.onMouseDown(e, this.props.row, this.props.index);
};
handleMouseEnter = (e: React.MouseEvent) => {
this.props.onMouseEnter?.(e, this.props.row, this.props.index);
};
render() { render() {
const { const {
index, index,
@@ -142,8 +154,6 @@ export default class TableRow extends React.PureComponent<Props> {
multiline, multiline,
columnKeys, columnKeys,
columnSizes, columnSizes,
onMouseEnter,
onMouseDown,
zebra, zebra,
onAddFilter, onAddFilter,
} = this.props; } = this.props;
@@ -157,8 +167,8 @@ export default class TableRow extends React.PureComponent<Props> {
multiline={multiline} multiline={multiline}
even={index % 2 === 0} even={index % 2 === 0}
zebra={zebra} zebra={zebra}
onMouseDown={onMouseDown} onMouseDown={this.handleMouseDown}
onMouseEnter={onMouseEnter} onMouseEnter={this.handleMouseEnter}
style={style} style={style}
highlightOnHover={row.highlightOnHover} highlightOnHover={row.highlightOnHover}
data-key={row.key} data-key={row.key}