Skip to content

Commit

Permalink
Simplify some code related to thread CPU deltas (#5265)
Browse files Browse the repository at this point in the history
There was a mismatch in whether `computeMaxThreadCPUDeltaPerMs` was
called before or after thread CPU delta processing. It didn't make a
difference in practice, but it's better to just always call it with the
unprocessed values. This will avoid problems if we end up giving the
processed values a different type.
  • Loading branch information
mstange authored Dec 31, 2024
2 parents 6fa213a + df6d128 commit b86c7c5
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 76 deletions.
39 changes: 18 additions & 21 deletions src/profile-logic/cpu.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ import type {
} from 'firefox-profiler/types';

/**
* Compute the max CPU delta value per ms for that thread. It computes the
* max value after the threadCPUDelta processing.
* Compute the max CPU cycles per ms for the thread. Should only be called when
* the cpu delta unit is 'variable CPU cycles'.
* This computes the max value before the threadCPUDelta processing -
* the threadCPUDelta processing wouldn't do anything other than remove nulls
* anyway, because if the unit is 'variable CPU cycles' then we don't do any
* clamping.
*/
export function computeMaxThreadCPUDeltaPerMs(
threads: Thread[],
profileInterval: Milliseconds
): number {
function _computeMaxVariableCPUCyclesPerMs(threads: Thread[]): number {
let maxThreadCPUDeltaPerMs = 0;
for (let threadIndex = 0; threadIndex < threads.length; threadIndex++) {
const { samples } = threads[threadIndex];
Expand All @@ -33,11 +34,10 @@ export function computeMaxThreadCPUDeltaPerMs(
continue;
}

// First element of CPU delta is always null because back-end doesn't know
// the delta since there is no previous sample.
// Ignore the first CPU delta value; it's meaningless because there is no
// previous sample.
for (let i = 1; i < samples.length; i++) {
const sampleTimeDeltaInMs =
i === 0 ? profileInterval : samples.time[i] - samples.time[i - 1];
const sampleTimeDeltaInMs = samples.time[i] - samples.time[i - 1];
if (sampleTimeDeltaInMs !== 0) {
const cpuDeltaPerMs = (threadCPUDelta[i] || 0) / sampleTimeDeltaInMs;
maxThreadCPUDeltaPerMs = Math.max(
Expand Down Expand Up @@ -70,33 +70,30 @@ export function computeMaxThreadCPUDeltaPerMs(
* Returns 1234567 * 3, i.e. "3703701 cycles per sample if each sample ticks at
* the declared 3ms interval and the CPU usage is at the observed maximum".
*/
export function computeMaxCPUDeltaPerInterval(profile: Profile): number | null {
export function computeMaxCPUDeltaPerMs(profile: Profile): number {
const sampleUnits = profile.meta.sampleUnits;
if (!sampleUnits) {
return null;
return 1;
}

const interval = profile.meta.interval;
const threadCPUDeltaUnit = sampleUnits.threadCPUDelta;

switch (threadCPUDeltaUnit) {
case 'µs':
case 'ns': {
const cpuDeltaTimeUnitMultiplier =
getCpuDeltaTimeUnitMultiplier(threadCPUDeltaUnit);
return cpuDeltaTimeUnitMultiplier * interval;
const deltaUnitPerMs = getCpuDeltaTimeUnitMultiplier(threadCPUDeltaUnit);
return deltaUnitPerMs;
}
case 'variable CPU cycles': {
const maxThreadCPUDeltaPerMs = computeMaxThreadCPUDeltaPerMs(
profile.threads,
interval
const maxThreadCPUDeltaPerMs = _computeMaxVariableCPUCyclesPerMs(
profile.threads
);
return maxThreadCPUDeltaPerMs * interval;
return maxThreadCPUDeltaPerMs;
}
default:
throw assertExhaustiveCheck(
threadCPUDeltaUnit,
'Unhandled threadCPUDelta unit in computeMaxCPUDeltaPerInterval.'
'Unhandled threadCPUDelta unit in computeMaxCPUDeltaPerMs.'
);
}
}
Expand Down
13 changes: 7 additions & 6 deletions src/profile-logic/tracks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ const AUDIO_THREAD_SAMPLE_SCORE_BOOST_FACTOR = 40;
export function computeThreadActivityScore(
profile: Profile,
thread: Thread,
maxCpuDeltaPerInterval: number | null
maxCpuDeltaPerMs: number
): ThreadActivityScore {
const isEssentialFirefoxThread = _isEssentialFirefoxThread(thread);
const isInParentProcess = thread.processType === 'default';
Expand All @@ -1186,7 +1186,7 @@ export function computeThreadActivityScore(
const sampleScore = _computeThreadSampleScore(
profile,
thread,
maxCpuDeltaPerInterval
maxCpuDeltaPerMs
);
const boostedSampleScore = isInterestingEvenWithMinimalActivity
? sampleScore * AUDIO_THREAD_SAMPLE_SCORE_BOOST_FACTOR
Expand Down Expand Up @@ -1225,12 +1225,12 @@ function _isFirefoxMediaThreadWhichIsUsuallyIdle(thread: Thread): boolean {
// If the thread does not have CPU delta information, we compute a
// "CPU-delta-like" number based on the number of samples which are in a
// non-idle category.
// If the profile has no cpu delta units, the return value is the number of
// non-idle samples.
// If the profile has no cpu delta units, the return value is based on the
// number of non-idle samples.
function _computeThreadSampleScore(
{ meta }: Profile,
{ samples, stackTable }: Thread,
maxCpuDeltaPerInterval: number | null
maxCpuDeltaPerMs: number
): number {
if (meta.sampleUnits && samples.threadCPUDelta) {
// Sum up all CPU deltas in this thread, to compute a total
Expand All @@ -1251,7 +1251,8 @@ function _computeThreadSampleScore(
(stack) =>
stack !== null && stackTable.category[stack] !== idleCategoryIndex
).length;
return nonIdleSampleCount * (maxCpuDeltaPerInterval ?? 1);
const maxCpuDeltaPerInterval = maxCpuDeltaPerMs * meta.interval;
return nonIdleSampleCount * maxCpuDeltaPerInterval;
}

function _findDefaultThread(threads: Thread[]): Thread | null {
Expand Down
43 changes: 3 additions & 40 deletions src/selectors/cpu.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,14 @@

import { createSelector } from 'reselect';

import {
getThreads,
getProfileInterval,
getSampleUnits,
getMeta,
getCounter,
} from './profile';
import { getThreadSelectors } from './per-thread';
import { computeMaxThreadCPUDeltaPerMs } from 'firefox-profiler/profile-logic/cpu';
import { getThreads, getSampleUnits, getMeta, getCounter } from './profile';

import type { Selector, State, Thread } from 'firefox-profiler/types';
import type { Selector } from 'firefox-profiler/types';

export const getIsCPUUtilizationProvided: Selector<boolean> = createSelector(
getSampleUnits,
getMeta,
getCPUProcessedThreads,
getThreads,
(sampleUnits, meta, threads) => {
return (
sampleUnits !== undefined &&
Expand All @@ -44,32 +36,3 @@ export const getAreThereAnyProcessCPUCounters: Selector<boolean> =
counters !== null &&
counters.some((counter) => counter.category === 'CPU')
);

/**
* This function returns the list of all threads after the CPU values have been
* processed. This uses a selector from the per-thread selectors. Because we'll
* use this selector for every thread, and also need the full state for this call,
* we can't use the simple memoization from `createSelector`, and instead we
* need to implement our own simple memoization.
*/
let _threads = null;
let _cpuProcessedThreads = null;
function getCPUProcessedThreads(state: State): Thread[] {
const threads = getThreads(state);

if (_threads !== threads || _cpuProcessedThreads === null) {
// Storing the threads makes it possible to invalidate the memoized value at
// the right moment.
_threads = threads;
_cpuProcessedThreads = threads.map((thread, threadIndex) =>
getThreadSelectors(threadIndex).getCPUProcessedThread(state)
);
}
return _cpuProcessedThreads;
}

export const getMaxThreadCPUDeltaPerMs: Selector<number> = createSelector(
getCPUProcessedThreads,
getProfileInterval,
computeMaxThreadCPUDeltaPerMs
);
16 changes: 7 additions & 9 deletions src/selectors/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,22 +718,20 @@ export const getHiddenTrackCount: Selector<HiddenTrackCount> = createSelector(
}
);

export const getMaxCPUDeltaPerInterval: Selector<number | null> =
createSelector(getProfile, CPU.computeMaxCPUDeltaPerInterval);
export const getMaxThreadCPUDeltaPerMs: Selector<number> = createSelector(
getProfile,
CPU.computeMaxCPUDeltaPerMs
);

export const getThreadActivityScores: Selector<Array<ThreadActivityScore>> =
createSelector(
getProfile,
getMaxCPUDeltaPerInterval,
(profile, maxCpuDeltaPerInterval) => {
getMaxThreadCPUDeltaPerMs,
(profile, maxCpuDeltaPerMs) => {
const { threads } = profile;

return threads.map((thread) =>
Tracks.computeThreadActivityScore(
profile,
thread,
maxCpuDeltaPerInterval
)
Tracks.computeThreadActivityScore(profile, thread, maxCpuDeltaPerMs)
);
}
);
Expand Down

0 comments on commit b86c7c5

Please sign in to comment.