Skip to content

Commit eccf8c3

Browse files
committed
Reduce allocations for StackLineInfo and StackAddressInfo.
Before: https://share.firefox.dev/3NnthpO After: https://share.firefox.dev/45aHlZW getStackLineInfo now allocates 74MB instead of 220MB, and takes 217ms instead of 338ms, when opening the source view for mozjemalloc.cpp on https://share.firefox.dev/3NnzEcF
1 parent 7dd0148 commit eccf8c3

File tree

7 files changed

+233
-120
lines changed

7 files changed

+233
-120
lines changed

src/profile-logic/address-timings.ts

Lines changed: 40 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ import type {
7676
StackAddressInfo,
7777
AddressTimings,
7878
Address,
79+
IndexIntoAddressSetTable,
7980
} from 'firefox-profiler/types';
81+
import { IntSetTableBuilder } from 'firefox-profiler/utils/intset-table';
8082

8183
/**
8284
* For each stack in `stackTable`, and one specific native symbol, compute the
@@ -131,48 +133,32 @@ export function getStackAddressInfo(
131133
_funcTable: FuncTable,
132134
nativeSymbol: IndexIntoNativeSymbolTable
133135
): StackAddressInfo {
134-
// "self address" == "the address which a stack's self time is contributed to"
135-
const selfAddressForAllStacks = [];
136-
// "total addresses" == "the set of addresses whose total time this stack contributes to"
137-
const totalAddressesForAllStacks: Array<Set<Address> | null> = [];
138136

139-
// This loop takes advantage of the fact that the stack table is topologically ordered:
140-
// Prefix stacks are always visited before their descendants.
141-
// Each stack inherits the "total" addresses from its parent stack, and then adds its
142-
// self address to that set. If the stack doesn't have a self address in the library, we just
143-
// re-use the prefix's set object without copying it.
137+
138+
const builder = new IntSetTableBuilder();
139+
const stackIndexToAddressSetIndex = new Int32Array(stackTable.length);
140+
144141
for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) {
145-
const frame = stackTable.frame[stackIndex];
146142
const prefixStack = stackTable.prefix[stackIndex];
147-
const nativeSymbolOfThisStack = frameTable.nativeSymbol[frame];
148-
149-
let selfAddress: Address | null = null;
150-
let totalAddresses: Set<Address> | null =
151-
prefixStack !== null ? totalAddressesForAllStacks[prefixStack] : null;
143+
const prefixAddressSet: IndexIntoAddressSetTable | -1 =
144+
prefixStack !== null ? stackIndexToAddressSetIndex[prefixStack] : -1;
145+
const prefixAddressSetOrNull = prefixAddressSet !== -1 ? prefixAddressSet : null;
152146

153-
if (nativeSymbolOfThisStack === nativeSymbol) {
154-
selfAddress = frameTable.address[frame];
155-
if (selfAddress !== -1) {
156-
// Add this stack's address to this stack's totalAddresses. The rest of this stack's
157-
// totalAddresses is the same as for the parent stack.
158-
// We avoid creating new Set objects unless the new set is actually
159-
// different.
160-
if (totalAddresses === null) {
161-
// None of the ancestor stack nodes have hit a address in the given library.
162-
totalAddresses = new Set([selfAddress]);
163-
} else if (!totalAddresses.has(selfAddress)) {
164-
totalAddresses = new Set(totalAddresses);
165-
totalAddresses.add(selfAddress);
166-
}
167-
}
168-
}
147+
const frame = stackTable.frame[stackIndex];
148+
const nativeSymbolOfThisStack = frameTable.nativeSymbol[frame];
149+
const selfAddress =
150+
nativeSymbolOfThisStack === nativeSymbol
151+
? (frameTable.address[frame])
152+
: -1;
169153

170-
selfAddressForAllStacks.push(selfAddress);
171-
totalAddressesForAllStacks.push(totalAddresses);
154+
stackIndexToAddressSetIndex[stackIndex] = builder.indexForSetWithParentAndSelf(
155+
prefixAddressSetOrNull,
156+
selfAddress
157+
);
172158
}
173159
return {
174-
selfAddress: selfAddressForAllStacks,
175-
stackAddresses: totalAddressesForAllStacks,
160+
stackIndexToAddressSetIndex,
161+
addressSetTable: builder.finish(),
176162
};
177163
}
178164

@@ -230,7 +216,7 @@ export function getAddressTimings(
230216
if (stackAddressInfo === null) {
231217
return emptyAddressTimings;
232218
}
233-
const { selfAddress, stackAddresses } = stackAddressInfo;
219+
const { stackIndexToAddressSetIndex, addressSetTable } = stackAddressInfo;
234220
const totalAddressHits: Map<Address, number> = new Map();
235221
const selfAddressHits: Map<Address, number> = new Map();
236222

@@ -243,18 +229,28 @@ export function getAddressTimings(
243229
continue;
244230
}
245231
const weight = samples.weight ? samples.weight[sampleIndex] : 1;
246-
const setOfHitAddresses = stackAddresses[stackIndex];
247-
if (setOfHitAddresses !== null) {
248-
for (const address of setOfHitAddresses) {
232+
const addressSetIndex = stackIndexToAddressSetIndex[stackIndex];
233+
if (addressSetIndex === -1) {
234+
continue;
235+
}
236+
237+
const selfAddress = addressSetTable.self[addressSetIndex];
238+
if (selfAddress !== -1) {
239+
const oldHitCount = selfAddressHits.get(selfAddress) ?? 0;
240+
selfAddressHits.set(selfAddress, oldHitCount + weight);
241+
}
242+
243+
for (
244+
let node: IndexIntoAddressSetTable | null = addressSetIndex;
245+
node !== null;
246+
node = addressSetTable.prefix[node]
247+
) {
248+
const address = addressSetTable.value[node];
249+
if (address !== -1) {
249250
const oldHitCount = totalAddressHits.get(address) ?? 0;
250251
totalAddressHits.set(address, oldHitCount + weight);
251252
}
252253
}
253-
const address = selfAddress[stackIndex];
254-
if (address !== null) {
255-
const oldHitCount = selfAddressHits.get(address) ?? 0;
256-
selfAddressHits.set(address, oldHitCount + weight);
257-
}
258254
}
259255
return { totalAddressHits, selfAddressHits };
260256
}

src/profile-logic/line-timings.ts

Lines changed: 38 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import type {
1111
LineTimings,
1212
LineNumber,
1313
IndexIntoSourceTable,
14+
IndexIntoLineSetTable,
1415
} from 'firefox-profiler/types';
16+
import { IntSetTableBuilder } from 'firefox-profiler/utils/intset-table';
1517

1618
/**
1719
* For each stack in `stackTable`, and one specific source file, compute the
@@ -56,53 +58,31 @@ export function getStackLineInfo(
5658
funcTable: FuncTable,
5759
sourceViewSourceIndex: IndexIntoSourceTable
5860
): StackLineInfo {
59-
// "self line" == "the line which a stack's self time is contributed to"
60-
const selfLineForAllStacks = [];
61-
// "total lines" == "the set of lines whose total time this stack contributes to"
62-
const totalLinesForAllStacks: Array<Set<LineNumber> | null> = [];
61+
const builder = new IntSetTableBuilder();
62+
const stackIndexToLineSetIndex = new Int32Array(stackTable.length);
6363

64-
// This loop takes advantage of the fact that the stack table is topologically ordered:
65-
// Prefix stacks are always visited before their descendants.
66-
// Each stack inherits the "total" lines from its parent stack, and then adds its
67-
// self line to that set. If the stack doesn't have a self line in the file, we just
68-
// re-use the prefix's set object without copying it.
6964
for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) {
70-
const frame = stackTable.frame[stackIndex];
7165
const prefixStack = stackTable.prefix[stackIndex];
66+
const prefixLineSet: IndexIntoLineSetTable | -1 =
67+
prefixStack !== null ? stackIndexToLineSetIndex[prefixStack] : -1;
68+
const prefixLineSetOrNull = prefixLineSet !== -1 ? prefixLineSet : null;
69+
70+
const frame = stackTable.frame[stackIndex];
7271
const func = frameTable.func[frame];
7372
const sourceIndexOfThisStack = funcTable.source[func];
73+
const selfLineOrNull =
74+
sourceIndexOfThisStack === sourceViewSourceIndex
75+
? (frameTable.line[frame] ?? funcTable.lineNumber[func])
76+
: null;
7477

75-
let selfLine: LineNumber | null = null;
76-
let totalLines: Set<LineNumber> | null =
77-
prefixStack !== null ? totalLinesForAllStacks[prefixStack] : null;
78-
79-
if (sourceIndexOfThisStack === sourceViewSourceIndex) {
80-
selfLine = frameTable.line[frame];
81-
// Fallback to func line info if frame line info is not available
82-
if (selfLine === null) {
83-
selfLine = funcTable.lineNumber[func];
84-
}
85-
if (selfLine !== null) {
86-
// Add this stack's line to this stack's totalLines. The rest of this stack's
87-
// totalLines is the same as for the parent stack.
88-
// We avoid creating new Set objects unless the new set is actually
89-
// different.
90-
if (totalLines === null) {
91-
// None of the ancestor stack nodes have hit a line in the given file.
92-
totalLines = new Set([selfLine]);
93-
} else if (!totalLines.has(selfLine)) {
94-
totalLines = new Set(totalLines);
95-
totalLines.add(selfLine);
96-
}
97-
}
98-
}
99-
100-
selfLineForAllStacks.push(selfLine);
101-
totalLinesForAllStacks.push(totalLines);
78+
stackIndexToLineSetIndex[stackIndex] = builder.indexForSetWithParentAndSelf(
79+
prefixLineSetOrNull,
80+
selfLineOrNull !== null ? selfLineOrNull : -1
81+
);
10282
}
10383
return {
104-
selfLine: selfLineForAllStacks,
105-
stackLines: totalLinesForAllStacks,
84+
stackIndexToLineSetIndex,
85+
lineSetTable: builder.finish(),
10686
};
10787
}
10888

@@ -153,7 +133,7 @@ export function getLineTimings(
153133
if (stackLineInfo === null) {
154134
return emptyLineTimings;
155135
}
156-
const { selfLine, stackLines } = stackLineInfo;
136+
const { stackIndexToLineSetIndex, lineSetTable } = stackLineInfo;
157137
const totalLineHits: Map<LineNumber, number> = new Map();
158138
const selfLineHits: Map<LineNumber, number> = new Map();
159139

@@ -166,18 +146,28 @@ export function getLineTimings(
166146
continue;
167147
}
168148
const weight = samples.weight ? samples.weight[sampleIndex] : 1;
169-
const setOfHitLines = stackLines[stackIndex];
170-
if (setOfHitLines !== null) {
171-
for (const line of setOfHitLines) {
149+
const lineSetIndex = stackIndexToLineSetIndex[stackIndex];
150+
if (lineSetIndex === -1) {
151+
continue;
152+
}
153+
154+
const selfAddress = lineSetTable.self[lineSetIndex];
155+
if (selfAddress !== -1) {
156+
const oldHitCount = selfLineHits.get(selfAddress) ?? 0;
157+
selfLineHits.set(selfAddress, oldHitCount + weight);
158+
}
159+
160+
for (
161+
let node: IndexIntoLineSetTable | null = lineSetIndex;
162+
node !== null;
163+
node = lineSetTable.prefix[node]
164+
) {
165+
const line = lineSetTable.value[node];
166+
if (line !== -1) {
172167
const oldHitCount = totalLineHits.get(line) ?? 0;
173168
totalLineHits.set(line, oldHitCount + weight);
174169
}
175170
}
176-
const line = selfLine[stackIndex];
177-
if (line !== null) {
178-
const oldHitCount = selfLineHits.get(line) ?? 0;
179-
selfLineHits.set(line, oldHitCount + weight);
180-
}
181171
}
182172
return { totalLineHits, selfLineHits };
183173
}

src/test/unit/address-timings.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ describe('getStackAddressInfo', function () {
4343

4444
// Expect the returned arrays to have the same length as the stackTable.
4545
expect(stackTable.length).toBe(9);
46-
expect(stackLineInfoOne.selfAddress.length).toBe(9);
47-
expect(stackLineInfoOne.stackAddresses.length).toBe(9);
46+
expect(stackLineInfoOne.stackIndexToAddressSetIndex.length).toBe(9);
47+
expect(stackLineInfoOne.addressSetTable.length).toBe(4);
4848
});
4949
});
5050

src/test/unit/line-timings.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ describe('getStackLineInfo', function () {
4343

4444
// Expect the returned arrays to have the same length as the stackTable.
4545
expect(stackTable.length).toBe(9);
46-
expect(stackLineInfoOne.selfLine.length).toBe(9);
47-
expect(stackLineInfoOne.stackLines.length).toBe(9);
46+
expect(stackLineInfoOne.lineSetTable.length).toBe(8);
47+
// expect(stackLineInfoOne.stackLines.length).toBe(9);
4848
});
4949
});
5050

src/types/profile-derived.ts

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import type { IndexedArray } from './utils';
3939
import type { BitSet } from '../utils/bitset';
4040
import type { StackTiming } from '../profile-logic/stack-timing';
4141
import type { StringTable } from '../utils/string-table';
42+
import type { IndexIntoIntSetTable, IntSetTable } from 'firefox-profiler/utils/intset-table';
4243
export type IndexIntoCallNodeTable = number;
4344

4445
/**
@@ -337,20 +338,12 @@ export type LineNumber = number;
337338
// stackLine may be null. This is fine because their values are not accessed
338339
// during the LineTimings computation.
339340
export type StackLineInfo = {
340-
// An array that contains, for each "self" stack, the line number that this stack
341-
// spends its self time in, in this file, or null if the self time of the
342-
// stack is in a different file or if the line number is not known.
343-
// For non-"self" stacks, i.e. stacks which are only used as prefix stacks and
344-
// never referred to from a SamplesLikeTable, the value may be null.
345-
selfLine: Array<LineNumber | null>;
346-
// An array that contains, for each "self" stack, all the lines that the frames in
347-
// this stack hit in this file, or null if this stack does not hit any line
348-
// in the given file.
349-
// For non-"self" stacks, i.e. stacks which are only used as prefix stacks and
350-
// never referred to from a SamplesLikeTable, the value may be null.
351-
stackLines: Array<Set<LineNumber> | null>;
341+
stackIndexToLineSetIndex: Int32Array;
342+
lineSetTable: IntSetTable;
352343
};
353344

345+
export type IndexIntoLineSetTable = IndexIntoIntSetTable;
346+
354347
// Stores, for all lines of one specific file, how many times each line is hit
355348
// by samples in a thread. The maps only contain non-zero values.
356349
// So map.get(line) === undefined should be treated as zero.
@@ -377,20 +370,12 @@ export type LineTimings = {
377370
// stackAddress may be null. This is fine because their values are not accessed
378371
// during the AddressTimings computation.
379372
export type StackAddressInfo = {
380-
// An array that contains, for each "self" stack, the address that this stack
381-
// spends its self time in, in this native symbol, or null if the self time of
382-
// the stack is in a different native symbol or if the address is not known.
383-
// For non-"self" stacks, i.e. stacks which are only used as prefix stacks and
384-
// never referred to from a SamplesLikeTable, the value may be null.
385-
selfAddress: Array<Address | null>;
386-
// An array that contains, for each "self" stack, all the addresses that the
387-
// frames in this stack hit in this native symbol, or null if this stack does
388-
// not hit any address in the given native symbol.
389-
// For non-"self" stacks, i.e. stacks which are only used as prefix stacks and
390-
// never referred to from a SamplesLikeTable, the value may be null.
391-
stackAddresses: Array<Set<Address> | null>;
373+
stackIndexToAddressSetIndex: Int32Array;
374+
addressSetTable: IntSetTable;
392375
};
393376

377+
export type IndexIntoAddressSetTable = IndexIntoIntSetTable;
378+
394379
// Stores, for all addresses of one specific library, how many times each
395380
// address is hit by samples in a thread. The maps only contain non-zero values.
396381
// So map.get(address) === undefined should be treated as zero.

src/utils/bitset.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,25 @@ export function checkBit(bitSet: BitSet, bitIndex: number): boolean {
4545
return (bitSet[q] & (1 << r)) !== 0;
4646
}
4747

48+
export function makeBitSetFromBoolArray(arr: boolean[]): BitSet {
49+
const length = arr.length;
50+
const bitset = makeBitSet(length);
51+
for (let chunkStart = 0; chunkStart < length; chunkStart += 32) {
52+
let chunkVal = 0;
53+
for (let bitIndex = 0; bitIndex < 32; bitIndex++) {
54+
const index = chunkStart + bitIndex;
55+
if (index >= length) {
56+
break;
57+
}
58+
if (arr[index]) {
59+
chunkVal |= 1 << bitIndex;
60+
}
61+
}
62+
bitset[chunkStart >> 5] = chunkVal;
63+
}
64+
return bitset;
65+
}
66+
4867
export class BitSetOutOfBoundsError extends Error {
4968
override name = 'BitSetOutOfBoundsError';
5069
bitIndex: number;

0 commit comments

Comments
 (0)