Skip to content

Commit c9baf74

Browse files
committed
Clean up profiler-cli counter internals
- Group counters under their process by counter.pid, as the timeline does. - Add a getCommittedRangeCounterSampleSum selector and use it for the total. - Drop the unused labelKey from counter stats; the CLI has no localization. - Cover counter nesting with profile-query unit tests instead of an old fixture.
1 parent 9130428 commit c9baf74

7 files changed

Lines changed: 90 additions & 54 deletions

File tree

profiler-cli/schemas.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ CounterSummary:
3232
unit, graphType,
3333
color, pid, mainThreadIndex, mainThreadHandle, mainThreadName,
3434
rangeSampleCount,
35-
stats: [{ source, label, labelKey?, value, formattedValue, carbon? }]
35+
stats: [{ source, label, value, formattedValue, carbon? }]
3636
}
3737

3838
profiler-cli counter list --json

profiler-cli/src/test/integration/counter.test.ts

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
import type {
1818
CounterListResult,
1919
CounterInfoResult,
20-
ProfileInfoResult,
2120
WithContext,
2221
} from '../../protocol';
2322

@@ -97,27 +96,6 @@ describe('profiler-cli counter commands', () => {
9796
expect(parsed.stats.some((s) => s.source === 'count-range')).toBe(true);
9897
});
9998

100-
it('profile info lists counters under their process', async () => {
101-
await cli(ctx, ['load', PROFILE_WITH_COUNTER]);
102-
103-
const result = await cli(ctx, ['profile', 'info']);
104-
105-
expect(result.exitCode).toBe(0);
106-
expect(result.stdout).toContain('c-0');
107-
expect(result.stdout).toContain('Memory');
108-
});
109-
110-
it('profile info --json nests counters in their owning process', async () => {
111-
await cli(ctx, ['load', PROFILE_WITH_COUNTER]);
112-
113-
const result = await cli(ctx, ['profile', 'info', '--json']);
114-
const parsed = JSON.parse(result.stdout) as WithContext<ProfileInfoResult>;
115-
116-
const counters = parsed.processes.flatMap((p) => p.counters ?? []);
117-
expect(counters).toHaveLength(1);
118-
expect(counters[0].counterHandle).toBe('c-0');
119-
});
120-
12199
it('counter list reports when a profile has no counters', async () => {
122100
await cli(ctx, ['load', PROFILE_WITHOUT_COUNTER]);
123101

src/profile-query/formatters/counter-info.ts

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,8 @@ import {
77
getProfileRootRange,
88
getCounters,
99
getCounterSelectors,
10-
getCommittedRange,
1110
getMeta,
1211
} from 'firefox-profiler/selectors/profile';
13-
import { getSampleIndexRangeForSelection } from 'firefox-profiler/profile-logic/profile-data';
1412
import {
1513
formatBytes,
1614
formatNumber,
@@ -29,7 +27,6 @@ import type {
2927
CounterIndex,
3028
CounterTooltipDataSource,
3129
CounterTooltipFormat,
32-
CounterSamplesTable,
3330
ProfileMeta,
3431
} from 'firefox-profiler/types';
3532
import type { Store } from '../../types/store';
@@ -50,19 +47,6 @@ const RANGE_AGGREGATE_SOURCES: Set<CounterTooltipDataSource> = new Set([
5047
'committed-range-total',
5148
]);
5249

53-
function sumCountOverRange(
54-
samples: CounterSamplesTable,
55-
start: number,
56-
end: number
57-
): number {
58-
const [begin, finish] = getSampleIndexRangeForSelection(samples, start, end);
59-
let sum = 0;
60-
for (let i = begin; i < finish; i++) {
61-
sum += samples.count[i];
62-
}
63-
return sum;
64-
}
65-
6650
/**
6751
* Format a resolved counter value exactly as the timeline tooltip does, minus
6852
* the React/localization wrapping. Only the range-aggregate sources reach this,
@@ -126,7 +110,6 @@ function collectCounterStats(
126110
const selectors = getCounterSelectors(counterIndex);
127111
const counter = selectors.getCounter(state);
128112
const accumulated = selectors.getAccumulateCounterSamples(state);
129-
const committedRange = getCommittedRange(state);
130113
const meta = getMeta(state);
131114

132115
const stats: CounterStat[] = [];
@@ -144,11 +127,7 @@ function collectCounterStats(
144127
const value =
145128
row.source === 'count-range'
146129
? accumulated.countRange
147-
: sumCountOverRange(
148-
counter.samples,
149-
committedRange.start,
150-
committedRange.end
151-
);
130+
: selectors.getCommittedRangeCounterSampleSum(state);
152131

153132
const { formattedValue, carbon } = formatCounterRowValue(
154133
value,
@@ -158,7 +137,6 @@ function collectCounterStats(
158137
stats.push({
159138
source: row.source,
160139
label: row.label,
161-
labelKey: row.labelKey,
162140
value,
163141
formattedValue,
164142
carbon,

src/profile-query/formatters/profile-info.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ function applySearchFilter(
3232

3333
for (const process of processes) {
3434
const processMatches =
35-
process.name.toLowerCase().includes(query) ||
36-
String(process.pid).includes(query);
35+
process.name.toLowerCase().includes(query) || process.pid.includes(query);
3736

3837
const matchingThreads = processMatches
3938
? process.threads
@@ -113,10 +112,9 @@ export function collectProfileInfo(
113112
const countersByPid = new Map<string, CounterSummary[]>();
114113
(getCounters(state) ?? []).forEach((_, index) => {
115114
const counter = collectCounterSummary(store, threadMap, index);
116-
const pid = String(profile.threads[counter.mainThreadIndex].pid);
117-
const list = countersByPid.get(pid) ?? [];
115+
const list = countersByPid.get(counter.pid) ?? [];
118116
list.push(counter);
119-
countersByPid.set(pid, list);
117+
countersByPid.set(counter.pid, list);
120118
});
121119

122120
const processesData: ProfileInfoResult['processes'] = processesToShow.map(
@@ -152,7 +150,7 @@ export function collectProfileInfo(
152150
cpuMs: thread.cpuMs,
153151
})),
154152
remainingThreads: processItem.remainingThreads,
155-
counters: countersByPid.get(String(processItem.pid)),
153+
counters: countersByPid.get(processItem.pid),
156154
};
157155
}
158156
);

src/profile-query/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,6 @@ export type ThreadPageLoadResult = {
679679
export type CounterStat = {
680680
source: CounterTooltipDataSource;
681681
label: string;
682-
labelKey?: string;
683682
value: number;
684683
formattedValue: string;
685684
carbon?: string;

src/selectors/profile.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
getFriendlyThreadName,
1717
processCounter,
1818
getInclusiveSampleIndexRangeForSelection,
19+
getSampleIndexRangeForSelection,
1920
computeInnerWindowIDToTabMap,
2021
computeTabToThreadIndexesMap,
2122
computeStackTableFromRawStackTable,
@@ -379,6 +380,23 @@ function _createCounterSelectors(counterIndex: CounterIndex) {
379380
)
380381
);
381382

383+
const getCommittedRangeCounterSampleSum: Selector<number> = createSelector(
384+
getCounter,
385+
getCommittedRange,
386+
(counter, range) => {
387+
const [begin, end] = getSampleIndexRangeForSelection(
388+
counter.samples,
389+
range.start,
390+
range.end
391+
);
392+
let sum = 0;
393+
for (let i = begin; i < end; i++) {
394+
sum += counter.samples.count[i];
395+
}
396+
return sum;
397+
}
398+
);
399+
382400
return {
383401
getCounter,
384402
getDescription,
@@ -387,6 +405,7 @@ function _createCounterSelectors(counterIndex: CounterIndex) {
387405
getMaxCounterSampleCountPerMs,
388406
getMaxRangeCounterSampleCountPerMs,
389407
getCommittedRangeCounterSampleRange,
408+
getCommittedRangeCounterSampleSum,
390409
};
391410
}
392411

src/test/unit/profile-query/profile-querier.test.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717
*/
1818

1919
import { ProfileQuerier } from 'firefox-profiler/profile-query';
20-
import { getProfileFromTextSamples } from '../../fixtures/profiles/processed-profile';
20+
import {
21+
getProfileFromTextSamples,
22+
getCounterForThread,
23+
} from '../../fixtures/profiles/processed-profile';
2124
import { getProfileRootRange } from 'firefox-profiler/selectors/profile';
2225
import { storeWithProfile } from '../../fixtures/stores';
2326

@@ -344,6 +347,67 @@ describe('ProfileQuerier', function () {
344347
});
345348
});
346349

350+
describe('counters', function () {
351+
function profileWithMemoryCounter() {
352+
const { profile } = getProfileFromTextSamples(`
353+
0 1 2
354+
A A A
355+
B B B
356+
`);
357+
const counter = getCounterForThread(profile.threads[0], 0, {
358+
name: 'malloc',
359+
category: 'Memory',
360+
hasCountNumber: true,
361+
});
362+
profile.counters = [counter];
363+
return { profile, counter };
364+
}
365+
366+
function querierFor(profile: Parameters<typeof storeWithProfile>[0]) {
367+
const store = storeWithProfile(profile);
368+
return new ProfileQuerier(store, getProfileRootRange(store.getState()));
369+
}
370+
371+
it('counterList returns a schema-driven summary per counter', async function () {
372+
const { profile } = profileWithMemoryCounter();
373+
const result = await querierFor(profile).counterList();
374+
375+
expect(result.counters).toHaveLength(1);
376+
expect(result.counters[0].counterHandle).toBe('c-0');
377+
expect(result.counters[0].label).toBe('Memory');
378+
expect(
379+
result.counters[0].stats.some((s) => s.source === 'count-range')
380+
).toBe(true);
381+
});
382+
383+
it('profileInfo nests each counter under its owning process', async function () {
384+
const { profile, counter } = profileWithMemoryCounter();
385+
const info = await querierFor(profile).profileInfo();
386+
387+
const owningProcess = info.processes.find((p) => p.pid === counter.pid);
388+
expect(owningProcess).toBeDefined();
389+
expect(owningProcess!.counters?.map((c) => c.counterHandle)).toEqual([
390+
'c-0',
391+
]);
392+
});
393+
394+
it('counterInfo resolves a counter by handle', async function () {
395+
const { profile } = profileWithMemoryCounter();
396+
const info = await querierFor(profile).counterInfo('c-0');
397+
398+
expect(info.counterHandle).toBe('c-0');
399+
expect(info.description).toBe('My Description');
400+
expect(info.sampleCount).toBeGreaterThan(0);
401+
});
402+
403+
it('counterInfo throws for an unknown handle', async function () {
404+
const { profile } = profileWithMemoryCounter();
405+
await expect(querierFor(profile).counterInfo('c-9')).rejects.toThrow(
406+
'Unknown counter c-9'
407+
);
408+
});
409+
});
410+
347411
describe('threadSamples', function () {
348412
it('searches all roots when choosing the heaviest stack', async function () {
349413
const { profile } = getProfileFromTextSamples(`

0 commit comments

Comments
 (0)