From 210c2371cf20b17e5c25267f375abdb9ec6172c2 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 13 Mar 2019 17:00:44 -0700 Subject: [PATCH] Naive pass at commit durationfiltering --- src/devtools/views/Profiler/FilterModal.js | 33 ++++----- .../views/Profiler/ProfilerContext.js | 22 ++++++ .../views/Profiler/SnapshotCommitList.js | 13 ++-- .../views/Profiler/SnapshotSelector.css | 8 +++ .../views/Profiler/SnapshotSelector.js | 70 +++++++++++++------ 5 files changed, 101 insertions(+), 45 deletions(-) diff --git a/src/devtools/views/Profiler/FilterModal.js b/src/devtools/views/Profiler/FilterModal.js index 34a25e52a7..8e91ef0a0d 100644 --- a/src/devtools/views/Profiler/FilterModal.js +++ b/src/devtools/views/Profiler/FilterModal.js @@ -1,7 +1,8 @@ // @flow -import React, { useCallback, useEffect, useRef } from 'react'; -import { useLocalStorage, useModalDismissSignal } from '../hooks'; +import React, { useCallback, useContext, useEffect, useRef } from 'react'; +import { useModalDismissSignal } from '../hooks'; +import { ProfilerContext } from './ProfilerContext'; import styles from './FilterModal.css'; @@ -10,33 +11,33 @@ type Props = {| |}; export default function FilterModal({ dismissModal }: Props) { - const [isEnabled, setIsEnabled] = useLocalStorage( - 'minCommitDurationFilterEnabled', - false - ); - const [value, setValue] = useLocalStorage( - 'minCommitDurationFilter', - 0 - ); + const { + isMinCommitDurationEnabled, + minCommitDuration, + setMinCommitDuration, + setIsMinCommitDurationEnabled, + } = useContext(ProfilerContext); const handleNumberChange = useCallback( ({ currentTarget }) => { const newValue = parseInt(currentTarget.value, 10); - setValue(Number.isNaN(newValue) || newValue <= 0 ? 0 : newValue); + setMinCommitDuration( + Number.isNaN(newValue) || newValue <= 0 ? 0 : newValue + ); }, - [setValue] + [setMinCommitDuration] ); const handleEnabledChange = useCallback( ({ currentTarget }) => { - setIsEnabled(currentTarget.checked); + setIsMinCommitDurationEnabled(currentTarget.checked); if (currentTarget.checked) { if (inputRef.current !== null) { inputRef.current.focus(); } } }, - [setIsEnabled] + [setIsMinCommitDurationEnabled] ); const inputRef = useRef(null); @@ -55,7 +56,7 @@ export default function FilterModal({ dismissModal }: Props) {
diff --git a/src/devtools/views/Profiler/ProfilerContext.js b/src/devtools/views/Profiler/ProfilerContext.js index 467d8247a2..9b02eed690 100644 --- a/src/devtools/views/Profiler/ProfilerContext.js +++ b/src/devtools/views/Profiler/ProfilerContext.js @@ -10,15 +10,20 @@ import React, { import { useSubscription } from '../hooks'; import { TreeContext } from 'src/devtools/views/Elements/TreeContext'; import { StoreContext } from '../context'; +import { useLocalStorage } from '../hooks'; import Store from '../../store'; type Context = {| commitIndex: number, hasProfilingData: boolean, + isMinCommitDurationEnabled: boolean, isProfiling: boolean, + minCommitDuration: number, rendererID: number | null, rootID: number | null, setCommitIndex: (value: number) => void, + setMinCommitDuration: (value: number) => void, + setIsMinCommitDurationEnabled: (value: boolean) => void, startProfiling(value: boolean): void, stopProfiling(value: boolean): void, |}; @@ -94,13 +99,26 @@ function ProfilerContextController({ children }: Props) { setCommitIndex(0); } + const [ + isMinCommitDurationEnabled, + setIsMinCommitDurationEnabled, + ] = useLocalStorage('isMinCommitDurationEnabled', false); + const [minCommitDuration, setMinCommitDuration] = useLocalStorage( + 'minCommitDuration', + 0 + ); + const value = useMemo( () => ({ commitIndex, hasProfilingData, + isMinCommitDurationEnabled, isProfiling, + minCommitDuration, rendererID, rootID, + setMinCommitDuration, + setIsMinCommitDurationEnabled, setCommitIndex, startProfiling, stopProfiling, @@ -108,9 +126,13 @@ function ProfilerContextController({ children }: Props) { [ commitIndex, hasProfilingData, + isMinCommitDurationEnabled, isProfiling, + minCommitDuration, rendererID, rootID, + setMinCommitDuration, + setIsMinCommitDurationEnabled, setCommitIndex, startProfiling, stopProfiling, diff --git a/src/devtools/views/Profiler/SnapshotCommitList.js b/src/devtools/views/Profiler/SnapshotCommitList.js index 4f4fe92661..69a10e3e18 100644 --- a/src/devtools/views/Profiler/SnapshotCommitList.js +++ b/src/devtools/views/Profiler/SnapshotCommitList.js @@ -12,8 +12,6 @@ import { FixedSizeList } from 'react-window'; import SnapshotCommitListItem from './SnapshotCommitListItem'; import { minBarWidth } from './constants'; -import type { ProfilingSummary } from './types'; - export type ItemData = {| commitDurations: Array, commitTimes: Array, @@ -24,7 +22,8 @@ export type ItemData = {| |}; type Props = {| - profilingSummary: ProfilingSummary, + commitDurations: Array, + commitTimes: Array, selectedCommitIndex: number, setCommitIndex: (index: number) => void, viewNextCommit: () => void, @@ -41,7 +40,8 @@ export default function SnapshotCommitList(props: Props) { type ListProps = {| height: number, - profilingSummary: ProfilingSummary, + commitDurations: Array, + commitTimes: Array, selectedCommitIndex: number, setCommitIndex: (index: number) => void, viewNextCommit: () => void, @@ -51,7 +51,8 @@ type ListProps = {| function List({ height, - profilingSummary, + commitDurations, + commitTimes, selectedCommitIndex, setCommitIndex, viewNextCommit, @@ -86,8 +87,6 @@ function List({ }; }, [handleMouseUp]); - const { commitDurations, commitTimes } = profilingSummary; - const itemSize = useMemo( () => Math.max(minBarWidth, width / commitDurations.length), [commitDurations, width] diff --git a/src/devtools/views/Profiler/SnapshotSelector.css b/src/devtools/views/Profiler/SnapshotSelector.css index b047febe26..48f11a2660 100644 --- a/src/devtools/views/Profiler/SnapshotSelector.css +++ b/src/devtools/views/Profiler/SnapshotSelector.css @@ -29,3 +29,11 @@ white-space: nowrap; font-family: var(--font-family-monospace); } + +.NoCommits { + height: 100%; + display: flex; + align-items: center; + justify-content: center; + color: var(--color-dim); +} diff --git a/src/devtools/views/Profiler/SnapshotSelector.js b/src/devtools/views/Profiler/SnapshotSelector.js index e63da13173..6146aaa1d1 100644 --- a/src/devtools/views/Profiler/SnapshotSelector.js +++ b/src/devtools/views/Profiler/SnapshotSelector.js @@ -21,26 +21,47 @@ export default function SnapshotSelectorSuspense(_: Props) { function SnapshotSelector(_: Props) { const { profilingCache } = useContext(StoreContext); - const { commitIndex, rendererID, rootID, setCommitIndex } = useContext( - ProfilerContext - ); + const { + commitIndex, + isMinCommitDurationEnabled, + minCommitDuration, + rendererID, + rootID, + setCommitIndex, + } = useContext(ProfilerContext); if (rendererID === null || rootID === null) { return null; } - // TODO (profiling) Parse the summary into something easier for the views to work with - const profilingSummary = profilingCache.ProfilingSummary.read({ - rendererID: ((rendererID: any): number), - rootID: ((rootID: any): number), - }); + const { commitDurations, commitTimes } = profilingCache.ProfilingSummary.read( + { + rendererID: ((rendererID: any): number), + rootID: ((rootID: any): number), + } + ); - const numCommits = profilingSummary.commitDurations.length; - - if (numCommits === 0) { - return null; + // TODO (profiling) This is not sufficient; index here doesn't map to a meaningful index in the profiling data. + let filteredCommitDurations = commitDurations; + let filteredCommitTimes = commitTimes; + if (isMinCommitDurationEnabled) { + filteredCommitDurations = []; + filteredCommitTimes = []; + for (let i = 0; i < commitDurations.length; i++) { + if (commitDurations[i] >= minCommitDuration) { + filteredCommitDurations.push(commitDurations[i]); + filteredCommitTimes.push(commitTimes[i]); + } + } } + const numCommits = filteredCommitDurations.length; + const currentCommitNumber = `${ + numCommits > 0 ? commitIndex + 1 : '-' + }`.padStart(`${numCommits}`.length, '0'); + + // TODO (profiler) We need to guard commit index and share filterd statuses in a better way. + const viewNextCommit = () => { setCommitIndex(Math.min(commitIndex + 1, numCommits - 1)); }; @@ -53,24 +74,29 @@ function SnapshotSelector(_: Props) {
- {`${commitIndex + 1}`.padStart(`${numCommits}`.length, '0')} /{' '} - {numCommits} + {currentCommitNumber} / {numCommits}
- + {numCommits > 0 && ( + + )} + {numCommits === 0 && ( +
No commits
+ )}