Skip to content

Commit f8bff82

Browse files
committed
Simplify the getStackLineInfo code by passing sourceIndex instead of filename
Now that we changed the place where we send information to this place, we can simplify it.
1 parent 533c711 commit f8bff82

File tree

4 files changed

+33
-44
lines changed

4 files changed

+33
-44
lines changed

src/profile-logic/line-timings.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ import type {
88
StackTable,
99
SamplesLikeTable,
1010
IndexIntoCallNodeTable,
11-
IndexIntoStringTable,
1211
StackLineInfo,
1312
LineTimings,
1413
LineNumber,
15-
SourceTable,
14+
IndexIntoSourceTable,
1615
} from 'firefox-profiler/types';
1716

1817
import { getMatchingAncestorStackForInvertedCallNode } from './profile-data';
@@ -59,8 +58,7 @@ export function getStackLineInfo(
5958
stackTable: StackTable,
6059
frameTable: FrameTable,
6160
funcTable: FuncTable,
62-
fileNameStringIndex: IndexIntoStringTable,
63-
sources: SourceTable
61+
sourceViewSourceIndex: IndexIntoSourceTable
6462
): StackLineInfo {
6563
// "self line" == "the line which a stack's self time is contributed to"
6664
const selfLineForAllStacks = [];
@@ -76,15 +74,13 @@ export function getStackLineInfo(
7674
const frame = stackTable.frame[stackIndex];
7775
const prefixStack = stackTable.prefix[stackIndex];
7876
const func = frameTable.func[frame];
79-
const sourceIndex = funcTable.source[func];
80-
const fileNameStringIndexOfThisStack =
81-
sourceIndex !== null ? (sources.filename[sourceIndex] ?? null) : null;
77+
const sourceIndexOfThisStack = funcTable.source[func];
8278

8379
let selfLine: LineNumber | null = null;
8480
let totalLines: Set<LineNumber> | null =
8581
prefixStack !== null ? totalLinesForAllStacks[prefixStack] : null;
8682

87-
if (fileNameStringIndexOfThisStack === fileNameStringIndex) {
83+
if (sourceIndexOfThisStack === sourceViewSourceIndex) {
8884
selfLine = frameTable.line[frame];
8985
if (selfLine !== null) {
9086
// Add this stack's line to this stack's totalLines. The rest of this stack's

src/selectors/per-thread/stack-sample.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -140,23 +140,15 @@ export function getStackAndSampleSelectorsPerThread(
140140
const getSourceViewStackLineInfo: Selector<StackLineInfo | null> =
141141
createSelector(
142142
threadSelectors.getFilteredThread,
143-
ProfileSelectors.getSourceViewFile,
144-
ProfileSelectors.getSourceTable,
143+
UrlState.getSourceViewSourceIndex,
145144
(
146-
{ stackTable, frameTable, funcTable, stringTable }: Thread,
147-
sourceViewFile,
148-
sources
145+
{ stackTable, frameTable, funcTable }: Thread,
146+
sourceIndex
149147
): StackLineInfo | null => {
150-
if (sourceViewFile === null) {
148+
if (sourceIndex === null) {
151149
return null;
152150
}
153-
return getStackLineInfo(
154-
stackTable,
155-
frameTable,
156-
funcTable,
157-
stringTable.indexForString(sourceViewFile),
158-
sources
159-
);
151+
return getStackLineInfo(stackTable, frameTable, funcTable, sourceIndex);
160152
}
161153
);
162154

src/test/store/transforms.test.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import {
2626
changeCallTreeSummaryStrategy,
2727
} from '../../actions/profile-view';
2828
import { selectedThreadSelectors } from '../../selectors/per-thread';
29-
import * as ProfileSelectors from '../../selectors/profile';
3029

3130
describe('"focus-subtree" transform', function () {
3231
describe('on a call tree', function () {
@@ -1224,13 +1223,13 @@ describe('"collapse-direct-recursion" transform', function () {
12241223
const { stackTable, frameTable, funcTable, samples, stringTable } =
12251224
filteredThread;
12261225
const fileStringIndex = stringTable.indexForString('b');
1227-
const sources = ProfileSelectors.getSourceTable(getState());
1226+
const fileSourceIndex =
1227+
profile.shared.sources.filename.indexOf(fileStringIndex);
12281228
const stackLineInfo = getStackLineInfo(
12291229
stackTable,
12301230
frameTable,
12311231
funcTable,
1232-
fileStringIndex,
1233-
sources
1232+
fileSourceIndex
12341233
);
12351234
const lineTimings = getLineTimings(stackLineInfo, samples);
12361235

@@ -1408,13 +1407,13 @@ describe('"collapse-recursion" transform', function () {
14081407
const { stackTable, frameTable, funcTable, samples, stringTable } =
14091408
filteredThread;
14101409
const fileStringIndex = stringTable.indexForString('b');
1411-
const sources = ProfileSelectors.getSourceTable(getState());
1410+
const fileSourceIndex =
1411+
profile.shared.sources.filename.indexOf(fileStringIndex);
14121412
const stackLineInfo = getStackLineInfo(
14131413
stackTable,
14141414
frameTable,
14151415
funcTable,
1416-
fileStringIndex,
1417-
sources
1416+
fileSourceIndex
14181417
);
14191418
const lineTimings = getLineTimings(stackLineInfo, samples);
14201419

@@ -1521,13 +1520,13 @@ describe('"collapse-recursion" transform', function () {
15211520
const { stackTable, frameTable, funcTable, samples, stringTable } =
15221521
filteredThread;
15231522
const fileStringIndex = stringTable.indexForString('b');
1524-
const sources = ProfileSelectors.getSourceTable(getState());
1523+
const fileSourceIndex =
1524+
profile.shared.sources.filename.indexOf(fileStringIndex);
15251525
const stackLineInfo = getStackLineInfo(
15261526
stackTable,
15271527
frameTable,
15281528
funcTable,
1529-
fileStringIndex,
1530-
sources
1529+
fileSourceIndex
15311530
);
15321531
const lineTimings = getLineTimings(stackLineInfo, samples);
15331532

@@ -1657,13 +1656,13 @@ describe('"collapse-recursion" transform', function () {
16571656
const { stackTable, frameTable, funcTable, samples, stringTable } =
16581657
filteredThread;
16591658
const fileStringIndex = stringTable.indexForString('b');
1660-
const sources = ProfileSelectors.getSourceTable(getState());
1659+
const fileSourceIndex =
1660+
profile.shared.sources.filename.indexOf(fileStringIndex);
16611661
const stackLineInfo = getStackLineInfo(
16621662
stackTable,
16631663
frameTable,
16641664
funcTable,
1665-
fileStringIndex,
1666-
sources
1665+
fileSourceIndex
16671666
);
16681667
const lineTimings = getLineTimings(stackLineInfo, samples);
16691668

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import type {
1717
CallNodePath,
1818
Thread,
1919
IndexIntoCategoryList,
20-
SourceTable,
20+
Profile,
2121
} from 'firefox-profiler/types';
2222

2323
describe('getStackLineInfo', function () {
@@ -31,13 +31,14 @@ describe('getStackLineInfo', function () {
3131
const [thread] = derivedThreads;
3232
const { stackTable, frameTable, funcTable, stringTable } = thread;
3333

34-
const fileOne = stringTable.indexForString('one.js');
34+
const fileOneStringIndex = stringTable.indexForString('one.js');
35+
const fileOneSourceIndex =
36+
profile.shared.sources.filename.indexOf(fileOneStringIndex);
3537
const stackLineInfoOne = getStackLineInfo(
3638
stackTable,
3739
frameTable,
3840
funcTable,
39-
fileOne,
40-
profile.shared.sources
41+
fileOneSourceIndex
4142
);
4243

4344
// Expect the returned arrays to have the same length as the stackTable.
@@ -48,15 +49,16 @@ describe('getStackLineInfo', function () {
4849
});
4950

5051
describe('getLineTimings for getStackLineInfo', function () {
51-
function getTimings(thread: Thread, file: string, sources: SourceTable) {
52+
function getTimings(profile: Profile, thread: Thread, file: string) {
5253
const { stackTable, frameTable, funcTable, samples, stringTable } = thread;
5354
const fileStringIndex = stringTable.indexForString(file);
55+
const fileSourceIndex =
56+
profile.shared.sources.filename.indexOf(fileStringIndex);
5457
const stackLineInfo = getStackLineInfo(
5558
stackTable,
5659
frameTable,
5760
funcTable,
58-
fileStringIndex,
59-
sources
61+
fileSourceIndex
6062
);
6163
return getLineTimings(stackLineInfo, samples);
6264
}
@@ -69,7 +71,7 @@ describe('getLineTimings for getStackLineInfo', function () {
6971
B[file:file.js][line:30]
7072
`);
7173
const [thread] = derivedThreads;
72-
const lineTimings = getTimings(thread, 'file.js', profile.shared.sources);
74+
const lineTimings = getTimings(profile, thread, 'file.js');
7375
expect(lineTimings.totalLineHits.get(20)).toBe(1);
7476
expect(lineTimings.totalLineHits.get(30)).toBe(1);
7577
expect(lineTimings.totalLineHits.size).toBe(2); // no other hits
@@ -87,7 +89,7 @@ describe('getLineTimings for getStackLineInfo', function () {
8789
`);
8890
const [thread] = derivedThreads;
8991

90-
const lineTimingsOne = getTimings(thread, 'one.js', profile.shared.sources);
92+
const lineTimingsOne = getTimings(profile, thread, 'one.js');
9193
expect(lineTimingsOne.totalLineHits.get(20)).toBe(2);
9294
expect(lineTimingsOne.totalLineHits.get(21)).toBe(1);
9395
// one.js line 30 was hit in every sample, twice in the first sample
@@ -101,7 +103,7 @@ describe('getLineTimings for getStackLineInfo', function () {
101103
expect(lineTimingsOne.selfLineHits.get(30)).toBe(1);
102104
expect(lineTimingsOne.selfLineHits.size).toBe(1); // no other hits
103105

104-
const lineTimingsTwo = getTimings(thread, 'two.js', profile.shared.sources);
106+
const lineTimingsTwo = getTimings(profile, thread, 'two.js');
105107
expect(lineTimingsTwo.totalLineHits.get(10)).toBe(1);
106108
expect(lineTimingsTwo.totalLineHits.get(11)).toBe(1);
107109
expect(lineTimingsTwo.totalLineHits.get(40)).toBe(1);

0 commit comments

Comments
 (0)