From cba64cead7b608bff2e1b24f76042192edc842b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 28 Jun 2022 18:13:02 +0200 Subject: [PATCH 1/6] Move the processing of the first sample index to getCounter --- src/profile-logic/profile-data.js | 49 +++++++++++++++++++++-------- src/selectors/profile.js | 11 ++++--- src/test/store/profile-view.test.js | 21 ++++++++++--- 3 files changed, 60 insertions(+), 21 deletions(-) diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index bc072b28c9..9c382a731b 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -1393,6 +1393,42 @@ export function filterThreadSamplesToRange( return newThread; } +/** + * Process the samples in the counter sample groups. + */ +export function processCounter(counter: Counter): Counter { + const processedGroups = counter.sampleGroups.map((sampleGroup) => { + const { samples } = sampleGroup; + const count = samples.count.slice(); + const number = samples.number.slice(); + + // These lines zero out the first values of the counters, as they are unreliable. In + // addition, there are probably some missed counts in the memory counters, so the + // first memory number slowly creeps up over time, and becomes very unrealistic. + // In order to not be affected by these platform limitations, zero out the first + // counter values. + // + // "Memory counter in Gecko Profiler isn't cleared when starting a new capture" + // https://bugzilla.mozilla.org/show_bug.cgi?id=1520587 + count[0] = 0; + number[0] = 0; + + return { + ...sampleGroup, + samples: { + ...samples, + number, + count, + }, + }; + }); + + return { + ...counter, + sampleGroups: processedGroups, + }; +} + export function filterCounterToRange( counter: Counter, rangeStart: number, @@ -1418,19 +1454,6 @@ export function filterCounterToRange( const count = samples.count.slice(sBegin, sEnd); const number = samples.number.slice(sBegin, sEnd); - if (sBegin === 0) { - // These lines zero out the first values of the counters, as they are unreliable. In - // addition, there are probably some missed counts in the memory counters, so the - // first memory number slowly creeps up over time, and becomes very unrealistic. - // In order to not be affected by these platform limitations, zero out the first - // counter values. - // - // "Memory counter in Gecko Profiler isn't cleared when starting a new capture" - // https://bugzilla.mozilla.org/show_bug.cgi?id=1520587 - count[0] = 0; - number[0] = 0; - } - return { ...sampleGroup, samples: { diff --git a/src/selectors/profile.js b/src/selectors/profile.js index b79335c058..b0744fa7be 100644 --- a/src/selectors/profile.js +++ b/src/selectors/profile.js @@ -13,6 +13,7 @@ import { extractProfileFilterPageData, computeMaxCounterSampleCountsPerMs, getFriendlyThreadName, + processCounter, } from '../profile-logic/profile-data'; import { IPCMarkerCorrelations, @@ -264,10 +265,12 @@ export const getCounterSelectors = (index: CounterIndex): CounterSelectors => { */ function _createCounterSelectors(counterIndex: CounterIndex) { const getCounter: Selector = (state) => - ensureExists( - getProfile(state).counters, - 'Attempting to get a counter by index, but no counters exist.' - )[counterIndex]; + processCounter( + ensureExists( + getProfile(state).counters, + 'Attempting to get a counter by index, but no counters exist.' + )[counterIndex] + ); const getDescription: Selector = (state) => getCounter(state).description; diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index a66f4e8845..b4d34741f0 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -48,6 +48,7 @@ import { import { ensureExists } from '../../utils/flow'; import { getCallNodeIndexFromPath, + processCounter, type BreakdownByCategory, } from '../../profile-logic/profile-data'; @@ -3117,13 +3118,25 @@ describe('counter selectors', function () { const counterB = getCounterForThread(thread, threadIndex); profile.counters = [counterA, counterB]; const { getState, dispatch } = storeWithProfile(profile); - return { getState, dispatch, counterA, counterB }; + const processedCounterA = processCounter(counterA); + const processedCounterB = processCounter(counterB); + return { + getState, + dispatch, + counterA, + processedCounterA, + processedCounterB, + }; } it('can get the counters', function () { - const { counterA, counterB, getState } = setup(); - expect(getCounterSelectors(0).getCounter(getState())).toBe(counterA); - expect(getCounterSelectors(1).getCounter(getState())).toBe(counterB); + const { processedCounterA, processedCounterB, getState } = setup(); + expect(getCounterSelectors(0).getCounter(getState())).toStrictEqual( + processedCounterA + ); + expect(getCounterSelectors(1).getCounter(getState())).toStrictEqual( + processedCounterB + ); }); it('can get the counter description', function () { From 178a4e6be576bd7ec5bba59211598c7418ceb7f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 28 Jun 2022 18:16:17 +0200 Subject: [PATCH 2/6] Use the full thread on the power graph --- src/components/timeline/TrackPowerGraph.js | 34 ++++++++++-- src/profile-logic/profile-data.js | 64 ++++++++++++++++------ src/selectors/profile.js | 28 +++++++--- 3 files changed, 95 insertions(+), 31 deletions(-) diff --git a/src/components/timeline/TrackPowerGraph.js b/src/components/timeline/TrackPowerGraph.js index a6acb8d673..8d740143fe 100644 --- a/src/components/timeline/TrackPowerGraph.js +++ b/src/components/timeline/TrackPowerGraph.js @@ -28,6 +28,7 @@ import type { Milliseconds, CssPixels, StartEndRange, + IndexIntoSamplesTable, } from 'firefox-profiler/types'; import type { SizeProps } from 'firefox-profiler/components/shared/WithSize'; @@ -42,6 +43,7 @@ type CanvasProps = {| +rangeStart: Milliseconds, +rangeEnd: Milliseconds, +counter: Counter, + +counterSampleRanges: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]>, +maxCounterSampleCountsPerMs: number[], +interval: Milliseconds, +width: CssPixels, @@ -68,6 +70,7 @@ class TrackPowerCanvas extends React.PureComponent { lineWidth, interval, maxCounterSampleCountsPerMs, + counterSampleRanges, } = this.props; if (width === 0) { // This is attempting to draw before the canvas was laid out. @@ -91,7 +94,7 @@ class TrackPowerCanvas extends React.PureComponent { ctx.clearRect(0, 0, deviceWidth, deviceHeight); const sampleGroups = counter.sampleGroups; - if (sampleGroups.length === 0) { + if (sampleGroups.length === 0 || counterSampleRanges.length === 0) { // Gecko failed to capture samples for some reason and it shouldn't happen for // malloc counter. Print an error and do not draw anything. throw new Error('No sample group found for power counter'); @@ -103,6 +106,7 @@ class TrackPowerCanvas extends React.PureComponent { return; } + const [sampleStart, sampleEnd] = counterSampleRanges[0]; const countRangePerMs = maxCounterSampleCountsPerMs[0]; { @@ -125,7 +129,7 @@ class TrackPowerCanvas extends React.PureComponent { let x = 0; let y = 0; let firstX = 0; - for (let i = 0; i < samples.length; i++) { + for (let i = sampleStart; i < sampleEnd; i++) { // Create a path for the top of the chart. This is the line that will have // a stroke applied to it. x = (samples.time[i] - rangeStart) * millisecondWidth; @@ -208,6 +212,7 @@ type StateProps = {| +rangeStart: Milliseconds, +rangeEnd: Milliseconds, +counter: Counter, + +counterSampleRanges: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]>, +maxCounterSampleCountsPerMs: number[], +interval: Milliseconds, +filteredThread: Thread, @@ -246,7 +251,14 @@ class TrackPowerGraphImpl extends React.PureComponent { const { pageX: mouseX, pageY: mouseY } = event; // Get the offset from here, and apply it to the time lookup. const { left } = event.currentTarget.getBoundingClientRect(); - const { width, rangeStart, rangeEnd, counter, interval } = this.props; + const { + width, + rangeStart, + rangeEnd, + counter, + interval, + counterSampleRanges, + } = this.props; const rangeLength = rangeEnd - rangeStart; const timeAtMouse = rangeStart + ((mouseX - left) / width) * rangeLength; @@ -266,7 +278,13 @@ class TrackPowerGraphImpl extends React.PureComponent { } else { // When the mouse pointer hovers between two points, select the point that's closer. let hoveredCounter; - const bisectionCounter = bisectionRight(samples.time, timeAtMouse); + const [sampleStart, sampleEnd] = counterSampleRanges[0]; + const bisectionCounter = bisectionRight( + samples.time, + timeAtMouse, + sampleStart, + sampleEnd + ); if (bisectionCounter > 0 && bisectionCounter < samples.time.length) { const leftDistance = timeAtMouse - samples.time[bisectionCounter - 1]; const rightDistance = samples.time[bisectionCounter] - timeAtMouse; @@ -418,6 +436,7 @@ class TrackPowerGraphImpl extends React.PureComponent { rangeEnd, unfilteredSamplesRange, counter, + counterSampleRanges, graphHeight, width, lineWidth, @@ -434,6 +453,7 @@ class TrackPowerGraphImpl extends React.PureComponent { rangeStart={rangeStart} rangeEnd={rangeEnd} counter={counter} + counterSampleRanges={counterSampleRanges} height={graphHeight} width={width} lineWidth={lineWidth} @@ -466,8 +486,11 @@ export const TrackPowerGraph = explicitConnect< mapStateToProps: (state, ownProps) => { const { counterIndex } = ownProps; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter(state); + const counter = counterSelectors.getCounter(state); const { start, end } = getCommittedRange(state); + const counterSampleRanges = + counterSelectors.getCommittedRangeCounterSampleRanges(state); + const selectors = getThreadSelectors(counter.mainThreadIndex); return { counter, @@ -476,6 +499,7 @@ export const TrackPowerGraph = explicitConnect< counterSelectors.getMaxRangeCounterSampleCountsPerMs(state), rangeStart: start, rangeEnd: end, + counterSampleRanges, interval: getProfileInterval(state), filteredThread: selectors.getFilteredThread(state), unfilteredSamplesRange: selectors.unfilteredSamplesRange(state), diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 9c382a731b..c8c9a53b2f 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -1276,6 +1276,34 @@ export function getSampleIndexRangeForSelection( return [sampleStart, sampleEnd]; } +/** + * This function takes a samples table and returns the sample range + * including the sample just before and after the range. This is needed to make + * sure that some charts will not be cut off at the edges when zoomed in to a range. + */ +export function getInclusiveSampleIndexRangeForSelection( + table: { time: Milliseconds[], length: number }, + rangeStart: number, + rangeEnd: number +): [IndexIntoSamplesTable, IndexIntoSamplesTable] { + let [sampleStart, sampleEnd] = getSampleIndexRangeForSelection( + table, + rangeStart, + rangeEnd + ); + + // Include the samples just before and after the selection range, so that charts will + // not be cut off at the edges. + if (sampleStart > 0) { + sampleStart--; + } + if (sampleEnd < table.length) { + sampleEnd++; + } + + return [sampleStart, sampleEnd]; +} + export function filterThreadSamplesToRange( thread: Thread, rangeStart: number, @@ -1436,30 +1464,18 @@ export function filterCounterToRange( ): Counter { const filteredGroups = counter.sampleGroups.map((sampleGroup) => { const samples = sampleGroup.samples; - let [sBegin, sEnd] = getSampleIndexRangeForSelection( + const [sBegin, sEnd] = getInclusiveSampleIndexRangeForSelection( samples, rangeStart, rangeEnd ); - // Include the samples just before and after the selection range, so that charts will - // not be cut off at the edges. - if (sBegin > 0) { - sBegin--; - } - if (sEnd < samples.length) { - sEnd++; - } - - const count = samples.count.slice(sBegin, sEnd); - const number = samples.number.slice(sBegin, sEnd); - return { ...sampleGroup, samples: { time: samples.time.slice(sBegin, sEnd), - number, - count, + number: samples.number.slice(sBegin, sEnd), + count: samples.count.slice(sBegin, sEnd), length: sEnd - sBegin, }, }; @@ -1507,14 +1523,26 @@ export function accumulateCounterSamples( /** * Compute the max counter sample counts per milliseconds to determine the range * of a counter. + * If a start-end range is provided, it only computes the max value between that + * range. */ export function computeMaxCounterSampleCountsPerMs( samplesArray: Array, - profileInterval: Milliseconds + profileInterval: Milliseconds, + sampleRanges?: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]> ): Array { - const maxSampleCounts = samplesArray.map((samples) => { + const maxSampleCounts = samplesArray.map((samples, index) => { let maxCount = 0; - for (let i = 0; i < samples.length; i++) { + // If a range is provided, use it instead. This will also include the + // samples right before and after the range. + const startSampleIndex = + sampleRanges && sampleRanges[index] ? sampleRanges[index][0] : 0; + const endSampleIndex = + sampleRanges && sampleRanges[index] + ? sampleRanges[index][1] + : samples.length; + + for (let i = startSampleIndex; i < endSampleIndex; i++) { const count = samples.count[i]; const sampleTimeDeltaInMs = i === 0 ? profileInterval : samples.time[i] - samples.time[i - 1]; diff --git a/src/selectors/profile.js b/src/selectors/profile.js index b0744fa7be..4cd70039a5 100644 --- a/src/selectors/profile.js +++ b/src/selectors/profile.js @@ -14,6 +14,7 @@ import { computeMaxCounterSampleCountsPerMs, getFriendlyThreadName, processCounter, + getInclusiveSampleIndexRangeForSelection, } from '../profile-logic/profile-data'; import { IPCMarkerCorrelations, @@ -72,6 +73,7 @@ import type { MarkerSchema, MarkerSchemaByName, SampleUnits, + IndexIntoSamplesTable, } from 'firefox-profiler/types'; export const getProfileView: Selector = (state) => @@ -302,19 +304,28 @@ function _createCounterSelectors(counterIndex: CounterIndex) { ) ); + const getCommittedRangeCounterSampleRanges: Selector< + Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]> + > = createSelector(getCounter, getCommittedRange, (counter, range) => + counter.sampleGroups.map((group) => + getInclusiveSampleIndexRangeForSelection( + group.samples, + range.start, + range.end + ) + ) + ); + const getMaxRangeCounterSampleCountsPerMs: Selector> = createSelector( getCounter, getProfileInterval, - getCommittedRange, - (counters, profileInterval, range) => + getCommittedRangeCounterSampleRanges, + (counters, profileInterval, sampleRange) => computeMaxCounterSampleCountsPerMs( - filterCounterToRange( - counters, - range.start, - range.end - ).sampleGroups.map((group) => group.samples), - profileInterval + counters.sampleGroups.map((group) => group.samples), + profileInterval, + sampleRange ) ); @@ -326,6 +337,7 @@ function _createCounterSelectors(counterIndex: CounterIndex) { getAccumulateCounterSamples, getMaxCounterSampleCountsPerMs, getMaxRangeCounterSampleCountsPerMs, + getCommittedRangeCounterSampleRanges, }; } From 3ee28dd27ccb73592290b991ced02d25e8489c8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 7 Jul 2022 12:01:19 +0200 Subject: [PATCH 3/6] Use the full thread on the memory graph --- src/components/timeline/TrackMemoryGraph.js | 33 ++++++++++++++++--- src/profile-logic/profile-data.js | 18 ++++++++--- src/selectors/profile.js | 36 ++++++++++++--------- 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/components/timeline/TrackMemoryGraph.js b/src/components/timeline/TrackMemoryGraph.js index 05f2753bf6..2520deb43f 100644 --- a/src/components/timeline/TrackMemoryGraph.js +++ b/src/components/timeline/TrackMemoryGraph.js @@ -33,6 +33,7 @@ import type { Milliseconds, CssPixels, StartEndRange, + IndexIntoSamplesTable, } from 'firefox-profiler/types'; import type { SizeProps } from 'firefox-profiler/components/shared/WithSize'; @@ -47,6 +48,7 @@ type CanvasProps = {| +rangeStart: Milliseconds, +rangeEnd: Milliseconds, +counter: Counter, + +counterSampleRanges: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]>, +accumulatedSamples: AccumulatedCounterSamples[], +interval: Milliseconds, +width: CssPixels, @@ -77,6 +79,7 @@ class TrackMemoryCanvas extends React.PureComponent { lineWidth, interval, accumulatedSamples, + counterSampleRanges, } = this.props; if (width === 0) { // This is attempting to draw before the canvas was laid out. @@ -100,7 +103,7 @@ class TrackMemoryCanvas extends React.PureComponent { ctx.clearRect(0, 0, deviceWidth, deviceHeight); const sampleGroups = counter.sampleGroups; - if (sampleGroups.length === 0) { + if (sampleGroups.length === 0 || counterSampleRanges.length === 0) { // Gecko failed to capture samples for some reason and it shouldn't happen for // malloc counter. Print an error and do not draw anything. throw new Error('No sample group found for memory counter'); @@ -121,6 +124,7 @@ class TrackMemoryCanvas extends React.PureComponent { throw new Error('No accumulated sample found for memory counter'); } const { minCount, countRange, accumulatedCounts } = accumulatedSamples[0]; + const [sampleStart, sampleEnd] = counterSampleRanges[0]; { // Draw the chart. @@ -142,7 +146,7 @@ class TrackMemoryCanvas extends React.PureComponent { let x = 0; let y = 0; let firstX = 0; - for (let i = 0; i < samples.length; i++) { + for (let i = sampleStart; i < sampleEnd; i++) { // Create a path for the top of the chart. This is the line that will have // a stroke applied to it. x = (samples.time[i] - rangeStart) * millisecondWidth; @@ -253,6 +257,7 @@ type StateProps = {| +rangeStart: Milliseconds, +rangeEnd: Milliseconds, +counter: Counter, + +counterSampleRanges: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]>, +accumulatedSamples: AccumulatedCounterSamples[], +interval: Milliseconds, +filteredThread: Thread, @@ -297,7 +302,14 @@ class TrackMemoryGraphImpl extends React.PureComponent { const { pageX: mouseX, pageY: mouseY } = event; // Get the offset from here, and apply it to the time lookup. const { left } = event.currentTarget.getBoundingClientRect(); - const { width, rangeStart, rangeEnd, counter, interval } = this.props; + const { + width, + rangeStart, + rangeEnd, + counter, + interval, + counterSampleRanges, + } = this.props; const rangeLength = rangeEnd - rangeStart; const timeAtMouse = rangeStart + ((mouseX - left) / width) * rangeLength; @@ -317,7 +329,13 @@ class TrackMemoryGraphImpl extends React.PureComponent { } else { // When the mouse pointer hovers between two points, select the point that's closer. let hoveredCounter; - const bisectionCounter = bisectionRight(samples.time, timeAtMouse); + const [sampleStart, sampleEnd] = counterSampleRanges[0]; + const bisectionCounter = bisectionRight( + samples.time, + timeAtMouse, + sampleStart, + sampleEnd + ); if (bisectionCounter > 0 && bisectionCounter < samples.time.length) { const leftDistance = timeAtMouse - samples.time[bisectionCounter - 1]; const rightDistance = samples.time[bisectionCounter] - timeAtMouse; @@ -464,6 +482,7 @@ class TrackMemoryGraphImpl extends React.PureComponent { rangeEnd, unfilteredSamplesRange, counter, + counterSampleRanges, graphHeight, width, lineWidth, @@ -480,6 +499,7 @@ class TrackMemoryGraphImpl extends React.PureComponent { rangeStart={rangeStart} rangeEnd={rangeEnd} counter={counter} + counterSampleRanges={counterSampleRanges} height={graphHeight} width={width} lineWidth={lineWidth} @@ -512,8 +532,10 @@ export const TrackMemoryGraph = explicitConnect< mapStateToProps: (state, ownProps) => { const { counterIndex } = ownProps; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter(state); + const counter = counterSelectors.getCounter(state); const { start, end } = getCommittedRange(state); + const counterSampleRanges = + counterSelectors.getCommittedRangeCounterSampleRanges(state); const selectors = getThreadSelectors(counter.mainThreadIndex); return { counter, @@ -521,6 +543,7 @@ export const TrackMemoryGraph = explicitConnect< accumulatedSamples: counterSelectors.getAccumulateCounterSamples(state), rangeStart: start, rangeEnd: end, + counterSampleRanges, interval: getProfileInterval(state), filteredThread: selectors.getFilteredThread(state), unfilteredSamplesRange: selectors.unfilteredSamplesRange(state), diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index c8c9a53b2f..6449de91dc 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -1494,14 +1494,24 @@ export function filterCounterToRange( * accumulatedCounts array. */ export function accumulateCounterSamples( - samplesArray: Array + samplesArray: Array, + sampleRanges?: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]> ): Array { - const accumulatedSamples = samplesArray.map((samples) => { + const accumulatedSamples = samplesArray.map((samples, index) => { let minCount = 0; let maxCount = 0; let accumulated = 0; - const accumulatedCounts = []; - for (let i = 0; i < samples.length; i++) { + const accumulatedCounts = Array(samples.length).fill(0); + // If a range is provided, use it instead. This will also include the + // samples right before and after the range. + const startSampleIndex = + sampleRanges && sampleRanges[index] ? sampleRanges[index][0] : 0; + const endSampleIndex = + sampleRanges && sampleRanges[index] + ? sampleRanges[index][1] + : samples.length; + + for (let i = startSampleIndex; i < endSampleIndex; i++) { accumulated += samples.count[i]; minCount = Math.min(accumulated, minCount); maxCount = Math.max(accumulated, maxCount); diff --git a/src/selectors/profile.js b/src/selectors/profile.js index 4cd70039a5..c1c677f5bb 100644 --- a/src/selectors/profile.js +++ b/src/selectors/profile.js @@ -285,12 +285,28 @@ function _createCounterSelectors(counterIndex: CounterIndex) { (counters, range) => filterCounterToRange(counters, range.start, range.end) ); + const getCommittedRangeCounterSampleRanges: Selector< + Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]> + > = createSelector(getCounter, getCommittedRange, (counter, range) => + counter.sampleGroups.map((group) => + getInclusiveSampleIndexRangeForSelection( + group.samples, + range.start, + range.end + ) + ) + ); + const getAccumulateCounterSamples: Selector< Array - > = createSelector(getCommittedRangeFilteredCounter, (counters) => - accumulateCounterSamples( - counters.sampleGroups.map((group) => group.samples) - ) + > = createSelector( + getCounter, + getCommittedRangeCounterSampleRanges, + (counters, sampleRanges) => + accumulateCounterSamples( + counters.sampleGroups.map((group) => group.samples), + sampleRanges + ) ); const getMaxCounterSampleCountsPerMs: Selector> = @@ -304,18 +320,6 @@ function _createCounterSelectors(counterIndex: CounterIndex) { ) ); - const getCommittedRangeCounterSampleRanges: Selector< - Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]> - > = createSelector(getCounter, getCommittedRange, (counter, range) => - counter.sampleGroups.map((group) => - getInclusiveSampleIndexRangeForSelection( - group.samples, - range.start, - range.end - ) - ) - ); - const getMaxRangeCounterSampleCountsPerMs: Selector> = createSelector( getCounter, From f48ac5cfffab02c0681b4eb2569644dde62867ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 7 Jul 2022 13:05:03 +0200 Subject: [PATCH 4/6] Fix the typo in the component name --- src/components/timeline/TrackProcessCPU.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/timeline/TrackProcessCPU.js b/src/components/timeline/TrackProcessCPU.js index 68ea4afce4..97316370f1 100644 --- a/src/components/timeline/TrackProcessCPU.js +++ b/src/components/timeline/TrackProcessCPU.js @@ -45,7 +45,7 @@ type Props = ConnectedProps; type State = {||}; -export class TracProcessCPUImpl extends React.PureComponent { +export class TrackProcessCPUImpl extends React.PureComponent { render() { const { counterIndex } = this.props; return ( @@ -83,5 +83,5 @@ export const TrackProcessCPU = explicitConnect< }; }, mapDispatchToProps: { updatePreviewSelection }, - component: TracProcessCPUImpl, + component: TrackProcessCPUImpl, }); From 3ba7aeceab02813adafb85279e74001cc2757507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 7 Jul 2022 13:10:44 +0200 Subject: [PATCH 5/6] Use the full thread on the process cpu graph --- .../timeline/TrackProcessCPUGraph.js | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/components/timeline/TrackProcessCPUGraph.js b/src/components/timeline/TrackProcessCPUGraph.js index 121a7d2233..9723f1a1fb 100644 --- a/src/components/timeline/TrackProcessCPUGraph.js +++ b/src/components/timeline/TrackProcessCPUGraph.js @@ -27,6 +27,7 @@ import type { Milliseconds, CssPixels, StartEndRange, + IndexIntoSamplesTable, } from 'firefox-profiler/types'; import type { SizeProps } from 'firefox-profiler/components/shared/WithSize'; @@ -41,6 +42,7 @@ type CanvasProps = {| +rangeStart: Milliseconds, +rangeEnd: Milliseconds, +counter: Counter, + +counterSampleRanges: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]>, +maxCounterSampleCountsPerMs: number[], +interval: Milliseconds, +width: CssPixels, @@ -67,6 +69,7 @@ class TrackProcessCPUCanvas extends React.PureComponent { lineWidth, interval, maxCounterSampleCountsPerMs, + counterSampleRanges, } = this.props; if (width === 0) { // This is attempting to draw before the canvas was laid out. @@ -90,7 +93,7 @@ class TrackProcessCPUCanvas extends React.PureComponent { ctx.clearRect(0, 0, deviceWidth, deviceHeight); const sampleGroups = counter.sampleGroups; - if (sampleGroups.length === 0) { + if (sampleGroups.length === 0 || counterSampleRanges.length === 0) { // Gecko failed to capture samples for some reason and it shouldn't happen for // malloc counter. Print an error and do not draw anything. throw new Error('No sample group found for process CPU counter'); @@ -102,6 +105,7 @@ class TrackProcessCPUCanvas extends React.PureComponent { return; } + const [sampleStart, sampleEnd] = counterSampleRanges[0]; const countRangePerMs = maxCounterSampleCountsPerMs[0]; { @@ -124,7 +128,7 @@ class TrackProcessCPUCanvas extends React.PureComponent { let x = 0; let y = 0; let firstX = 0; - for (let i = 0; i < samples.length; i++) { + for (let i = sampleStart; i < sampleEnd; i++) { // Create a path for the top of the chart. This is the line that will have // a stroke applied to it. x = (samples.time[i] - rangeStart) * millisecondWidth; @@ -210,6 +214,7 @@ type StateProps = {| +rangeStart: Milliseconds, +rangeEnd: Milliseconds, +counter: Counter, + +counterSampleRanges: Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]>, +maxCounterSampleCountsPerMs: number[], +interval: Milliseconds, +filteredThread: Thread, @@ -248,7 +253,14 @@ class TrackProcessCPUGraphImpl extends React.PureComponent { const { pageX: mouseX, pageY: mouseY } = event; // Get the offset from here, and apply it to the time lookup. const { left } = event.currentTarget.getBoundingClientRect(); - const { width, rangeStart, rangeEnd, counter, interval } = this.props; + const { + width, + rangeStart, + rangeEnd, + counter, + interval, + counterSampleRanges, + } = this.props; const rangeLength = rangeEnd - rangeStart; const timeAtMouse = rangeStart + ((mouseX - left) / width) * rangeLength; @@ -268,7 +280,13 @@ class TrackProcessCPUGraphImpl extends React.PureComponent { } else { // When the mouse pointer hovers between two points, select the point that's closer. let hoveredCounter; - const bisectionCounter = bisectionRight(samples.time, timeAtMouse); + const [sampleStart, sampleEnd] = counterSampleRanges[0]; + const bisectionCounter = bisectionRight( + samples.time, + timeAtMouse, + sampleStart, + sampleEnd + ); if (bisectionCounter > 0 && bisectionCounter < samples.time.length) { const leftDistance = timeAtMouse - samples.time[bisectionCounter - 1]; const rightDistance = samples.time[bisectionCounter] - timeAtMouse; @@ -409,6 +427,7 @@ class TrackProcessCPUGraphImpl extends React.PureComponent { rangeEnd, unfilteredSamplesRange, counter, + counterSampleRanges, graphHeight, width, lineWidth, @@ -425,6 +444,7 @@ class TrackProcessCPUGraphImpl extends React.PureComponent { rangeStart={rangeStart} rangeEnd={rangeEnd} counter={counter} + counterSampleRanges={counterSampleRanges} height={graphHeight} width={width} lineWidth={lineWidth} @@ -457,8 +477,10 @@ export const TrackProcessCPUGraph = explicitConnect< mapStateToProps: (state, ownProps) => { const { counterIndex } = ownProps; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter(state); + const counter = counterSelectors.getCounter(state); const { start, end } = getCommittedRange(state); + const counterSampleRanges = + counterSelectors.getCommittedRangeCounterSampleRanges(state); const selectors = getThreadSelectors(counter.mainThreadIndex); return { counter, @@ -467,6 +489,7 @@ export const TrackProcessCPUGraph = explicitConnect< counterSelectors.getMaxCounterSampleCountsPerMs(state), rangeStart: start, rangeEnd: end, + counterSampleRanges, interval: getProfileInterval(state), filteredThread: selectors.getFilteredThread(state), unfilteredSamplesRange: selectors.unfilteredSamplesRange(state), From ac6a6be49553c0b6ed1f2da7cf55bdb75116868b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Thu, 7 Jul 2022 13:21:47 +0200 Subject: [PATCH 6/6] Remove the unnecessary getCommittedRangeFilteredCounter selector --- src/actions/profile-view.js | 8 ++---- src/components/timeline/TrackMemory.js | 2 +- src/components/timeline/TrackPower.js | 2 +- src/components/timeline/TrackProcessCPU.js | 2 +- src/profile-logic/profile-data.js | 30 ---------------------- src/selectors/profile.js | 8 ------ src/test/store/profile-view.test.js | 16 ------------ 7 files changed, 5 insertions(+), 63 deletions(-) diff --git a/src/actions/profile-view.js b/src/actions/profile-view.js index 7eb2b6de08..db78967c31 100644 --- a/src/actions/profile-view.js +++ b/src/actions/profile-view.js @@ -349,9 +349,7 @@ export function selectTrack( case 'memory': { const { counterIndex } = localTrack; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter( - getState() - ); + const counter = counterSelectors.getCounter(getState()); selectedThreadIndex = counter.mainThreadIndex; break; } @@ -363,9 +361,7 @@ export function selectTrack( case 'power': { const { counterIndex } = localTrack; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter( - getState() - ); + const counter = counterSelectors.getCounter(getState()); selectedThreadIndex = counter.mainThreadIndex; break; } diff --git a/src/components/timeline/TrackMemory.js b/src/components/timeline/TrackMemory.js index db05561a0a..a31187716f 100644 --- a/src/components/timeline/TrackMemory.js +++ b/src/components/timeline/TrackMemory.js @@ -90,7 +90,7 @@ export const TrackMemory = explicitConnect( mapStateToProps: (state, ownProps) => { const { counterIndex } = ownProps; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter(state); + const counter = counterSelectors.getCounter(state); const { start, end } = getCommittedRange(state); return { threadIndex: counter.mainThreadIndex, diff --git a/src/components/timeline/TrackPower.js b/src/components/timeline/TrackPower.js index 1cc1ce7109..454b2daad6 100644 --- a/src/components/timeline/TrackPower.js +++ b/src/components/timeline/TrackPower.js @@ -70,7 +70,7 @@ export const TrackPower = explicitConnect({ mapStateToProps: (state, ownProps) => { const { counterIndex } = ownProps; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter(state); + const counter = counterSelectors.getCounter(state); const { start, end } = getCommittedRange(state); return { threadIndex: counter.mainThreadIndex, diff --git a/src/components/timeline/TrackProcessCPU.js b/src/components/timeline/TrackProcessCPU.js index 97316370f1..40923d5286 100644 --- a/src/components/timeline/TrackProcessCPU.js +++ b/src/components/timeline/TrackProcessCPU.js @@ -74,7 +74,7 @@ export const TrackProcessCPU = explicitConnect< mapStateToProps: (state, ownProps) => { const { counterIndex } = ownProps; const counterSelectors = getCounterSelectors(counterIndex); - const counter = counterSelectors.getCommittedRangeFilteredCounter(state); + const counter = counterSelectors.getCounter(state); const { start, end } = getCommittedRange(state); return { threadIndex: counter.mainThreadIndex, diff --git a/src/profile-logic/profile-data.js b/src/profile-logic/profile-data.js index 6449de91dc..e26d969802 100644 --- a/src/profile-logic/profile-data.js +++ b/src/profile-logic/profile-data.js @@ -1457,36 +1457,6 @@ export function processCounter(counter: Counter): Counter { }; } -export function filterCounterToRange( - counter: Counter, - rangeStart: number, - rangeEnd: number -): Counter { - const filteredGroups = counter.sampleGroups.map((sampleGroup) => { - const samples = sampleGroup.samples; - const [sBegin, sEnd] = getInclusiveSampleIndexRangeForSelection( - samples, - rangeStart, - rangeEnd - ); - - return { - ...sampleGroup, - samples: { - time: samples.time.slice(sBegin, sEnd), - number: samples.number.slice(sBegin, sEnd), - count: samples.count.slice(sBegin, sEnd), - length: sEnd - sBegin, - }, - }; - }); - - return { - ...counter, - sampleGroups: filteredGroups, - }; -} - /** * The memory counter contains relative offsets of memory. In order to draw an interesting * graph, take the memory counts, and find the minimum and maximum values, by diff --git a/src/selectors/profile.js b/src/selectors/profile.js index c1c677f5bb..64beb8093e 100644 --- a/src/selectors/profile.js +++ b/src/selectors/profile.js @@ -8,7 +8,6 @@ import * as Tracks from '../profile-logic/tracks'; import * as UrlState from './url-state'; import { ensureExists, assertExhaustiveCheck } from '../utils/flow'; import { - filterCounterToRange, accumulateCounterSamples, extractProfileFilterPageData, computeMaxCounterSampleCountsPerMs, @@ -279,12 +278,6 @@ function _createCounterSelectors(counterIndex: CounterIndex) { const getPid: Selector = (state) => getCounter(state).pid; - const getCommittedRangeFilteredCounter: Selector = createSelector( - getCounter, - getCommittedRange, - (counters, range) => filterCounterToRange(counters, range.start, range.end) - ); - const getCommittedRangeCounterSampleRanges: Selector< Array<[IndexIntoSamplesTable, IndexIntoSamplesTable]> > = createSelector(getCounter, getCommittedRange, (counter, range) => @@ -337,7 +330,6 @@ function _createCounterSelectors(counterIndex: CounterIndex) { getCounter, getDescription, getPid, - getCommittedRangeFilteredCounter, getAccumulateCounterSamples, getMaxCounterSampleCountsPerMs, getMaxRangeCounterSampleCountsPerMs, diff --git a/src/test/store/profile-view.test.js b/src/test/store/profile-view.test.js index b4d34741f0..e7568f4f46 100644 --- a/src/test/store/profile-view.test.js +++ b/src/test/store/profile-view.test.js @@ -3151,22 +3151,6 @@ describe('counter selectors', function () { expect(getCounterSelectors(0).getPid(getState())).toBe(0); }); - it('can get the commited range filtered counters', function () { - const { getState, dispatch } = setup(); - // The range includes the sample just before and the sample just after the selection - // range. - dispatch(ProfileView.commitRange(3.5, 5.5)); - const originalCounter = getCounterSelectors(0).getCounter(getState()); - expect(originalCounter.sampleGroups[0].samples.time).toEqual([ - 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, - ]); - - const filteredCounter = getCounterSelectors( - 0 - ).getCommittedRangeFilteredCounter(getState()); - expect(filteredCounter.sampleGroups[0].samples.time).toEqual([3, 4, 5, 6]); - }); - it('can accumulate samples', function () { const { getState, counterA } = setup(); counterA.sampleGroups[0].samples.count = [