diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index 918c5b23b4..7d35fa46c7 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -6,6 +6,12 @@ Note that this is not an exhaustive list. Processed profile format upgraders can ## Processed profile format +### Version 52 + +No format changes, but a front-end behavior change: The schema for a marker is now looked up purely based on its `data.type`. In the past there were some special cases when `data` was `null`, or when `data.type` was `tracing` or `Text`. These special cases have been removed. The new behavior is simpler and more predictable, and was probably what you expected anyway. + +This change came with a new version because we needed to upgrade old profiles from Firefox which were relying on the more complex behavior. + ### Version 51 Two new marker schema field format types have been added: `flow-id` and `terminating-flow-id`, with string index values (like `unique-string`). diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js index af83bc8053..84ee8b7521 100644 --- a/src/actions/receive-profile.js +++ b/src/actions/receive-profile.js @@ -38,7 +38,6 @@ import { getRelevantPagesForActiveTab, getSymbolServerUrl, getActiveTabID, - getMarkerSchemaByName, } from 'firefox-profiler/selectors'; import { getSelectedTab, @@ -321,11 +320,7 @@ export function finalizeFullProfileView( tabFilter, tabToThreadIndexesMap ); - const localTracksByPid = computeLocalTracksByPid( - profile, - globalTracks, - getMarkerSchemaByName(getState()) - ); + const localTracksByPid = computeLocalTracksByPid(profile, globalTracks); const legacyThreadOrder = getLegacyThreadOrder(getState()); const globalTrackOrder = initializeGlobalTrackOrder( @@ -1835,11 +1830,7 @@ export function changeTabFilter(tabID: TabID | null): ThunkAction { tabID, tabToThreadIndexesMap ); - const localTracksByPid = computeLocalTracksByPid( - profile, - globalTracks, - getMarkerSchemaByName(getState()) - ); + const localTracksByPid = computeLocalTracksByPid(profile, globalTracks); const legacyThreadOrder = getLegacyThreadOrder(getState()); const globalTrackOrder = initializeGlobalTrackOrder( diff --git a/src/app-logic/constants.js b/src/app-logic/constants.js index f80c2fdad8..10b8dc0375 100644 --- a/src/app-logic/constants.js +++ b/src/app-logic/constants.js @@ -14,7 +14,7 @@ export const GECKO_PROFILE_VERSION = 31; // The current version of the "processed" profile format. // Please don't forget to update the processed profile format changelog in // `docs-developer/CHANGELOG-formats.md`. -export const PROCESSED_PROFILE_VERSION = 51; +export const PROCESSED_PROFILE_VERSION = 52; // 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/components/tooltip/Marker.js b/src/components/tooltip/Marker.js index 916f224037..df3797ae67 100644 --- a/src/components/tooltip/Marker.js +++ b/src/components/tooltip/Marker.js @@ -239,11 +239,7 @@ class MarkerTooltipContents extends React.PureComponent { if (data) { // Add the details for the markers based on their Marker schema. - const schema = getSchemaFromMarker( - markerSchemaByName, - marker.name, - marker.data - ); + const schema = getSchemaFromMarker(markerSchemaByName, marker.data); if (schema) { for (const schemaData of schema.data) { // Check for a schema that is looking up and formatting a value from diff --git a/src/profile-logic/gecko-profile-versioning.js b/src/profile-logic/gecko-profile-versioning.js index d7f86f6d0d..9520905b59 100644 --- a/src/profile-logic/gecko-profile-versioning.js +++ b/src/profile-logic/gecko-profile-versioning.js @@ -1252,27 +1252,27 @@ const _upgraders = { // eventType is in the payload as well. ], }, + + // The following three schemas should have just been a single schema named + // "tracing". They are kept here for historical accuracy. + // There is a processed profile format upgrader (version 52) which adds the + // "tracing" schema for profiles which don't have it. { - // TODO - Note that this marker is a "tracing" marker currently. - // See issue #2749 name: 'Paint', display: ['marker-chart', 'marker-table', 'timeline-overview'], data: [{ key: 'category', label: 'Type', format: 'string' }], }, { - // 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' }], }, { - // 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' }], }, + { name: 'IPC', tooltipLabel: 'IPC — {marker.data.niceDirection}', @@ -1288,6 +1288,14 @@ const _upgraders = { ], }, { + // An unused schema for RefreshDriverTick markers. + // This schema is not consistent with what post-schema Firefox would + // output. Firefox (as of Jan 2026) is still using Text markers and does + // not have a RefreshDriverTick schema. Furthermore, upgraded profiles + // which get this schema do not have any { type: 'RefreshDriverTick' } + // markers - in the past they picked up this schema due to a compat hack, + // but this hack is now removed. So this schema is unused. It is kept + // here for historical accuracy. name: 'RefreshDriverTick', display: ['marker-chart', 'marker-table', 'timeline-overview'], data: [{ key: 'name', label: 'Tick Reasons', format: 'string' }], diff --git a/src/profile-logic/marker-data.js b/src/profile-logic/marker-data.js index fa1aa55edf..98f8278181 100644 --- a/src/profile-logic/marker-data.js +++ b/src/profile-logic/marker-data.js @@ -14,7 +14,6 @@ import { INTERVAL_END, } from 'firefox-profiler/app-logic/constants'; import { - getMarkerSchemaName, getSchemaFromMarker, markerPayloadMatchesSearch, } from './marker-schema'; @@ -226,11 +225,7 @@ function positiveFilterMarker( } // Now check the schema for the marker payload for searchable - const markerSchema = getSchemaFromMarker( - markerSchemaByName, - marker.name, - marker.data - ); + const markerSchema = getSchemaFromMarker(markerSchemaByName, marker.data); if ( markerSchema && markerPayloadMatchesSearch(markerSchema, marker, stringTable, test) @@ -292,11 +287,7 @@ function negativeFilterMarker( } // Now check the schema for the marker payload for searchable - const markerSchema = getSchemaFromMarker( - markerSchemaByName, - marker.name, - marker.data - ); + const markerSchema = getSchemaFromMarker(markerSchemaByName, marker.data); if ( markerSchema && @@ -1287,14 +1278,12 @@ export function getAllowMarkersWithNoSchema( const { data } = marker; if (!data) { - // Keep the marker if there is payload. + // Keep the marker if there is no payload. return true; } if (!markerSchemaByName[data.type]) { - // Keep the marker if there is no schema. Note that this function doesn't use - // the getMarkerSchemaName function, as that function attempts to find a - // marker schema name in a very permissive manner. In the marker chart + // Keep the marker if there is no schema. In the marker chart // and marker table, most likely we want to show everything. return true; } @@ -1528,9 +1517,8 @@ export function filterMarkerByDisplayLocation( return additionalResult; } - return markerTypes.has( - getMarkerSchemaName(markerSchemaByName, marker.name, marker.data) - ); + const schemaName = marker.data ? (marker.data.type ?? null) : null; + return schemaName !== null && markerTypes.has(schemaName); }); } diff --git a/src/profile-logic/marker-schema.js b/src/profile-logic/marker-schema.js index 91046ffbd6..18b17c2dec 100644 --- a/src/profile-logic/marker-schema.js +++ b/src/profile-logic/marker-schema.js @@ -82,56 +82,16 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [ }, ]; -/** - * For the most part, schema is matched up by the Payload's "type" field, - * but for practical purposes, there are a few other options, see the - * implementation of this function for details. - */ -export function getMarkerSchemaName( - markerSchemaByName: MarkerSchemaByName, - markerName: string, - markerData: MarkerPayload | null -): string { - if (!markerData) { - // Fall back to using the name if no payload exists. - return markerName; - } - - const { type } = markerData; - if (type === 'tracing' && markerData.category) { - // TODO - Tracing markers have a duplicate "category" field. - // See issue #2749 - - // Does a marker schema for the "category" exist? - return markerSchemaByName[markerData.category] === undefined - ? // If not, default back to tracing - 'tracing' - : // If so, use the category as the schema name. - markerData.category; - } - if (type === 'Text') { - // Text markers are a cheap and easy way to create markers with - // a category. Check for schema if it exists, if not, fallback to - // a Text type marker. - return markerSchemaByName[markerName] === undefined ? 'Text' : markerName; - } - return type; -} - /** * This function takes the intended marker schema for a marker field, and applies * the appropriate formatting function. */ export function getSchemaFromMarker( markerSchemaByName: MarkerSchemaByName, - markerName: string, markerData: MarkerPayload | null ): MarkerSchema | null { - return ( - markerSchemaByName[ - getMarkerSchemaName(markerSchemaByName, markerName, markerData) - ] || null - ); + const schemaName = markerData ? markerData.type : null; + return schemaName ? (markerSchemaByName[schemaName] ?? null) : null; } /** @@ -385,12 +345,8 @@ export function getLabelGetter( // No label exists, it will have to be generated for the first time. if (label === undefined) { const marker = getMarker(markerIndex); - const schemaName = getMarkerSchemaName( - markerSchemaByName, - marker.name, - marker.data - ); - const applyLabel = labelFns.get(schemaName); + const schemaName = marker.data ? marker.data.type : null; + const applyLabel = schemaName ? labelFns.get(schemaName) : null; label = applyLabel ? // A label function is available, apply it. diff --git a/src/profile-logic/processed-profile-versioning.js b/src/profile-logic/processed-profile-versioning.js index 6dc2eec8ca..c50b1067ca 100644 --- a/src/profile-logic/processed-profile-versioning.js +++ b/src/profile-logic/processed-profile-versioning.js @@ -1640,27 +1640,26 @@ const _upgraders = { // eventType is in the payload as well. ], }, + + // The following three schemas should have just been a single schema named + // "tracing". They are kept here for historical accuracy. + // The upgrader for version 52 adds the missing "tracing" schema. { - // TODO - Note that this marker is a "tracing" marker currently. - // See issue #2749 name: 'Paint', display: ['marker-chart', 'marker-table', 'timeline-overview'], data: [{ key: 'category', label: 'Type', format: 'string' }], }, { - // 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' }], }, { - // 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' }], }, + { name: 'IPC', tooltipLabel: 'IPC — {marker.data.niceDirection}', @@ -1676,6 +1675,14 @@ const _upgraders = { ], }, { + // An unused schema for RefreshDriverTick markers. + // This schema is not consistent with what post-schema Firefox would + // output. Firefox (as of Jan 2025) is still using Text markers and does + // not have a RefreshDriverTick schema. Furthermore, upgraded profiles + // which get this schema do not have any { type: 'RefreshDriverTick' } + // markers - in the past they picked up this schema due to a compat hack, + // but this hack is now removed. So this schema is unused. It is kept + // here for historical accuracy. name: 'RefreshDriverTick', display: ['marker-chart', 'marker-table', 'timeline-overview'], data: [{ key: 'name', label: 'Tick Reasons', format: 'string' }], @@ -2270,6 +2277,76 @@ const _upgraders = { // marker data with the new field types data, and no modification is needed in the // frontend to display older formats. }, + [52]: (profile) => { + // This version simplifies how markers are mapped to their schema. + // The schema is now purely determined by data.type. The marker's name is ignored. + // If a marker has a null data, then it has no schema. + // + // In earlier versions, there were special cases for mapping markers with type + // "tracing" and "Text", and for markers with a null data property. These + // special cases have been removed. + // + // The upgrader for version 52 makes it so that existing profiles appear the + // same with the simplified logic. Specifically: + // - Some old profiles have markers with data.type === 'tracing' but no schema + // with the name 'tracing'. To ensure that the tracing markers from these + // profile still show up in the 'timeline-overview' area, this upgrader adds + // a schema to such profiles. + // - Some old profiles have CC markers which only showed up in the memory track + // because of special treatment of 'tracing' markers - the markers would have + // data.type === 'tracing' and data.category === 'CC', and there would be a + // 'CC' schema with 'timeline-memory'. This upgrader moves these tracing CC + // markers to a new 'tracingCCFrom52Upgrader' schema. + // + // Profiles from modern versions of Firefox already include a 'tracing' schema. + // And they don't use tracing markers for CC markers. + + const schemaNames = new Set(profile.meta.markerSchema.map((s) => s.name)); + const kTracingCCSchemaName = 'tracingCCFrom52Upgrader'; + const shouldMigrateTracingCCMarkers = schemaNames.has('CC'); + let hasTracingMarkers = false; + let hasMigratedTracingCCMarkers = false; + for (const thread of profile.threads) { + const { markers } = thread; + for (let i = 0; i < markers.length; i++) { + const data = markers.data[i]; + if (data && data.type === 'tracing' && data.category) { + hasTracingMarkers = true; + if (shouldMigrateTracingCCMarkers && data.category === 'CC') { + data.type = kTracingCCSchemaName; + hasMigratedTracingCCMarkers = true; + if (data.interval) { + // Also delete the interval property. This is present on old + // profiles where marker phase information was represented in the + // payload, i.e. you'd have interval: "start" / "end" on the + // data object. + // Our kTracingCCSchemaName schema does not list the interval field, + // so we shouldn't have this field on the payload either. + delete data.interval; + } + } + } + } + } + if (hasTracingMarkers && !schemaNames.has('tracing')) { + // Make sure that tracing markers still show up in the timeline-overview area. + profile.meta.markerSchema.push({ + name: 'tracing', + display: ['marker-chart', 'marker-table', 'timeline-overview'], + data: [{ key: 'category', label: 'Type', format: 'string' }], + }); + } + + if (hasMigratedTracingCCMarkers) { + // Add the kTracingCCSchemaName schema for migrated tracing CC markers, to + // make sure that these markers still show up in the timeline-memory area. + profile.meta.markerSchema.push({ + name: kTracingCCSchemaName, + display: ['marker-chart', 'marker-table', 'timeline-memory'], + data: [{ key: 'category', label: 'Type', format: 'string' }], + }); + } + }, // If you add a new upgrader here, please document the change in // `docs-developer/CHANGELOG-formats.md`. }; diff --git a/src/profile-logic/sanitize.js b/src/profile-logic/sanitize.js index 2ffc6d35fb..2b364959d8 100644 --- a/src/profile-logic/sanitize.js +++ b/src/profile-logic/sanitize.js @@ -299,11 +299,8 @@ function sanitizeThreadPII( if (currentMarker && PIIToBeRemoved.shouldRemoveUrls) { // Use the schema to find some properties that need to be sanitized. - const markerNameIndex = markerTable.name[i]; - const markerName = thread.stringTable.getString(markerNameIndex); const markerSchema = getSchemaFromMarker( markerSchemaByName, - markerName, currentMarker ); if (markerSchema) { diff --git a/src/profile-logic/tracks.js b/src/profile-logic/tracks.js index 4b69c15577..9047854d0d 100644 --- a/src/profile-logic/tracks.js +++ b/src/profile-logic/tracks.js @@ -15,7 +15,6 @@ import type { Counter, Tid, TrackReference, - MarkerSchemaByName, TabID, } from 'firefox-profiler/types'; @@ -23,7 +22,6 @@ import { defaultThreadOrder, getFriendlyThreadName } from './profile-data'; import { intersectSets, subtractSets } from '../utils/set'; import { splitSearchString, stringsToRegExp } from '../utils/string'; import { ensureExists, assertExhaustiveCheck } from '../utils/flow'; -import { getMarkerSchemaName } from './marker-schema'; export type TracksWithOrder = {| +globalTracks: GlobalTrack[], @@ -258,8 +256,7 @@ export function initializeLocalTrackOrderByPid( */ export function computeLocalTracksByPid( profile: Profile, - availableGlobalTracks: GlobalTrack[], - markerSchemaByName: MarkerSchemaByName + availableGlobalTracks: GlobalTrack[] ): Map { const localTracksByPid = new Map(); @@ -323,12 +320,8 @@ export function computeLocalTracksByPid( for (let i = 0; i < markers.length; ++i) { const markerNameIndex = markers.name[i]; const markerData = markers.data[i]; - const markerSchemaName = getMarkerSchemaName( - markerSchemaByName, - thread.stringTable.getString(markerNameIndex), - markerData - ); - if (markerData && markerSchemaByName) { + const markerSchemaName = markerData ? markerData.type : null; + if (markerData && markerSchemaName) { const mapEntry = markerTracksBySchemaName.get(markerSchemaName); if (mapEntry && mapEntry.keys.every((k) => k in markerData)) { mapEntry.markerNames.add(markerNameIndex); diff --git a/src/selectors/per-thread/markers.js b/src/selectors/per-thread/markers.js index 393ee3f168..d64ab7e706 100644 --- a/src/selectors/per-thread/markers.js +++ b/src/selectors/per-thread/markers.js @@ -11,10 +11,7 @@ import * as MarkerData from '../../profile-logic/marker-data'; import * as MarkerTimingLogic from '../../profile-logic/marker-timing'; import * as ProfileSelectors from '../profile'; import { getRightClickedMarkerInfo } from '../right-clicked-marker'; -import { - getLabelGetter, - getMarkerSchemaName, -} from '../../profile-logic/marker-schema'; +import { getLabelGetter } from '../../profile-logic/marker-schema'; import { getInclusiveSampleIndexRangeForSelection } from '../../profile-logic/profile-data'; import type { BasicThreadSelectorsPerThread } from './thread'; @@ -682,11 +679,7 @@ export function getMarkerSelectorsPerThread( if ( data && marker.name === name && - getMarkerSchemaName( - ProfileSelectors.getMarkerSchemaByName, - marker.name, - data - ) === schemaName && + data.type === schemaName && keys.every((key) => key in data) ) { markerIndexes.push(index); diff --git a/src/test/components/TimelineMarkers.test.js b/src/test/components/TimelineMarkers.test.js index b275bf5775..12cd5aa21b 100644 --- a/src/test/components/TimelineMarkers.test.js +++ b/src/test/components/TimelineMarkers.test.js @@ -179,12 +179,12 @@ describe('TimelineMarkers', function () { window.devicePixelRatio = 1; const { container } = setupWithMarkers({ rangeStart: 0, rangeEnd: 15 }, [ - ['DOMEvent', 0, 10], - ['DOMEvent', 0, 10], - ['DOMEvent', 5, 15], - ['Paint', 2, 13], - ['Navigation', 2, 6], - ['Layout', 6, 8], + ['DOMEvent', 0, 10, { type: 'tracing', category: 'JS' }], + ['DOMEvent', 0, 10, { type: 'tracing', category: 'JS' }], + ['DOMEvent', 5, 15, { type: 'DOMEvent', latency: 3.7, eventType: 'msg' }], + ['Paint', 2, 13, { type: 'tracing', category: 'Paint' }], + ['Navigation', 2, 6, { type: 'tracing', category: 'Navigation' }], + ['Layout', 6, 8, { type: 'tracing', category: 'Layout' }], // These 2 will be ignored. ['CC', 0, 5], ['GCMajor', 5, 10], @@ -205,7 +205,7 @@ describe('TimelineMarkers', function () { { rangeStart: 0, rangeEnd: 15, component: TimelineMarkersMemory }, [ // The first one will be ignored. - ['DOMEvent', 0, 10], + ['DOMEvent', 0, 10, { type: 'tracing', category: 'JS' }], ['CC', 0, 5], ['GCMajor', 5, 10], ] @@ -253,7 +253,7 @@ describe('TimelineMarkers', function () { { rangeStart: 0, rangeEnd: 10 }, [ ['DOMEvent', 0, 3], - ['Navigation', 6, 10], + ['Load', 6, 10, { type: 'tracing', category: 'Navigation' }], ] ); @@ -300,7 +300,7 @@ describe('TimelineMarkers', function () { { rangeStart: 0, rangeEnd: 10 }, [ ['DOMEvent', 0, 3], - ['Navigation', 6, 10], + ['Load', 6, 10, { type: 'tracing', category: 'Navigation' }], ] ); @@ -315,7 +315,7 @@ describe('TimelineMarkers', function () { expect(getContextMenu()).toHaveClass('react-contextmenu--visible'); clickOnMenuItem('Copy description'); - expect(copy).toHaveBeenLastCalledWith('Navigation'); + expect(copy).toHaveBeenLastCalledWith('Load'); expect(getContextMenu()).not.toHaveClass('react-contextmenu--visible'); diff --git a/src/test/components/__snapshots__/MenuButtons.test.js.snap b/src/test/components/__snapshots__/MenuButtons.test.js.snap index 97eefd2356..0e79a8879f 100644 --- a/src/test/components/__snapshots__/MenuButtons.test.js.snap +++ b/src/test/components/__snapshots__/MenuButtons.test.js.snap @@ -2211,7 +2211,7 @@ exports[`app/MenuButtons matches the snapshot for the menu buttons and class="menuButtonsDownloadSize" > ( - 1.65 kB + 1.63 kB ) @@ -2329,7 +2329,7 @@ exports[`app/MenuButtons matches the snapshot for the opened panel for class="menuButtonsDownloadSize" > ( - 1.63 kB + 1.61 kB ) @@ -2442,7 +2442,7 @@ exports[`app/MenuButtons matches the snapshot for the opened panel for class="menuButtonsDownloadSize" > ( - 1.65 kB + 1.63 kB ) diff --git a/src/test/fixtures/profiles/marker-schema.js b/src/test/fixtures/profiles/marker-schema.js index 8a783bd024..4a1ddf7e23 100644 --- a/src/test/fixtures/profiles/marker-schema.js +++ b/src/test/fixtures/profiles/marker-schema.js @@ -144,26 +144,13 @@ export const markerSchemaForTests: MarkerSchema[] = [ ], }, { - // TODO - Note that this marker is a "tracing" marker currently. - // See issue #2749 - name: 'Paint', + name: 'tracing', display: ['marker-chart', 'marker-table', 'timeline-overview'], 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', 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: [ @@ -185,16 +172,9 @@ export const markerSchemaForTests: MarkerSchema[] = [ ], }, { - name: 'RefreshDriverTick', + name: 'VisibleInTimelineOverview', display: ['marker-chart', 'marker-table', 'timeline-overview'], - data: [ - { - key: 'name', - label: 'Tick Reasons', - format: 'string', - searchable: true, - }, - ], + data: [], }, { name: 'StringTesting', diff --git a/src/test/fixtures/profiles/processed-profile.js b/src/test/fixtures/profiles/processed-profile.js index c3e9de110c..fe96acb72a 100644 --- a/src/test/fixtures/profiles/processed-profile.js +++ b/src/test/fixtures/profiles/processed-profile.js @@ -62,17 +62,23 @@ type MarkerName = string; type MarkerTime = Milliseconds; // These markers can create an Instant or a complete Interval marker, depending -// on if an end time is passed in. The definition uses a union, becaus as far +// on if an end time is passed in. +// +// If the data field is left out (undefined), a default value { type: MarkerName } +// is used. If the data field is manually set to null, a null data is used. +// +// The definition uses a union, becaus as far // as I can tell, Flow doesn't support multiple arity tuples. export type TestDefinedMarkers = Array< - // Instant marker: + // Instant marker, payload defaulting to { type: MarkerName }: | [MarkerName, MarkerTime] - // No payload: + // Interval marker: | [ MarkerName, MarkerTime, // start time MarkerTime | null, // end time ] + // Marker with manual payload: | [ MarkerName, MarkerTime, // start time @@ -165,7 +171,8 @@ export function addMarkersToThreadWithCorrespondingSamples( const startTime = tuple[1]; // Flow doesn't support variadic tuple types. const maybeEndTime = (tuple: any)[2] || null; - const payload: MarkerPayload | null = (tuple: any)[3] || null; + const maybePayload: MarkerPayload | null | void = (tuple: any)[3]; + const payload = maybePayload === undefined ? { type: name } : maybePayload; markersTable.name.push(stringTable.indexForString(name)); if (maybeEndTime === null) { @@ -184,7 +191,7 @@ export function addMarkersToThreadWithCorrespondingSamples( markerSchemaForTests, stringTable ); - markersTable.data.push(payload); + markersTable.data.push((payload: any)); markersTable.category.push(0); markersTable.length++; }); diff --git a/src/test/integration/symbolicator-cli/__snapshots__/symbolicator-cli.test.js.snap b/src/test/integration/symbolicator-cli/__snapshots__/symbolicator-cli.test.js.snap index de1ddacb60..9f03640ed2 100644 --- a/src/test/integration/symbolicator-cli/__snapshots__/symbolicator-cli.test.js.snap +++ b/src/test/integration/symbolicator-cli/__snapshots__/symbolicator-cli.test.js.snap @@ -88,7 +88,7 @@ Object { "markerSchema": Array [], "oscpu": "macOS 14.6.1", "pausedRanges": Array [], - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "a.out", "sampleUnits": Object { diff --git a/src/test/store/__snapshots__/markers.test.js.snap b/src/test/store/__snapshots__/markers.test.js.snap index 7ecb6b42d7..b66d251669 100644 --- a/src/test/store/__snapshots__/markers.test.js.snap +++ b/src/test/store/__snapshots__/markers.test.js.snap @@ -17,7 +17,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "TTI", + }, "end": null, "name": "TTI", "start": 6, @@ -49,7 +51,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "FirstContentfulPaint", + }, "end": 8, "name": "FirstContentfulPaint", "start": 7, diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 138d5480f8..0ce6dd13ba 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -335,23 +335,7 @@ Object { "marker-table", "timeline-overview", ], - "name": "Paint", - }, - Object { - "data": Array [ - Object { - "format": "string", - "key": "category", - "label": "Type", - "searchable": true, - }, - ], - "display": Array [ - "marker-chart", - "marker-table", - "timeline-overview", - ], - "name": "Navigation", + "name": "tracing", }, Object { "data": Array [ @@ -403,20 +387,13 @@ Object { "tooltipLabel": "IPC — {marker.data.niceDirection}", }, Object { - "data": Array [ - Object { - "format": "string", - "key": "name", - "label": "Tick Reasons", - "searchable": true, - }, - ], + "data": Array [], "display": Array [ "marker-chart", "marker-table", "timeline-overview", ], - "name": "RefreshDriverTick", + "name": "VisibleInTimelineOverview", }, Object { "data": Array [ @@ -457,7 +434,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "sourceURL": "", @@ -996,12 +973,24 @@ Object { 0, ], "data": Array [ - null, - null, - null, - null, - null, - null, + Object { + "type": "A", + }, + Object { + "type": "B", + }, + Object { + "type": "C", + }, + Object { + "type": "D", + }, + Object { + "type": "E", + }, + Object { + "type": "F", + }, Object { "URI": "https://mozilla.org", "endTime": 6.5, @@ -1678,12 +1667,24 @@ Array [ 0, ], "data": Array [ - null, - null, - null, - null, - null, - null, + Object { + "type": "A", + }, + Object { + "type": "B", + }, + Object { + "type": "C", + }, + Object { + "type": "D", + }, + Object { + "type": "E", + }, + Object { + "type": "F", + }, Object { "URI": "https://mozilla.org", "endTime": 6.5, @@ -1841,7 +1842,9 @@ exports[`snapshots of selectors/profile matches the last stored run of markerThr Array [ Object { "category": 0, - "data": null, + "data": Object { + "type": "D", + }, "end": null, "name": "D", "start": 3, @@ -1849,7 +1852,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "E", + }, "end": null, "name": "E", "start": 4, @@ -1857,7 +1862,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "F", + }, "end": null, "name": "F", "start": 5, @@ -1908,7 +1915,9 @@ exports[`snapshots of selectors/profile matches the last stored run of markerThr Array [ Object { "category": 0, - "data": null, + "data": Object { + "type": "A", + }, "end": null, "name": "A", "start": 0, @@ -1916,7 +1925,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "B", + }, "end": null, "name": "B", "start": 1, @@ -1924,7 +1935,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "C", + }, "end": null, "name": "C", "start": 2, @@ -1932,7 +1945,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "D", + }, "end": null, "name": "D", "start": 3, @@ -1940,7 +1955,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "E", + }, "end": null, "name": "E", "start": 4, @@ -1948,7 +1965,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "F", + }, "end": null, "name": "F", "start": 5, @@ -2068,7 +2087,9 @@ exports[`snapshots of selectors/profile matches the last stored run of markerThr Array [ Object { "category": 0, - "data": null, + "data": Object { + "type": "D", + }, "end": null, "name": "D", "start": 3, @@ -2076,7 +2097,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "E", + }, "end": null, "name": "E", "start": 4, @@ -2084,7 +2107,9 @@ Array [ }, Object { "category": 0, - "data": null, + "data": Object { + "type": "F", + }, "end": null, "name": "F", "start": 5, diff --git a/src/test/store/markers.test.js b/src/test/store/markers.test.js index 61450e3768..b14784ee37 100644 --- a/src/test/store/markers.test.js +++ b/src/test/store/markers.test.js @@ -2,10 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // @flow -import { - selectedThreadSelectors, - getMarkerSchemaByName, -} from 'firefox-profiler/selectors'; +import { selectedThreadSelectors } from 'firefox-profiler/selectors'; import { changeTimelineTrackOrganization } from 'firefox-profiler/actions/receive-profile'; import { unserializeProfileOfArbitraryFormat } from 'firefox-profiler/profile-logic/process-profile'; import { ensureExists } from 'firefox-profiler/utils/flow'; @@ -227,10 +224,10 @@ describe('memory markers', function () { return storeWithProfile( getProfileWithMarkers([ - ['DOMEvent', 0, null], - ['Navigation', 1, null], - ['Paint', 2, null], - ['IdleForgetSkippable', 3, 4, { type: 'tracing', category: 'CC' }], + ['DOMEvent', 0, null, { type: 'tracing', category: 'JS' }], + ['Navigation', 1, null, { type: 'tracing', category: 'Navigation' }], + ['Paint', 2, null, { type: 'tracing', category: 'Paint' }], + ['IdleForgetSkippable', 3, 4, { type: 'CC' }], ['GCMinor', 5, null, { type: 'GCMinor', nursery: any }], ['GCMajor', 6, null, { type: 'GCMajor', timings: any }], ['GCSlice', 7, null, { type: 'GCSlice', timings: any }], @@ -419,7 +416,8 @@ describe('Marker schema filtering', function () { const profile = getProfileWithMarkers([ ['no payload', 0, null, null], ['payload no schema', 0, null, { type: 'no schema marker' }], - ['RefreshDriverTick', 0, null, { type: 'Text', name: 'RefreshDriverTick' }], + ['RefreshDriverTick', 0, null, { type: 'Text', name: 'Tick with 1 observer' }], + ['VisibleInTimelineOverview', 0, null], ['UserTiming', 5, 6, { type: 'UserTiming', name: 'name', entryType: 'mark' }], // The following is a tracing marker without a schema attached, this was a // regression reported in Bug 1678698. @@ -432,15 +430,6 @@ describe('Marker schema filtering', function () { function setup(profile) { const { getState } = storeWithProfile(profile); const getMarker = selectedThreadSelectors.getMarkerGetter(getState()); - const markerSchemaByName = getMarkerSchemaByName(getState()); - - if (markerSchemaByName.RandomTracingMarker) { - throw new Error( - 'This test assumes that the RandomTracingMarker marker has no schema. If this ' + - 'schema were added somewhere else, then rename RandomTracingMarker to ' + - 'something else. ' - ); - } function getMarkerNames(selector): string[] { return selector(getState()) @@ -459,6 +448,7 @@ describe('Marker schema filtering', function () { 'no payload', 'payload no schema', 'RefreshDriverTick', + 'VisibleInTimelineOverview', 'Load 0: https://mozilla.org', 'UserTiming', 'RandomTracingMarker', @@ -473,6 +463,7 @@ describe('Marker schema filtering', function () { 'no payload', 'payload no schema', 'RefreshDriverTick', + 'VisibleInTimelineOverview', 'Load 0: https://mozilla.org', 'UserTiming', 'RandomTracingMarker', @@ -664,7 +655,7 @@ describe('Marker schema filtering', function () { const { getMarkerNames } = setup(getProfileForMarkerSchema()); expect( getMarkerNames(selectedThreadSelectors.getTimelineOverviewMarkerIndexes) - ).toEqual(['RefreshDriverTick']); + ).toEqual(['VisibleInTimelineOverview', 'RandomTracingMarker']); }); }); diff --git a/src/test/unit/__snapshots__/profile-conversion.test.js.snap b/src/test/unit/__snapshots__/profile-conversion.test.js.snap index 59355ebd9d..6d0f6a837f 100644 --- a/src/test/unit/__snapshots__/profile-conversion.test.js.snap +++ b/src/test/unit/__snapshots__/profile-conversion.test.js.snap @@ -469,7 +469,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -100220,7 +100220,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -382162,7 +382162,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 119159778.026, @@ -430868,7 +430868,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 119159778.026, @@ -479574,7 +479574,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 66155012.423, @@ -482616,7 +482616,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -485734,7 +485734,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 355035987.653, @@ -489802,7 +489802,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 66155012.423, @@ -491002,7 +491002,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -498527,7 +498527,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -517940,7 +517940,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -525458,7 +525458,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -549052,7 +549052,7 @@ Object { "keepProfileThreadOrder": true, "markerSchema": Array [], "platform": "Android", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "com.example.sampleapplication", "sourceCodeIsNotOnSearchfox": true, @@ -640488,7 +640488,7 @@ Object { "keepProfileThreadOrder": true, "markerSchema": Array [], "platform": "Android", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "com.example.sampleapplication", "sourceCodeIsNotOnSearchfox": true, @@ -740767,7 +740767,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "target/debug/examples/work_log (dhat)", "sourceURL": "", diff --git a/src/test/unit/__snapshots__/profile-upgrading.test.js.snap b/src/test/unit/__snapshots__/profile-upgrading.test.js.snap index f45bdfb44e..badcd1800b 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": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -10967,11 +10967,26 @@ Object { ], "name": "Network", }, + Object { + "data": Array [ + Object { + "format": "string", + "key": "category", + "label": "Type", + }, + ], + "display": Array [ + "marker-chart", + "marker-table", + "timeline-overview", + ], + "name": "tracing", + }, ], "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -12621,11 +12636,26 @@ Object { ], "name": "Network", }, + Object { + "data": Array [ + Object { + "format": "string", + "key": "category", + "label": "Type", + }, + ], + "display": Array [ + "marker-chart", + "marker-table", + "timeline-overview", + ], + "name": "tracing", + }, ], "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -14418,11 +14448,26 @@ Object { ], "name": "Network", }, + Object { + "data": Array [ + Object { + "format": "string", + "key": "category", + "label": "Type", + }, + ], + "display": Array [ + "marker-chart", + "marker-table", + "timeline-overview", + ], + "name": "tracing", + }, ], "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 51, + "preprocessedProfileVersion": 52, "processType": 0, "product": "Firefox", "stackwalk": 1, diff --git a/src/types/profile.js b/src/types/profile.js index 03bfe096ab..719dadcd6f 100644 --- a/src/types/profile.js +++ b/src/types/profile.js @@ -227,18 +227,6 @@ export type NativeAllocationsTable = | UnbalancedNativeAllocationsTable | BalancedNativeAllocationsTable; -/** - * This is the base abstract class that marker payloads inherit from. This probably isn't - * used directly in profiler.firefox.com, but is provided here for mainly documentation - * purposes. - */ -export type ProfilerMarkerPayload = { - type: string, - startTime?: Milliseconds, - endTime?: Milliseconds, - stack?: Thread, -}; - /** * Markers represent arbitrary events that happen within the browser. They have a * name, time, and potentially a JSON data payload. These can come from all over the