Skip to content

Commit e386949

Browse files
mstangemstange
authored
Fix search filtering (#5599)
[Production](https://profiler.firefox.com/public/pwsq3dj62mwhhrn7ync08ec0hma303me5bg1y5g/flame-graph/?globalTrackOrder=a08w6341259&hiddenGlobalTracks=0w8a&hiddenLocalTracksByPid=78348-1w4~78359-0~78353-0~78351-0~78363-0~78385-0~78362-0~78352-0~78354-0w2~78361-01&range=875m803~1585159u6480~1585190u1182&search=fetch&thread=f&v=11) | [Deploy preview](https://deploy-preview-5599--perf-html.netlify.app/public/pwsq3dj62mwhhrn7ync08ec0hma303me5bg1y5g/flame-graph/?globalTrackOrder=a08w6341259&hiddenGlobalTracks=0w8a&hiddenLocalTracksByPid=78348-1w4~78359-0~78353-0~78351-0~78363-0~78385-0~78362-0~78352-0~78354-0w2~78361-01&range=875m803~1585159u6480~1585190u1182&search=fetch&thread=f&v=11) Fixes #5598.
2 parents 71f1b0b + 3f01b04 commit e386949

File tree

3 files changed

+189
-1
lines changed

3 files changed

+189
-1
lines changed

‎src/profile-logic/profile-data.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1538,7 +1538,7 @@ export function filterThreadToSearchString(
15381538
}
15391539
}
15401540

1541-
const stackMatchesSearch = makeBitSet(funcTable.length);
1541+
const stackMatchesSearch = makeBitSet(stackTable.length);
15421542
for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) {
15431543
const prefix = stackTable.prefix[stackIndex];
15441544
if (prefix !== null && checkBit(stackMatchesSearch, prefix)) {

‎src/test/store/profile-view.test.ts‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { withAnalyticsMock } from '../fixtures/mocks/analytics';
2727
import { getProfileWithNiceTracks } from '../fixtures/profiles/tracks';
2828
import { blankStore, storeWithProfile } from '../fixtures/stores';
2929
import { assertSetContainsOnly } from '../fixtures/custom-assertions';
30+
import { formatTree } from '../fixtures/utils';
3031

3132
import * as App from '../../actions/app';
3233
import * as ProfileView from '../../actions/profile-view';
@@ -750,6 +751,175 @@ describe('actions/ProfileView', function () {
750751
});
751752
});
752753
});
754+
755+
function setup() {
756+
// Create a profile which has more than 32 stacks, so that, if any parts of the implementation
757+
// use a BitSet to keep track of something that's per-stack (such as whether a stack matches
758+
// the search filter), the BitSet needs at least two 32-bit slots.
759+
const { profile } = getProfileFromTextSamples(`
760+
A[lib:K][file:S] A[lib:K][file:S] A[lib:K][file:S] D[lib:nNn][file:uV] C[lib:m][file:t]
761+
B[lib:L][file:t] B[lib:L][file:t] E[lib:O][file:Pq]
762+
A[lib:K][file:S] C[lib:m][file:t]
763+
B[lib:L][file:t] D[lib:n][file:uV]
764+
B[lib:L][file:t]
765+
B[lib:L][file:t]
766+
B[lib:L][file:t]
767+
B[lib:L][file:t]
768+
B[lib:L][file:t]
769+
B[lib:L][file:t]
770+
B[lib:L][file:t]
771+
B[lib:L][file:t]
772+
B[lib:L][file:t]
773+
B[lib:L][file:t]
774+
B[lib:L][file:t]
775+
B[lib:L][file:t]
776+
B[lib:L][file:t]
777+
B[lib:L][file:t]
778+
B[lib:L][file:t]
779+
B[lib:L][file:t]
780+
B[lib:L][file:t]
781+
B[lib:L][file:t]
782+
B[lib:L][file:t]
783+
B[lib:L][file:t]
784+
B[lib:L][file:t]
785+
B[lib:L][file:t]
786+
B[lib:L][file:t]
787+
B[lib:L][file:t]
788+
B[lib:L][file:t]
789+
B[lib:L][file:t]
790+
B[lib:L][file:t]
791+
B[lib:L][file:t]
792+
B[lib:L][file:t]
793+
B[lib:L][file:t]
794+
`);
795+
796+
const { dispatch, getState } = storeWithProfile(profile);
797+
return { dispatch, getState, profile };
798+
}
799+
800+
it('starts as an unfiltered call tree', function () {
801+
const { getState } = setup();
802+
const originalCallTree = selectedThreadSelectors.getCallTree(getState());
803+
expect(formatTree(originalCallTree)).toEqual([
804+
'- A (total: 3, self: —)',
805+
' - B (total: 2, self: —)',
806+
' - A (total: 1, self: —)',
807+
' - B (total: 1, self: —)',
808+
' - B (total: 1, self: —)',
809+
' - B (total: 1, self: —)',
810+
' - B (total: 1, self: —)',
811+
' - B (total: 1, self: —)',
812+
' - B (total: 1, self: —)',
813+
' - B (total: 1, self: —)',
814+
' - B (total: 1, self: —)',
815+
' - B (total: 1, self: —)',
816+
' - B (total: 1, self: —)',
817+
' - B (total: 1, self: —)',
818+
' - B (total: 1, self: —)',
819+
' - B (total: 1, self: —)',
820+
' - B (total: 1, self: —)',
821+
' - B (total: 1, self: —)',
822+
' - B (total: 1, self: —)',
823+
' - B (total: 1, self: —)',
824+
' - B (total: 1, self: —)',
825+
' - B (total: 1, self: —)',
826+
' - B (total: 1, self: —)',
827+
' - B (total: 1, self: —)',
828+
' - B (total: 1, self: —)',
829+
' - B (total: 1, self: —)',
830+
' - B (total: 1, self: —)',
831+
' - B (total: 1, self: —)',
832+
' - B (total: 1, self: —)',
833+
' - B (total: 1, self: —)',
834+
' - B (total: 1, self: —)',
835+
' - B (total: 1, self: —)',
836+
' - B (total: 1, self: —)',
837+
' - B (total: 1, self: 1)',
838+
' - C (total: 1, self: —)',
839+
' - D (total: 1, self: 1)',
840+
' - E (total: 1, self: 1)',
841+
'- D (total: 1, self: 1)',
842+
'- C (total: 1, self: 1)',
843+
]);
844+
});
845+
846+
it('filters out all samples if there is no match', function () {
847+
const { dispatch, getState } = setup();
848+
dispatch(ProfileView.changeCallTreeSearchString('F'));
849+
const callTree = selectedThreadSelectors.getCallTree(getState());
850+
expect(formatTree(callTree)).toEqual([]);
851+
});
852+
853+
it('filters based on function names', function () {
854+
const { dispatch, getState } = setup();
855+
dispatch(ProfileView.changeCallTreeSearchString('c'));
856+
const callTree = selectedThreadSelectors.getCallTree(getState());
857+
// Keep all stacks which include function C
858+
expect(formatTree(callTree)).toEqual([
859+
'- A (total: 1, self: —)',
860+
' - B (total: 1, self: —)',
861+
' - C (total: 1, self: —)',
862+
' - D (total: 1, self: 1)',
863+
'- C (total: 1, self: 1)',
864+
]);
865+
866+
// Also test with uppercase 'C'
867+
dispatch(ProfileView.changeCallTreeSearchString('C'));
868+
const callTree2 = selectedThreadSelectors.getCallTree(getState());
869+
// Keep all stacks which include function C
870+
expect(formatTree(callTree2)).toEqual([
871+
'- A (total: 1, self: —)',
872+
' - B (total: 1, self: —)',
873+
' - C (total: 1, self: —)',
874+
' - D (total: 1, self: 1)',
875+
'- C (total: 1, self: 1)',
876+
]);
877+
});
878+
879+
it('filters based on filenames', function () {
880+
const { dispatch, getState } = setup();
881+
dispatch(ProfileView.changeCallTreeSearchString('u'));
882+
const callTree_u = selectedThreadSelectors.getCallTree(getState());
883+
// Keep all stacks which include function D, which has filename uV
884+
expect(formatTree(callTree_u)).toEqual([
885+
'- A (total: 1, self: —)',
886+
' - B (total: 1, self: —)',
887+
' - C (total: 1, self: —)',
888+
' - D (total: 1, self: 1)',
889+
'- D (total: 1, self: 1)',
890+
]);
891+
dispatch(ProfileView.changeCallTreeSearchString('pQ'));
892+
const callTree_pQ = selectedThreadSelectors.getCallTree(getState());
893+
// Keep all stacks which include function E, which has filename Pq
894+
expect(formatTree(callTree_pQ)).toEqual([
895+
'- A (total: 1, self: —)',
896+
' - E (total: 1, self: 1)',
897+
]);
898+
});
899+
900+
it('filters based on library names', function () {
901+
const { dispatch, getState } = setup();
902+
dispatch(ProfileView.changeCallTreeSearchString('M'));
903+
const callTree_M = selectedThreadSelectors.getCallTree(getState());
904+
// Keep all stacks which include function C, which has lib name m
905+
expect(formatTree(callTree_M)).toEqual([
906+
'- A (total: 1, self: —)',
907+
' - B (total: 1, self: —)',
908+
' - C (total: 1, self: —)',
909+
' - D (total: 1, self: 1)',
910+
'- C (total: 1, self: 1)',
911+
]);
912+
dispatch(ProfileView.changeCallTreeSearchString('NN'));
913+
const callTree_NN = selectedThreadSelectors.getCallTree(getState());
914+
// Keep all stacks which include function D, which has filename nNn
915+
expect(formatTree(callTree_NN)).toEqual([
916+
'- A (total: 1, self: —)',
917+
' - B (total: 1, self: —)',
918+
' - C (total: 1, self: —)',
919+
' - D (total: 1, self: 1)',
920+
'- D (total: 1, self: 1)',
921+
]);
922+
});
753923
});
754924

755925
/**

‎src/utils/bitset.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,35 @@ export function makeBitSet(length: number): BitSet {
2121
export function setBit(bitSet: BitSet, bitIndex: number) {
2222
const q = bitIndex >> 5;
2323
const r = bitIndex & 0b11111;
24+
if (q >= bitSet.length) {
25+
throw new BitSetOutOfBoundsError(bitIndex);
26+
}
2427
bitSet[q] |= 1 << r;
2528
}
2629

2730
export function clearBit(bitSet: BitSet, bitIndex: number) {
2831
const q = bitIndex >> 5;
2932
const r = bitIndex & 0b11111;
33+
if (q >= bitSet.length) {
34+
throw new BitSetOutOfBoundsError(bitIndex);
35+
}
3036
bitSet[q] &= ~(1 << r);
3137
}
3238

3339
export function checkBit(bitSet: BitSet, bitIndex: number): boolean {
3440
const q = bitIndex >> 5;
3541
const r = bitIndex & 0b11111;
42+
if (q >= bitSet.length) {
43+
throw new BitSetOutOfBoundsError(bitIndex);
44+
}
3645
return (bitSet[q] & (1 << r)) !== 0;
3746
}
47+
48+
export class BitSetOutOfBoundsError extends Error {
49+
override name = 'BitSetOutOfBoundsError';
50+
bitIndex: number;
51+
constructor(bitIndex: number) {
52+
super(`Bit index ${bitIndex} is out of bounds`);
53+
this.bitIndex = bitIndex;
54+
}
55+
}

0 commit comments

Comments
 (0)