From 566f2bf96e00e888132fe63e27d216467cfe457e Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Mon, 9 Sep 2019 06:11:54 -0700 Subject: [PATCH] Do not use custom serializer for all the plugins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: This diff solves the problem where the export for the graphql plugin was super super super sloooooowwww...... The reason being that the graphql plugin had chunky graphql responses which were json blob which was being serialized by our custom serializer. Instead of serializing those with custom serializer we can directly serialize them as they won't have any map's, sets, classes etc. This diff adds the two static functions on the plugin which will provide the serialized and deserialized object for the persistedstate. As the plugin knows the structure of its state it can optimize the serialization and deserialization of its data. This change solves the slow export issue and makes it blazing fast..... 🏎 Bug: {F206550514} Reviewed By: danielbuechele Differential Revision: D17166054 fbshipit-source-id: 058b903c03c12c9194702162c46763ef5b5e7283 --- flow-typed/flipper.js | 8 + .../__tests__/headlessIntegrationTests.js | 5 +- src/devices/BaseDevice.tsx | 2 +- src/dispatcher/application.tsx | 14 +- src/index.tsx | 1 + src/plugin.tsx | 19 ++- src/utils/__tests__/exportData.electron.js | 76 ++++++---- src/utils/exportData.tsx | 138 +++++++++++++++--- 8 files changed, 204 insertions(+), 59 deletions(-) diff --git a/flow-typed/flipper.js b/flow-typed/flipper.js index ae6bb3e3d..79f67fe34 100644 --- a/flow-typed/flipper.js +++ b/flow-typed/flipper.js @@ -94,6 +94,14 @@ declare module 'flipper' { persistedState: ?PersistedState, store: ?MiddlewareAPI, ) => Promise; + static serializePersistedState: ( + persistedState: PersistedState, + statusUpdate?: (msg: string) => void, + idler?: Idler, + ) => Promise; + static deserializePersistedState: ( + serializedString: string, + ) => PersistedState; static getActiveNotifications: ?( persistedState: PersistedState, ) => Array; diff --git a/headless-tests/__tests__/headlessIntegrationTests.js b/headless-tests/__tests__/headlessIntegrationTests.js index 8a0a84a62..bac72c1b7 100644 --- a/headless-tests/__tests__/headlessIntegrationTests.js +++ b/headless-tests/__tests__/headlessIntegrationTests.js @@ -87,7 +87,7 @@ const runHeadless = memoize( }, ); -function getPluginState(app: string, plugin: string): Promise { +function getPluginState(app: string, plugin: string): Promise { return runHeadless(basicArgs).then(result => { const pluginStates = result.output.store.pluginStates; for (const pluginId of Object.keys(pluginStates)) { @@ -225,7 +225,8 @@ test('test layout snapshot stripping', () => { }); test('Sample app layout hierarchy matches snapshot', () => { - return getPluginState('Flipper', 'Inspector').then(state => { + return getPluginState('Flipper', 'Inspector').then(result => { + const state = JSON.parse(result); expect(state.rootAXElement).toBe('com.facebook.flipper.sample'); expect(state.rootElement).toBe('com.facebook.flipper.sample'); const canonicalizedElements = Object.values(state.elements) diff --git a/src/devices/BaseDevice.tsx b/src/devices/BaseDevice.tsx index 851f7d7ef..0dd728b2f 100644 --- a/src/devices/BaseDevice.tsx +++ b/src/devices/BaseDevice.tsx @@ -41,7 +41,7 @@ export type DeviceType = | 'archivedPhysical'; export type DeviceExport = { - os: string; + os: OS; title: string; deviceType: DeviceType; serial: string; diff --git a/src/dispatcher/application.tsx b/src/dispatcher/application.tsx index 3b982e97c..c10705bd3 100644 --- a/src/dispatcher/application.tsx +++ b/src/dispatcher/application.tsx @@ -51,14 +51,14 @@ export default (store: Store, logger: Logger) => { }); }); - ipcRenderer.on('flipper-protocol-handler', (event, url) => { - if (url.startsWith('flipper://import')) { - const {search} = new URL(url); - const download = qs.parse(search) ? qs.parse(search) : undefined; + ipcRenderer.on('flipper-protocol-handler', (event, query: string) => { + if (query.startsWith('flipper://import')) { + const {search} = new URL(query); + const {url} = qs.parse(search); store.dispatch(toggleAction('downloadingImportData', true)); return ( - download && - fetch(String(download)) + typeof url === 'string' && + fetch(url) .then(res => res.text()) .then(data => importDataToStore(data, store)) .then(() => { @@ -70,7 +70,7 @@ export default (store: Store, logger: Logger) => { }) ); } - const match = uriComponents(url); + const match = uriComponents(query); if (match.length > 1) { // flipper://// return store.dispatch( diff --git a/src/index.tsx b/src/index.tsx index d4ae747e0..5d1c90517 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -26,6 +26,7 @@ export {connect} from 'react-redux'; export {selectPlugin} from './reducers/connections'; export {writeBufferToFile, bufferToBlob} from './utils/screenshot'; export {getPluginKey, getPersistedState} from './utils/pluginUtils'; +export {Idler} from './utils/Idler'; export {Store, MiddlewareAPI} from './reducers/index'; export {default as BaseDevice} from './devices/BaseDevice'; export { diff --git a/src/plugin.tsx b/src/plugin.tsx index fc512223b..a7f188360 100644 --- a/src/plugin.tsx +++ b/src/plugin.tsx @@ -13,7 +13,8 @@ import {Store, MiddlewareAPI} from './reducers/index'; import {MetricType} from './utils/exportMetrics'; import {ReactNode, Component} from 'react'; import BaseDevice from './devices/BaseDevice'; - +import {serialize, deserialize} from './utils/serialization'; +import {Idler} from './utils/Idler'; type Parameters = any; // This function is intended to be called from outside of the plugin. @@ -135,6 +136,22 @@ export abstract class FlipperBasePlugin< // methods to be overriden by plugins init(): void {} + static serializePersistedState: ( + persistedState: StaticPersistedState, + statusUpdate?: (msg: string) => void, + idler?: Idler, + ) => Promise = ( + persistedState: StaticPersistedState, + statusUpdate?: (msg: string) => void, + idler?: Idler, + ) => { + return serialize(persistedState, idler, statusUpdate); + }; + static deserializePersistedState: ( + serializedString: string, + ) => StaticPersistedState = (serializedString: string) => { + return deserialize(serializedString); + }; teardown(): void {} computeNotifications( _props: Props, diff --git a/src/utils/__tests__/exportData.electron.js b/src/utils/__tests__/exportData.electron.js index cd83bc8b5..248fa7de5 100644 --- a/src/utils/__tests__/exportData.electron.js +++ b/src/utils/__tests__/exportData.electron.js @@ -156,6 +156,7 @@ test('test processStore function for empty state', () => { pluginStates: {}, clients: [], devicePlugins: new Map(), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -169,6 +170,7 @@ test('test processStore function for an iOS device connected', async () => { pluginStates: {}, clients: [], devicePlugins: new Map(), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -198,18 +200,24 @@ test('test processStore function for an iOS device connected with client plugin const json = await processStore({ activeNotifications: [], device, - pluginStates: {[clientIdentifier]: {msg: 'Test plugin'}}, + pluginStates: { + [`${clientIdentifier}#TestDevicePlugin`]: {msg: 'Test plugin'}, + }, clients: [generateClientFromDevice(device, 'testapp')], devicePlugins: new Map(), + clientPlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), salt: 'salt', selectedPlugins: [], }); expect(json).toBeDefined(); const {pluginStates} = json.store; const expectedPluginState = { - [generateClientIdentifierWithSalt(clientIdentifier, 'salt')]: { + [`${generateClientIdentifierWithSalt( + clientIdentifier, + 'salt', + )}#TestDevicePlugin`]: JSON.stringify({ msg: 'Test plugin', - }, + }), }; expect(pluginStates).toEqual(expectedPluginState); }); @@ -246,10 +254,10 @@ test('test processStore function to have only the client for the selected device activeNotifications: [], device: selectedDevice, pluginStates: { - [unselectedDeviceClientIdentifier + '#testapp']: { + [unselectedDeviceClientIdentifier + '#TestDevicePlugin']: { msg: 'Test plugin unselected device', }, - [selectedDeviceClientIdentifier + '#testapp']: { + [selectedDeviceClientIdentifier + '#TestDevicePlugin']: { msg: 'Test plugin selected device', }, }, @@ -258,6 +266,7 @@ test('test processStore function to have only the client for the selected device generateClientFromDevice(unselectedDevice, 'testapp'), ], devicePlugins: new Map(), + clientPlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), salt: 'salt', selectedPlugins: [], }); @@ -267,9 +276,9 @@ test('test processStore function to have only the client for the selected device const {pluginStates} = json.store; const expectedPluginState = { [generateClientIdentifierWithSalt(selectedDeviceClientIdentifier, 'salt') + - '#testapp']: { + '#TestDevicePlugin']: JSON.stringify({ msg: 'Test plugin selected device', - }, + }), }; expect(clients).toEqual([ generateClientFromClientWithSalt(selectedDeviceClient, 'salt'), @@ -302,10 +311,10 @@ test('test processStore function to have multiple clients for the selected devic activeNotifications: [], device: selectedDevice, pluginStates: { - [clientIdentifierApp1 + '#testapp1']: { + [clientIdentifierApp1 + '#TestDevicePlugin']: { msg: 'Test plugin App1', }, - [clientIdentifierApp2 + '#testapp2']: { + [clientIdentifierApp2 + '#TestDevicePlugin']: { msg: 'Test plugin App2', }, }, @@ -314,6 +323,7 @@ test('test processStore function to have multiple clients for the selected devic generateClientFromDevice(selectedDevice, 'testapp2'), ], devicePlugins: new Map(), + clientPlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), salt: 'salt', selectedPlugins: [], }); @@ -322,13 +332,13 @@ test('test processStore function to have multiple clients for the selected devic const {pluginStates} = json.store; const expectedPluginState = { [generateClientIdentifierWithSalt(clientIdentifierApp1, 'salt') + - '#testapp1']: { + '#TestDevicePlugin']: JSON.stringify({ msg: 'Test plugin App1', - }, + }), [generateClientIdentifierWithSalt(clientIdentifierApp2, 'salt') + - '#testapp2']: { + '#TestDevicePlugin']: JSON.stringify({ msg: 'Test plugin App2', - }, + }), }; expect(clients).toEqual([ generateClientFromClientWithSalt(client1, 'salt'), @@ -356,6 +366,7 @@ test('test processStore function for device plugin state and no clients', async }, clients: [], devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -363,7 +374,7 @@ test('test processStore function for device plugin state and no clients', async const {pluginStates} = json.store; const {clients} = json; const expectedPluginState = { - 'salt-serial#TestDevicePlugin': {msg: 'Test Device plugin'}, + 'salt-serial#TestDevicePlugin': JSON.stringify({msg: 'Test Device plugin'}), }; expect(pluginStates).toEqual(expectedPluginState); expect(clients).toEqual([]); @@ -388,6 +399,7 @@ test('test processStore function for unselected device plugin state and no clien }, clients: [], devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -425,6 +437,7 @@ test('test processStore function for notifications for selected device', async ( pluginStates: {}, clients: [client], devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -482,6 +495,7 @@ test('test processStore function for notifications for unselected device', async pluginStates: {}, clients: [client, unselectedclient], devicePlugins: new Map(), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -505,10 +519,10 @@ test('test processStore function for selected plugins', async () => { const client = generateClientFromDevice(selectedDevice, 'app'); const pluginstates = { - [generateClientIdentifier(selectedDevice, 'app') + '#plugin1']: { + [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin1']: { msg: 'Test plugin1', }, - [generateClientIdentifier(selectedDevice, 'app') + '#plugin2']: { + [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin2']: { msg: 'Test plugin2', }, }; @@ -517,9 +531,13 @@ test('test processStore function for selected plugins', async () => { device: selectedDevice, pluginStates: pluginstates, clients: [client], - devicePlugins: new Map(), + devicePlugins: new Map([ + ['TestDevicePlugin1', TestDevicePlugin], + ['TestDevicePlugin2', TestDevicePlugin], + ]), + clientPlugins: new Map(), salt: 'salt', - selectedPlugins: ['plugin2'], + selectedPlugins: ['TestDevicePlugin2'], }); expect(json).toBeDefined(); const {pluginStates} = json.store; @@ -528,9 +546,9 @@ test('test processStore function for selected plugins', async () => { [generateClientIdentifierWithSalt( generateClientIdentifier(selectedDevice, 'app'), 'salt', - ) + '#plugin2']: { + ) + '#TestDevicePlugin2']: JSON.stringify({ msg: 'Test plugin2', - }, + }), }); expect(clients).toEqual([generateClientFromClientWithSalt(client, 'salt')]); const {activeNotifications} = json.store; @@ -547,10 +565,10 @@ test('test processStore function for no selected plugins', async () => { ); const client = generateClientFromDevice(selectedDevice, 'app'); const pluginstates = { - [generateClientIdentifier(selectedDevice, 'app') + '#plugin1']: { + [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin1']: { msg: 'Test plugin1', }, - [generateClientIdentifier(selectedDevice, 'app') + '#plugin2']: { + [generateClientIdentifier(selectedDevice, 'app') + '#TestDevicePlugin2']: { msg: 'Test plugin2', }, }; @@ -559,7 +577,11 @@ test('test processStore function for no selected plugins', async () => { device: selectedDevice, pluginStates: pluginstates, clients: [client], - devicePlugins: new Map(), + devicePlugins: new Map([ + ['TestDevicePlugin1', TestDevicePlugin], + ['TestDevicePlugin2', TestDevicePlugin], + ]), + clientPlugins: new Map(), salt: 'salt', selectedPlugins: [], }); @@ -571,15 +593,15 @@ test('test processStore function for no selected plugins', async () => { [generateClientIdentifierWithSalt( generateClientIdentifier(selectedDevice, 'app'), 'salt', - ) + '#plugin2']: { + ) + '#TestDevicePlugin2']: JSON.stringify({ msg: 'Test plugin2', - }, + }), [generateClientIdentifierWithSalt( generateClientIdentifier(selectedDevice, 'app'), 'salt', - ) + '#plugin1']: { + ) + '#TestDevicePlugin1']: JSON.stringify({ msg: 'Test plugin1', - }, + }), }); expect(clients).toEqual([generateClientFromClientWithSalt(client, 'salt')]); const {activeNotifications} = json.store; diff --git a/src/utils/exportData.tsx b/src/utils/exportData.tsx index d73784d94..784de2ab5 100644 --- a/src/utils/exportData.tsx +++ b/src/utils/exportData.tsx @@ -12,7 +12,12 @@ import {PluginNotification} from '../reducers/notifications'; import {ClientExport} from '../Client.js'; import {State as PluginsState} from '../reducers/plugins'; import {pluginKey} from '../reducers/pluginStates'; -import {FlipperDevicePlugin, FlipperPlugin, callClient} from '../plugin'; +import { + FlipperDevicePlugin, + FlipperPlugin, + callClient, + FlipperBasePlugin, +} from '../plugin'; import {default as BaseDevice} from '../devices/BaseDevice'; import {default as ArchivedDevice} from '../devices/ArchivedDevice'; import {default as Client} from '../Client'; @@ -28,13 +33,16 @@ import {Idler} from './Idler'; export const IMPORT_FLIPPER_TRACE_EVENT = 'import-flipper-trace'; export const EXPORT_FLIPPER_TRACE_EVENT = 'export-flipper-trace'; +export type PluginStatesExportState = { + [pluginKey: string]: string; +}; export type ExportType = { fileVersion: string; flipperReleaseRevision: string | null; clients: Array; device: DeviceExport | null; store: { - pluginStates: PluginStatesState; + pluginStates: PluginStatesExportState; activeNotifications: Array; }; }; @@ -56,11 +64,15 @@ type ProcessNotificationStatesOptions = { statusUpdate?: (msg: string) => void; }; +type SerializePluginStatesOptions = { + pluginStates: PluginStatesState; +}; + type AddSaltToDeviceSerialOptions = { salt: string; device: BaseDevice; clients: Array; - pluginStates: PluginStatesState; + pluginStates: PluginStatesExportState; pluginNotification: Array; selectedPlugins: Array; statusUpdate?: (msg: string) => void; @@ -107,7 +119,7 @@ export function processPluginStates( statusUpdate, } = options; - let pluginStates = {}; + let pluginStates: PluginStatesState = {}; statusUpdate && statusUpdate('Filtering the plugin states for the filtered Clients...'); for (const key in allPluginStates) { @@ -155,6 +167,61 @@ export function processNotificationStates( return activeNotifications; } +const serializePluginStates = async ( + pluginStates: PluginStatesState, + clientPlugins: Map, + devicePlugins: Map, + statusUpdate?: (msg: string) => void, + idler?: Idler, +): Promise => { + const pluginsMap: Map = new Map([]); + clientPlugins.forEach((val, key) => { + pluginsMap.set(key, val); + }); + devicePlugins.forEach((val, key) => { + pluginsMap.set(key, val); + }); + const pluginExportState: PluginStatesExportState = {}; + for (const key in pluginStates) { + const keyArray = key.split('#'); + const pluginName = keyArray.pop(); + statusUpdate && statusUpdate(`Serialising ${pluginName}...`); + + const pluginClass = pluginsMap.get(pluginName); + if (pluginClass) { + pluginExportState[key] = await pluginClass.serializePersistedState( + pluginStates[key], + statusUpdate, + idler, + ); + } + } + return pluginExportState; +}; + +const deserializePluginStates = ( + pluginStatesExportState: PluginStatesExportState, + clientPlugins: Map, + devicePlugins: Map, +): PluginStatesState => { + const pluginsMap: Map = new Map([]); + clientPlugins.forEach((val, key) => { + pluginsMap.set(key, val); + }); + devicePlugins.forEach((val, key) => { + pluginsMap.set(key, val); + }); + const pluginsState: PluginStatesState = {}; + for (const key in pluginStatesExportState) { + const keyArray = key.split('#'); + const pluginName = keyArray.pop(); + pluginsState[key] = pluginsMap + .get(pluginName) + .deserializePersistedState(pluginStatesExportState[key]); + } + return pluginsState; +}; + const addSaltToDeviceSerial = async ( options: AddSaltToDeviceSerialOptions, ): Promise => { @@ -235,6 +302,7 @@ type ProcessStoreOptions = { pluginStates: PluginStatesState; clients: Array; devicePlugins: Map; + clientPlugins: Map; salt: string; selectedPlugins: Array; statusUpdate?: (msg: string) => void; @@ -242,6 +310,7 @@ type ProcessStoreOptions = { export const processStore = async ( options: ProcessStoreOptions, + idler?: Idler, ): Promise => { const { activeNotifications, @@ -249,6 +318,7 @@ export const processStore = async ( pluginStates, clients, devicePlugins, + clientPlugins, salt, selectedPlugins, statusUpdate, @@ -272,16 +342,25 @@ export const processStore = async ( devicePlugins, statusUpdate, }); + + const exportPluginState = await serializePluginStates( + processedPluginStates, + clientPlugins, + devicePlugins, + statusUpdate, + idler, + ); // Adding salt to the device id, so that the device_id in the device list is unique. const exportFlipperData = await addSaltToDeviceSerial({ salt, device, clients: processedClients, - pluginStates: processedPluginStates, + pluginStates: exportPluginState, pluginNotification: processedActiveNotifications, statusUpdate, selectedPlugins, }); + return exportFlipperData; } return null; @@ -340,6 +419,7 @@ export async function fetchMetadata( export async function getStoreExport( store: MiddlewareAPI, statusUpdate?: (msg: string) => void, + idler?: Idler, ): Promise<{exportData: ExportType | null; errorArray: Array}> { const state = store.getState(); const {clients} = state.connections; @@ -373,17 +453,21 @@ export async function getStoreExport( const newPluginState = metadata.pluginStates; const {activeNotifications} = store.getState().notifications; - const {devicePlugins} = store.getState().plugins; - const exportData = await processStore({ - activeNotifications, - device: selectedDevice, - pluginStates: newPluginState, - clients: clients.map(client => client.toJSON()), - devicePlugins, - salt: uuid.v4(), - selectedPlugins: store.getState().plugins.selectedPlugins, - statusUpdate, - }); + const {devicePlugins, clientPlugins} = store.getState().plugins; + const exportData = await processStore( + { + activeNotifications, + device: selectedDevice, + pluginStates: newPluginState, + clients: clients.map(client => client.toJSON()), + devicePlugins, + clientPlugins, + salt: uuid.v4(), + selectedPlugins: store.getState().plugins.selectedPlugins, + statusUpdate, + }, + idler, + ); return {exportData, errorArray}; } @@ -399,6 +483,7 @@ export function exportStore( const {exportData, errorArray} = await getStoreExport( store, statusUpdate, + idler, ); if (!exportData) { console.error('Make sure a device is connected'); @@ -443,8 +528,11 @@ export const exportStoreToFile = ( export function importDataToStore(data: string, store: Store) { getLogger().track('usage', IMPORT_FLIPPER_TRACE_EVENT); - const json = deserialize(data); + const json: ExportType = deserialize(data); const {device, clients} = json; + if (device == null) { + return; + } const {serial, deviceType, title, os, logs} = device; const archivedDevice = new ArchivedDevice( serial, @@ -474,25 +562,33 @@ export function importDataToStore(data: string, store: Store) { }); const {pluginStates} = json.store; - const keys = Object.keys(pluginStates); + const processedPluginStates: PluginStatesState = deserializePluginStates( + pluginStates, + store.getState().plugins.clientPlugins, + store.getState().plugins.devicePlugins, + ); + const keys = Object.keys(processedPluginStates); keys.forEach(key => { store.dispatch({ type: 'SET_PLUGIN_STATE', payload: { pluginKey: key, - state: pluginStates[key], + state: processedPluginStates[key], }, }); }); clients.forEach(client => { - const clientPlugins = keys + const clientPlugins: Array = keys .filter(key => { const arr = key.split('#'); arr.pop(); const clientPlugin = arr.join('#'); return client.id === clientPlugin; }) - .map(client => client.split('#').pop()); + .map(client => { + const elem = client.split('#').pop(); + return elem || ''; + }); store.dispatch({ type: 'NEW_CLIENT', payload: new Client(