From 3e19491b655e84ea7840fa5a6bd8afaa734729c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 2 Dec 2022 12:22:43 +0100 Subject: [PATCH 1/4] Change the marker searching to use the marker schema instead --- src/profile-logic/marker-data.js | 71 +++++++++-------------------- src/profile-logic/marker-schema.js | 38 ++++++++++++++- src/selectors/per-thread/markers.js | 2 + 3 files changed, 60 insertions(+), 51 deletions(-) diff --git a/src/profile-logic/marker-data.js b/src/profile-logic/marker-data.js index 02df5ad8c1..7887d3628a 100644 --- a/src/profile-logic/marker-data.js +++ b/src/profile-logic/marker-data.js @@ -13,7 +13,11 @@ import { INTERVAL_START, INTERVAL_END, } from 'firefox-profiler/app-logic/constants'; -import { getMarkerSchemaName } from './marker-schema'; +import { + getMarkerSchemaName, + getSchemaFromMarker, + markerPayloadMatchesSearch, +} from './marker-schema'; import type { Thread, @@ -115,85 +119,52 @@ export function deriveJankMarkers( export function getSearchFilteredMarkerIndexes( getMarker: (MarkerIndex) => Marker, markerIndexes: MarkerIndex[], + markerSchemaByName: MarkerSchemaByName, searchRegExp: RegExp | null, categoryList: CategoryList ): MarkerIndex[] { if (!searchRegExp) { return markerIndexes; } + // Need to assign it to a constant variable so Flow doesn't complain about + // passing it inside a function below. + const regExp = searchRegExp; const newMarkers: MarkerIndex[] = []; for (const markerIndex of markerIndexes) { - const { data, name, category } = getMarker(markerIndex); + const marker = getMarker(markerIndex); + const { data, name, category } = marker; // Reset regexp for each iteration. Otherwise state from previous // iterations can cause matches to fail if the search is global or // sticky. - searchRegExp.lastIndex = 0; + regExp.lastIndex = 0; if (categoryList[category] !== undefined) { const markerCategory = categoryList[category].name; - if (searchRegExp.test(markerCategory)) { + if (regExp.test(markerCategory)) { newMarkers.push(markerIndex); continue; } } - if (searchRegExp.test(name)) { + if (regExp.test(name)) { newMarkers.push(markerIndex); continue; } + if (data && typeof data === 'object') { - if (searchRegExp.test(data.type)) { + if (regExp.test(data.type)) { + // Check the type of the marker payload first. newMarkers.push(markerIndex); continue; } - if (data.type === 'FileIO') { - const { filename, operation, source, threadId } = data; - if ( - searchRegExp.test(filename || '') || - searchRegExp.test(operation) || - searchRegExp.test(source) || - (threadId !== undefined && searchRegExp.test(threadId.toString())) - ) { - newMarkers.push(markerIndex); - continue; - } - } else if (data.type === 'IPC') { - const { messageType, otherPid } = data; - if ( - searchRegExp.test(messageType) || - searchRegExp.test(otherPid.toString()) - ) { - newMarkers.push(markerIndex); - continue; - } - } else if (data.type === 'Log') { - const { name, module } = data; - - if (searchRegExp.test(name) || searchRegExp.test(module)) { - newMarkers.push(markerIndex); - continue; - } - } - - if ( - typeof data.eventType === 'string' && - searchRegExp.test(data.eventType) - ) { - // Match DOMevents data.eventType - newMarkers.push(markerIndex); - continue; - } - if (typeof data.name === 'string' && searchRegExp.test(data.name)) { - // Match UserTiming's name. - newMarkers.push(markerIndex); - continue; - } + // Now check the schema for the marker payload for searchable + const markerSchema = getSchemaFromMarker(markerSchemaByName, marker); if ( - typeof data.category === 'string' && - searchRegExp.test(data.category) + markerSchema && + markerPayloadMatchesSearch(markerSchema, marker, regExp) ) { newMarkers.push(markerIndex); continue; diff --git a/src/profile-logic/marker-schema.js b/src/profile-logic/marker-schema.js index 9235ee5d3a..23613714ca 100644 --- a/src/profile-logic/marker-schema.js +++ b/src/profile-logic/marker-schema.js @@ -56,10 +56,16 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [ chartLabel: '{marker.data.messageType}', display: ['marker-chart', 'marker-table', 'timeline-ipc'], data: [ - { key: 'messageType', label: 'Type', format: 'string' }, + { key: 'messageType', label: 'Type', format: 'string', searchable: true }, { key: 'sync', label: 'Sync', format: 'string' }, { key: 'sendThreadName', label: 'From', format: 'string' }, { key: 'recvThreadName', label: 'To', format: 'string' }, + { + key: 'otherPid', + label: 'Other Pid', + format: 'string', + searchable: true, + }, ], }, { @@ -564,3 +570,33 @@ export function formatMarkupFromMarkerSchema( throw new Error(`Unknown format type ${JSON.stringify((format: empty))}`); } } + +/** + * Takes a marker and a RegExp and checks if any of its `searchable` marker + * payload fields match the search regular expression. + */ +export function markerPayloadMatchesSearch( + markerSchema: MarkerSchema, + marker: Marker, + searchRegExp: RegExp +): boolean { + const { data } = marker; + if (!data) { + return false; + } + // Check if searchable fields match the search regular expression. + for (const payloadField of markerSchema.data) { + if (payloadField.searchable) { + const value = data[payloadField.key]; + if (value === undefined || value === null || value === '') { + continue; + } + + if (searchRegExp.test(value)) { + return true; + } + } + } + + return false; +} diff --git a/src/selectors/per-thread/markers.js b/src/selectors/per-thread/markers.js index 60b3e79d03..96b5760e20 100644 --- a/src/selectors/per-thread/markers.js +++ b/src/selectors/per-thread/markers.js @@ -286,6 +286,7 @@ export function getMarkerSelectorsPerThread( createSelector( getMarkerGetter, getCommittedRangeAndTabFilteredMarkerIndexes, + ProfileSelectors.getMarkerSchemaByName, UrlState.getMarkersSearchStringsAsRegExp, ProfileSelectors.getCategories, MarkerData.getSearchFilteredMarkerIndexes @@ -343,6 +344,7 @@ export function getMarkerSelectorsPerThread( createSelector( getMarkerGetter, getNetworkMarkerIndexes, + ProfileSelectors.getMarkerSchemaByName, UrlState.getNetworkSearchStringsAsRegExp, ProfileSelectors.getCategories, MarkerData.getSearchFilteredMarkerIndexes From 8ff237703e44715e0832798d34375bac29ec3afd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 2 Dec 2022 12:23:05 +0100 Subject: [PATCH 2/4] Add an upgrader step to properly support older profiles --- src/app-logic/constants.js | 4 +- src/profile-logic/gecko-profile-versioning.js | 57 +++++++++++++++++++ .../processed-profile-versioning.js | 44 ++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/app-logic/constants.js b/src/app-logic/constants.js index 4c9c8cac8e..8cd9b024ae 100644 --- a/src/app-logic/constants.js +++ b/src/app-logic/constants.js @@ -7,10 +7,10 @@ import type { MarkerPhase } from 'firefox-profiler/types'; // The current version of the Gecko profile format. -export const GECKO_PROFILE_VERSION = 25; +export const GECKO_PROFILE_VERSION = 26; // The current version of the "processed" profile format. -export const PROCESSED_PROFILE_VERSION = 43; +export const PROCESSED_PROFILE_VERSION = 44; // The following are the margin sizes for the left and right of the timeline. Independent // components need to share these values. diff --git a/src/profile-logic/gecko-profile-versioning.js b/src/profile-logic/gecko-profile-versioning.js index 664851eb65..c2a0f041aa 100644 --- a/src/profile-logic/gecko-profile-versioning.js +++ b/src/profile-logic/gecko-profile-versioning.js @@ -1365,5 +1365,62 @@ const _upgraders = { // capturing this data and no new mandatory values are present in this // version. }, + [26]: (profile) => { + // `searchable` property in the marker schema wasn't implemented before and + // we had some manual checks for the marker fields below. With this version, + // we removed this manual check and started to use the `searchable` property + // of the marker schema. + function convertToVersion26Recursive(p) { + for (const schema of p.meta.markerSchema) { + let searchableFieldKeys; + switch (schema.name) { + case 'FileIO': { + searchableFieldKeys = [ + 'filename', + 'operation', + 'source', + 'threadId', + ]; + break; + } + case 'Log': { + searchableFieldKeys = ['name', 'module']; + break; + } + case 'DOMEvent': { + // In the earlier versions of Firefox, DOMEvent doesn't include + // eventType in the backend. + schema.data.push({ + key: 'eventType', + label: 'Event Type', + format: 'string', + searchable: true, + }); + // 'target' wasn't included in our code before. But I thought this + // would be a useful addition. + searchableFieldKeys = ['target']; + break; + } + default: { + searchableFieldKeys = ['name', 'category']; + break; + } + } + + const searchableFields = schema.data.filter((field) => + searchableFieldKeys.includes(field.key) + ); + for (const field of searchableFields) { + field.searchable = true; + } + } + + for (const subprocessProfile of p.processes) { + convertToVersion26Recursive(subprocessProfile); + } + } + + convertToVersion26Recursive(profile); + }, }; /* eslint-enable no-useless-computed-key */ diff --git a/src/profile-logic/processed-profile-versioning.js b/src/profile-logic/processed-profile-versioning.js index c69372c9ad..6d09f282da 100644 --- a/src/profile-logic/processed-profile-versioning.js +++ b/src/profile-logic/processed-profile-versioning.js @@ -2141,5 +2141,49 @@ const _upgraders = { [43]: (_) => { // The number property in counters is now optional. }, + [44]: (profile) => { + // `searchable` property in the marker schema wasn't implemented before and + // we had some manual checks for the marker fields below. With this version, + // we removed this manual check and started to use the `searchable` property + // of the marker schema. + for (const schema of profile.meta.markerSchema) { + let searchableFieldKeys; + switch (schema.name) { + case 'FileIO': { + searchableFieldKeys = ['filename', 'operation', 'source', 'threadId']; + break; + } + case 'Log': { + searchableFieldKeys = ['name', 'module']; + break; + } + case 'DOMEvent': { + // In the earlier versions of Firefox, DOMEvent doesn't include + // eventType in the backend. + schema.data.push({ + key: 'eventType', + label: 'Event Type', + format: 'string', + searchable: true, + }); + // 'target' wasn't included in our code before. But I thought this + // would be a useful addition. + searchableFieldKeys = ['target']; + break; + } + default: { + searchableFieldKeys = ['name', 'category']; + break; + } + } + + const searchableFields = schema.data.filter((field) => + searchableFieldKeys.includes(field.key) + ); + for (const field of searchableFields) { + field.searchable = true; + } + } + }, }; /* eslint-enable no-useless-computed-key */ From 802f8f28f743458a1e171cc44ababd1ad3a59855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Fri, 2 Dec 2022 12:27:56 +0100 Subject: [PATCH 3/4] Update the tests and snapshots --- .../__snapshots__/MarkerChart.test.js.snap | 14 ++++ .../__snapshots__/MarkerSidebar.test.js.snap | 7 ++ .../__snapshots__/StackChart.test.js.snap | 7 ++ .../__snapshots__/TooltipMarker.test.js.snap | 56 +++++++++++++ src/test/fixtures/profiles/marker-schema.js | 40 +++++++--- .../__snapshots__/profile-view.test.js.snap | 24 +++++- src/test/store/markers.test.js | 2 + .../profile-conversion.test.js.snap | 66 +++++++++++----- .../profile-upgrading.test.js.snap | 79 +++++++++++++++++-- 9 files changed, 256 insertions(+), 39 deletions(-) diff --git a/src/test/components/__snapshots__/MarkerChart.test.js.snap b/src/test/components/__snapshots__/MarkerChart.test.js.snap index 8d076e9cac..9eb68aa44e 100644 --- a/src/test/components/__snapshots__/MarkerChart.test.js.snap +++ b/src/test/components/__snapshots__/MarkerChart.test.js.snap @@ -300,6 +300,13 @@ exports[`MarkerChart renders the hoveredItem markers properly 2`] = `
+
+ Name + : +
+ Marker B
@@ -1238,6 +1245,13 @@ exports[`MarkerChart with active tab renders the hovered marker properly 2`] = ` :
7ms +
+ Event Type + : +
+ click
diff --git a/src/test/components/__snapshots__/MarkerSidebar.test.js.snap b/src/test/components/__snapshots__/MarkerSidebar.test.js.snap index fba10ddefa..756ae71e38 100644 --- a/src/test/components/__snapshots__/MarkerSidebar.test.js.snap +++ b/src/test/components/__snapshots__/MarkerSidebar.test.js.snap @@ -52,6 +52,13 @@ exports[`MarkerSidebar matches the snapshots when displaying data about the curr :
Empty (Thread ID: 0) +
+ Other Pid + : +
+ 2222
diff --git a/src/test/components/__snapshots__/StackChart.test.js.snap b/src/test/components/__snapshots__/StackChart.test.js.snap index 77e42d0292..f9d94abb57 100644 --- a/src/test/components/__snapshots__/StackChart.test.js.snap +++ b/src/test/components/__snapshots__/StackChart.test.js.snap @@ -889,6 +889,13 @@ exports[`MarkerChart shows a tooltip when hovering 1`] = `
+
+ Name + : +
+ componentB
diff --git a/src/test/components/__snapshots__/TooltipMarker.test.js.snap b/src/test/components/__snapshots__/TooltipMarker.test.js.snap index ad8bb30d28..4fa09b2247 100644 --- a/src/test/components/__snapshots__/TooltipMarker.test.js.snap +++ b/src/test/components/__snapshots__/TooltipMarker.test.js.snap @@ -2053,6 +2053,13 @@ exports[`TooltipMarker renders tooltips for various markers: DOMEvent-10.5 1`] =
+
+ Event Type + : +
+ commandupdate
@@ -2110,6 +2117,13 @@ exports[`TooltipMarker renders tooltips for various markers: DOMEvent-10.6 1`] =
+
+ Event Type + : +
+ load
@@ -2165,6 +2179,13 @@ exports[`TooltipMarker renders tooltips for various markers: DOMEvent-10.7 1`] =
+
+ Event Type + : +
+ load
@@ -2220,6 +2241,13 @@ exports[`TooltipMarker renders tooltips for various markers: DOMEvent-10.8 1`] =
+
+ Event Type + : +
+ load
@@ -3295,6 +3323,13 @@ exports[`TooltipMarker renders tooltips for various markers: IPCOut-120 1`] = ` :
Main Thread (Thread ID: 0) +
+ Other Pid + : +
+ 2222
@@ -4646,6 +4681,13 @@ exports[`TooltipMarker renders tooltips for various markers: UserTiming-12.5 1`]
+
+ Name + : +
+ foobar
@@ -4831,6 +4873,13 @@ exports[`TooltipMarker shows the source thread for markers from a merged thread
+
+ Event Type + : +
+ click
@@ -4892,6 +4941,13 @@ exports[`TooltipMarker shows the source thread for markers from a merged thread
+
+ Event Type + : +
+ pageload
diff --git a/src/test/fixtures/profiles/marker-schema.js b/src/test/fixtures/profiles/marker-schema.js index 7ed210aaa4..baf4f3f9f2 100644 --- a/src/test/fixtures/profiles/marker-schema.js +++ b/src/test/fixtures/profiles/marker-schema.js @@ -86,7 +86,7 @@ export const markerSchemaForTests: MarkerSchema[] = [ name: 'PreferenceRead', display: ['marker-chart', 'marker-table'], data: [ - { key: 'prefName', label: 'Name', format: 'string' }, + { key: 'prefName', label: 'Name', format: 'string', searchable: true }, { key: 'prefKind', label: 'Kind', format: 'string' }, { key: 'prefType', label: 'Type', format: 'string' }, { key: 'prefValue', label: 'Value', format: 'string' }, @@ -99,7 +99,7 @@ export const markerSchemaForTests: MarkerSchema[] = [ tableLabel: '{marker.data.name}', display: ['marker-chart', 'marker-table'], data: [ - // name + { key: 'name', label: 'Name', format: 'string', searchable: true }, { label: 'Marker', value: 'UserTiming' }, { key: 'entryType', label: 'Entry Type', format: 'string' }, { @@ -114,15 +114,17 @@ export const markerSchemaForTests: MarkerSchema[] = [ tableLabel: '{marker.name} — {marker.data.name}', chartLabel: '{marker.name} — {marker.data.name}', display: ['marker-chart', 'marker-table'], - data: [{ key: 'name', label: 'Details', format: 'string' }], + data: [ + { key: 'name', label: 'Details', format: 'string', searchable: true }, + ], }, { name: 'Log', display: ['marker-table'], tableLabel: '({marker.data.module}) {marker.data.name}', data: [ - { key: 'module', label: 'Module', format: 'string' }, - { key: 'name', label: 'Name', format: 'string' }, + { key: 'module', label: 'Module', format: 'string', searchable: true }, + { key: 'name', label: 'Name', format: 'string', searchable: true }, ], }, { @@ -133,7 +135,12 @@ export const markerSchemaForTests: MarkerSchema[] = [ display: ['marker-chart', 'marker-table', 'timeline-overview'], data: [ { key: 'latency', label: 'Latency', format: 'duration' }, - // eventType is in the payload as well. + { + key: 'eventType', + label: 'Event Type', + format: 'string', + searchable: true, + }, ], }, { @@ -141,21 +148,27 @@ export const markerSchemaForTests: MarkerSchema[] = [ // See issue #2749 name: 'Paint', display: ['marker-chart', 'marker-table', 'timeline-overview'], - data: [{ key: 'category', label: 'Type', format: 'string' }], + data: [ + { key: 'category', label: 'Type', format: 'string', searchable: true }, + ], }, { // TODO - Note that this marker is a "tracing" marker currently. // See issue #2749 name: 'Navigation', display: ['marker-chart', 'marker-table', 'timeline-overview'], - data: [{ key: 'category', label: 'Type', format: 'string' }], + data: [ + { key: 'category', label: 'Type', format: 'string', searchable: true }, + ], }, { // TODO - Note that this marker is a "tracing" marker currently. // See issue #2749 name: 'Layout', display: ['marker-chart', 'marker-table', 'timeline-overview'], - data: [{ key: 'category', label: 'Type', format: 'string' }], + data: [ + { key: 'category', label: 'Type', format: 'string', searchable: true }, + ], }, { name: 'IPC', @@ -174,6 +187,13 @@ export const markerSchemaForTests: MarkerSchema[] = [ { name: 'RefreshDriverTick', display: ['marker-chart', 'marker-table', 'timeline-overview'], - data: [{ key: 'name', label: 'Tick Reasons', format: 'string' }], + data: [ + { + key: 'name', + label: 'Tick Reasons', + format: 'string', + searchable: true, + }, + ], }, ]; diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 2419dc9d7c..c4350c48b1 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -204,6 +204,7 @@ Object { "format": "string", "key": "prefName", "label": "Name", + "searchable": true, }, Object { "format": "string", @@ -230,6 +231,12 @@ Object { Object { "chartLabel": "{marker.data.name}", "data": Array [ + Object { + "format": "string", + "key": "name", + "label": "Name", + "searchable": true, + }, Object { "label": "Marker", "value": "UserTiming", @@ -259,6 +266,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -274,11 +282,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -295,6 +305,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -311,6 +327,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -326,6 +343,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -341,6 +359,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -389,6 +408,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -403,7 +423,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "sourceURL": "", @@ -411,7 +431,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 25, + "version": 26, }, "pages": Array [ Object { diff --git a/src/test/store/markers.test.js b/src/test/store/markers.test.js index e4f0adf5e4..3a82f6f67f 100644 --- a/src/test/store/markers.test.js +++ b/src/test/store/markers.test.js @@ -18,6 +18,7 @@ import { getNetworkMarkers, } from '../fixtures/profiles/processed-profile'; import { storeWithProfile } from '../fixtures/stores'; +import { markerSchemaForTests } from '../fixtures/profiles/marker-schema'; describe('selectors/getMarkerChartTimingAndBuckets', function () { function getMarkerChartTimingAndBuckets(testMarkers: TestDefinedMarkers) { @@ -723,6 +724,7 @@ describe('profile upgrading and markers', () => { physicalCPUs: 0, logicalCPUs: 0, symbolicated: true, + markerSchema: markerSchemaForTests, }, pages: [], threads: [ diff --git a/src/test/unit/__snapshots__/profile-conversion.test.js.snap b/src/test/unit/__snapshots__/profile-conversion.test.js.snap index 912ecbe17e..6600d5507d 100644 --- a/src/test/unit/__snapshots__/profile-conversion.test.js.snap +++ b/src/test/unit/__snapshots__/profile-conversion.test.js.snap @@ -299,6 +299,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -314,11 +315,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -335,6 +338,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -351,6 +360,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -366,6 +376,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -381,6 +392,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -429,6 +441,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -450,7 +463,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -460,7 +473,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -102757,6 +102770,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -102772,11 +102786,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -102793,6 +102809,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -102809,6 +102831,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -102824,6 +102847,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -102839,6 +102863,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -102887,6 +102912,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -102908,7 +102934,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -102918,7 +102944,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -390202,7 +390228,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -390210,7 +390236,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 25, + "version": 26, }, "pages": Array [], "threads": Array [ @@ -444116,7 +444142,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -444124,7 +444150,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 25, + "version": 26, }, "pages": Array [], "threads": Array [ @@ -447149,7 +447175,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -447157,7 +447183,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 25, + "version": 26, }, "pages": Array [], "threads": Array [ @@ -448395,7 +448421,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -448405,7 +448431,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -456138,7 +456164,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -456148,7 +456174,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -476162,7 +476188,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -476172,7 +476198,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -483910,7 +483936,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -483920,7 +483946,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -508324,7 +508350,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "target/debug/examples/work_log (dhat)", "sourceURL": "", @@ -508332,7 +508358,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 25, + "version": 26, }, "pages": Array [], "threads": Array [ diff --git a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap index cdaa1a580d..3e2c111302 100644 --- a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap +++ b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap @@ -40,7 +40,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -50,7 +50,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 25, + "version": 26, "visualMetrics": undefined, }, "pages": Array [], @@ -8018,6 +8018,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -8033,11 +8034,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -8054,6 +8057,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -8070,6 +8079,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -8085,6 +8095,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -8100,6 +8111,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -8148,6 +8160,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -8174,7 +8187,7 @@ Object { "stackwalk": 1, "startTime": 1460221352723.438, "toolkit": "cocoa", - "version": 25, + "version": 26, }, "pausedRanges": Array [], "processes": Array [ @@ -9463,6 +9476,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -9478,11 +9492,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -9499,6 +9515,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -9515,6 +9537,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -9530,6 +9553,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -9545,6 +9569,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -9593,6 +9618,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -9619,7 +9645,7 @@ Object { "stackwalk": 1, "startTime": 1460221352723.438, "toolkit": "cocoa", - "version": 25, + "version": 26, }, "pages": Array [ Object { @@ -10993,6 +11019,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -11008,11 +11035,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -11029,6 +11058,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -11045,6 +11080,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -11060,6 +11096,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -11075,6 +11112,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -11123,6 +11161,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -11143,7 +11182,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -12652,6 +12691,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -12667,11 +12707,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -12688,6 +12730,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -12704,6 +12752,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -12719,6 +12768,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -12734,6 +12784,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -12782,6 +12833,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -12802,7 +12854,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -14461,6 +14513,7 @@ Object { "format": "string", "key": "name", "label": "Details", + "searchable": true, }, ], "display": Array [ @@ -14476,11 +14529,13 @@ Object { "format": "string", "key": "module", "label": "Module", + "searchable": true, }, Object { "format": "string", "key": "name", "label": "Name", + "searchable": true, }, ], "display": Array [ @@ -14497,6 +14552,12 @@ Object { "key": "latency", "label": "Latency", }, + Object { + "format": "string", + "key": "eventType", + "label": "Event Type", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -14513,6 +14574,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -14528,6 +14590,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -14543,6 +14606,7 @@ Object { "format": "string", "key": "category", "label": "Type", + "searchable": true, }, ], "display": Array [ @@ -14591,6 +14655,7 @@ Object { "format": "string", "key": "name", "label": "Tick Reasons", + "searchable": true, }, ], "display": Array [ @@ -14611,7 +14676,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 43, + "preprocessedProfileVersion": 44, "processType": 0, "product": "Firefox", "stackwalk": 1, From 8de1e7b311f934cbeb58c9d7b8041d4086b2979d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Naz=C4=B1m=20Can=20Alt=C4=B1nova?= Date: Tue, 6 Dec 2022 13:37:26 +0100 Subject: [PATCH 4/4] Address the review comments --- src/profile-logic/gecko-profile-versioning.js | 29 +++++++++++------- src/profile-logic/marker-schema.js | 6 ++++ .../processed-profile-versioning.js | 24 +++++++++++---- .../profile-conversion.test.js.snap | 12 ++++++++ .../profile-upgrading.test.js.snap | 30 +++++++++++++++++++ 5 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/profile-logic/gecko-profile-versioning.js b/src/profile-logic/gecko-profile-versioning.js index c2a0f041aa..dcda455954 100644 --- a/src/profile-logic/gecko-profile-versioning.js +++ b/src/profile-logic/gecko-profile-versioning.js @@ -1375,12 +1375,14 @@ const _upgraders = { let searchableFieldKeys; switch (schema.name) { case 'FileIO': { - searchableFieldKeys = [ - 'filename', - 'operation', - 'source', - 'threadId', - ]; + // threadId wasn't in the schema before, so we need to add manually. + schema.data.push({ + key: 'threadId', + label: 'Thread ID', + format: 'string', + searchable: true, + }); + searchableFieldKeys = []; break; } case 'Log': { @@ -1401,17 +1403,22 @@ const _upgraders = { searchableFieldKeys = ['target']; break; } + case 'TraceEvent': { + // These weren't included in our code before. But I thought this + // would be a useful addition. + searchableFieldKeys = ['name1', 'name2', 'val1', 'val2']; + break; + } default: { searchableFieldKeys = ['name', 'category']; break; } } - const searchableFields = schema.data.filter((field) => - searchableFieldKeys.includes(field.key) - ); - for (const field of searchableFields) { - field.searchable = true; + for (const field of schema.data) { + if (searchableFieldKeys.includes(field.key)) { + field.searchable = true; + } } } diff --git a/src/profile-logic/marker-schema.js b/src/profile-logic/marker-schema.js index 23613714ca..f8c8d7c029 100644 --- a/src/profile-logic/marker-schema.js +++ b/src/profile-logic/marker-schema.js @@ -584,6 +584,12 @@ export function markerPayloadMatchesSearch( if (!data) { return false; } + + // Reset regexp for each marker. Otherwise state from previous + // usages can cause matches to fail if the search is global or + // sticky. + searchRegExp.lastIndex = 0; + // Check if searchable fields match the search regular expression. for (const payloadField of markerSchema.data) { if (payloadField.searchable) { diff --git a/src/profile-logic/processed-profile-versioning.js b/src/profile-logic/processed-profile-versioning.js index 6d09f282da..bfc5f0c397 100644 --- a/src/profile-logic/processed-profile-versioning.js +++ b/src/profile-logic/processed-profile-versioning.js @@ -2150,7 +2150,14 @@ const _upgraders = { let searchableFieldKeys; switch (schema.name) { case 'FileIO': { - searchableFieldKeys = ['filename', 'operation', 'source', 'threadId']; + // threadId wasn't in the schema before, so we need to add manually. + schema.data.push({ + key: 'threadId', + label: 'Thread ID', + format: 'string', + searchable: true, + }); + searchableFieldKeys = []; break; } case 'Log': { @@ -2171,17 +2178,22 @@ const _upgraders = { searchableFieldKeys = ['target']; break; } + case 'TraceEvent': { + // These weren't included in our code before. But I thought this + // would be a useful addition. + searchableFieldKeys = ['name1', 'name2', 'val1', 'val2']; + break; + } default: { searchableFieldKeys = ['name', 'category']; break; } } - const searchableFields = schema.data.filter((field) => - searchableFieldKeys.includes(field.key) - ); - for (const field of searchableFields) { - field.searchable = true; + for (const field of schema.data) { + if (searchableFieldKeys.includes(field.key)) { + field.searchable = true; + } } } }, diff --git a/src/test/unit/__snapshots__/profile-conversion.test.js.snap b/src/test/unit/__snapshots__/profile-conversion.test.js.snap index 6600d5507d..942d87f59a 100644 --- a/src/test/unit/__snapshots__/profile-conversion.test.js.snap +++ b/src/test/unit/__snapshots__/profile-conversion.test.js.snap @@ -177,6 +177,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -102648,6 +102654,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart", diff --git a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap index 3e2c111302..ab6a947ff3 100644 --- a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap +++ b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap @@ -7896,6 +7896,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -9354,6 +9360,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -10897,6 +10909,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -12569,6 +12587,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart", @@ -14391,6 +14415,12 @@ Object { "label": "Filename", "searchable": true, }, + Object { + "format": "string", + "key": "threadId", + "label": "Thread ID", + "searchable": true, + }, ], "display": Array [ "marker-chart",