From 8141498588b3acfddb40c1eea6697780987c6326 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Thu, 31 Oct 2024 20:17:10 +0100 Subject: [PATCH] Profile format changes for flows and stack-based markers This changeset changes the type definition for marker schema field types and for the marker schema itself, to include the new field types 'flow-id' and 'terminating-flow-id'. It also has some minimal changes to treat the new field types as if they were unique-string fields. Somewhat unrelated to flows, it also adds an optional `isStackBased` field to the marker schema type, which is currently unused. This is for https://github.com/firefox-devtools/profiler/issues/3141 . Additionally, it increments the version of the two profile formats, so that profiles with the new field types won't be loaded by older front-ends. --- docs-developer/CHANGELOG-formats.md | 10 ++++ src/app-logic/constants.js | 4 +- src/profile-logic/gecko-profile-versioning.js | 9 ++++ src/profile-logic/marker-schema.js | 8 +++- .../processed-profile-versioning.js | 9 ++++ .../fixtures/profiles/processed-profile.js | 4 ++ .../symbolicator-cli/symbolicated.json | 2 +- .../__snapshots__/profile-view.test.js.snap | 4 +- .../profile-conversion.test.js.snap | 48 +++++++++---------- .../profile-upgrading.test.js.snap | 14 +++--- src/types/markers.js | 22 +++++++++ 11 files changed, 97 insertions(+), 37 deletions(-) diff --git a/docs-developer/CHANGELOG-formats.md b/docs-developer/CHANGELOG-formats.md index 3c9173d7e5..918c5b23b4 100644 --- a/docs-developer/CHANGELOG-formats.md +++ b/docs-developer/CHANGELOG-formats.md @@ -6,6 +6,11 @@ Note that this is not an exhaustive list. Processed profile format upgraders can ## Processed profile format +### 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`). +An optional `isStackBased` boolean field has been added to the marker schema. + ### Version 50 The serialized format can now optionally store sample and counter sample times as time deltas instead of absolute timestamps to reduce the JSON size. The unserialized version is unchanged. @@ -78,6 +83,11 @@ Older versions are not documented in this changelog but can be found in [process ## Gecko profile format +### Version 31 + +Two new marker schema field format types have been added: `flow-id` and `terminating-flow-id`, with string index values (like `unique-string`). +An optional `isStackBased` boolean field has been added to the marker schema. + ### Version 30 A new `sanitized-string` marker schema format type has been added, allowing markers to carry arbitrary strings containing PII that will be sanitized along with URLs and FilePaths. diff --git a/src/app-logic/constants.js b/src/app-logic/constants.js index 18b460ee79..2b8c1fa2fe 100644 --- a/src/app-logic/constants.js +++ b/src/app-logic/constants.js @@ -9,12 +9,12 @@ import type { MarkerPhase } from 'firefox-profiler/types'; // The current version of the Gecko profile format. // Please don't forget to update the gecko profile format changelog in // `docs-developer/CHANGELOG-formats.md`. -export const GECKO_PROFILE_VERSION = 30; +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 = 50; +export const PROCESSED_PROFILE_VERSION = 51; // 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 a0df20f626..869aac1ebf 100644 --- a/src/profile-logic/gecko-profile-versioning.js +++ b/src/profile-logic/gecko-profile-versioning.js @@ -1480,6 +1480,15 @@ const _upgraders = { // marker data with sanitized-string typed data, and no modification is needed in the // frontend to display older formats. }, + [31]: (_) => { + // This version bump added two new form types for new marker schema field: + // "flow-id" and "terminating-flow-id". + // Older frontends will not be able to display these fields. + // Furthermore, the marker schema itself has an optional isStackBased field. + // No upgrade is needed, as older versions of firefox would not generate + // marker data with the new field types data, and no modification is needed in the + // frontend to display older formats. + }, // If you add a new upgrader here, please document the change in // `docs-developer/CHANGELOG-formats.md`. diff --git a/src/profile-logic/marker-schema.js b/src/profile-logic/marker-schema.js index 9e8cd20f91..2b40be481e 100644 --- a/src/profile-logic/marker-schema.js +++ b/src/profile-logic/marker-schema.js @@ -477,6 +477,8 @@ export function formatFromMarkerSchema( // Make sure a non-empty string is returned here. return String(value) || '(empty)'; case 'unique-string': + case 'flow-id': + case 'terminating-flow-id': return stringTable.getString(value, '(empty)'); case 'duration': case 'time': @@ -671,7 +673,11 @@ export function markerPayloadMatchesSearch( for (const payloadField of markerSchema.data) { if (payloadField.searchable) { let value = data[payloadField.key]; - if (payloadField.format === 'unique-string') { + if ( + payloadField.format === 'unique-string' || + payloadField.format === 'flow-id' || + payloadField.format === 'terminating-flow-id' + ) { value = stringTable.getString(value); } if (value === undefined || value === null || value === '') { diff --git a/src/profile-logic/processed-profile-versioning.js b/src/profile-logic/processed-profile-versioning.js index accd48a8bb..6c9c00b96d 100644 --- a/src/profile-logic/processed-profile-versioning.js +++ b/src/profile-logic/processed-profile-versioning.js @@ -2273,6 +2273,15 @@ const _upgraders = { // The unserialized version is unchanged, and because the upgraders run // after unserialization they see no difference. }, + [51]: (_) => { + // This version bump added two new form types for new marker schema field: + // "flow-id" and "terminating-flow-id". + // Older frontends will not be able to display these fields. + // Furthermore, the marker schema itself has an optional isStackBased field. + // No upgrade is needed, as older versions of firefox would not generate + // marker data with the new field types data, and no modification is needed in the + // frontend to display older formats. + }, // If you add a new upgrader here, please document the change in // `docs-developer/CHANGELOG-formats.md`. }; diff --git a/src/test/fixtures/profiles/processed-profile.js b/src/test/fixtures/profiles/processed-profile.js index 8d960adebe..8df17b287a 100644 --- a/src/test/fixtures/profiles/processed-profile.js +++ b/src/test/fixtures/profiles/processed-profile.js @@ -120,6 +120,9 @@ export function addRawMarkersToThread( } } +// This function is called with test-defined payloads. For convenience, we allow +// providing payload values as strings, and then this function makes it so that, +// for fields of type 'unique-string', the values become string indexes. function _replaceUniqueStringFieldValuesWithStringIndexesInMarkerPayload( payload: MixedObject | null, markerSchemas: MarkerSchema[], @@ -148,6 +151,7 @@ function _replaceUniqueStringFieldValuesWithStringIndexesInMarkerPayload( } } +// This is used in tests, with TestDefinedMarkers. export function addMarkersToThreadWithCorrespondingSamples( thread: Thread, markers: TestDefinedMarkers diff --git a/src/test/integration/symbolicator-cli/symbolicated.json b/src/test/integration/symbolicator-cli/symbolicated.json index e117c2f0ab..8db97e9b2a 100644 --- a/src/test/integration/symbolicator-cli/symbolicated.json +++ b/src/test/integration/symbolicator-cli/symbolicated.json @@ -7,7 +7,7 @@ "debug": false, "extensions": { "baseURL": [], "id": [], "length": 0, "name": [] }, "interval": 1, - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "a.out", "oscpu": "macOS 14.6.1", diff --git a/src/test/store/__snapshots__/profile-view.test.js.snap b/src/test/store/__snapshots__/profile-view.test.js.snap index 34be6786a1..c35d6f9ac9 100644 --- a/src/test/store/__snapshots__/profile-view.test.js.snap +++ b/src/test/store/__snapshots__/profile-view.test.js.snap @@ -457,7 +457,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "sourceURL": "", @@ -465,7 +465,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "pages": Array [ Object { diff --git a/src/test/unit/__snapshots__/profile-conversion.test.js.snap b/src/test/unit/__snapshots__/profile-conversion.test.js.snap index c5543fa99f..6c4a1b4c63 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": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -479,7 +479,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -100220,7 +100220,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "ART Trace (Android)", "sampleUnits": undefined, @@ -100230,7 +100230,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -382162,7 +382162,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 119159778.026, @@ -382172,7 +382172,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "pages": Array [], "threads": Array [ @@ -430868,7 +430868,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 119159778.026, @@ -430878,7 +430878,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "pages": Array [], "threads": Array [ @@ -479574,7 +479574,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 66155012.423, @@ -479584,7 +479584,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "pages": Array [], "threads": Array [ @@ -482616,7 +482616,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Chrome Trace", "sourceURL": "", @@ -482624,7 +482624,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "pages": Array [], "threads": Array [ @@ -485734,7 +485734,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Chrome Trace", "profilingEndTime": 66155012.423, @@ -485744,7 +485744,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "pages": Array [], "threads": Array [ @@ -486934,7 +486934,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -486944,7 +486944,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -494459,7 +494459,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -494469,7 +494469,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -513872,7 +513872,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -513882,7 +513882,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -521390,7 +521390,7 @@ Object { "oscpu": undefined, "physicalCPUs": undefined, "platform": undefined, - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -521400,7 +521400,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -544990,7 +544990,7 @@ Object { "oscpu": "", "physicalCPUs": 0, "platform": "", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "target/debug/examples/work_log (dhat)", "sourceURL": "", @@ -544998,7 +544998,7 @@ Object { "startTime": 0, "symbolicated": true, "toolkit": "", - "version": 30, + "version": 31, }, "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 13d004660b..338bc2f303 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": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "sampleUnits": undefined, @@ -50,7 +50,7 @@ Object { "symbolicated": true, "toolkit": undefined, "updateChannel": undefined, - "version": 30, + "version": 31, "visualMetrics": undefined, }, "pages": Array [], @@ -7975,7 +7975,7 @@ Object { "stackwalk": 1, "startTime": 1460221352723.438, "toolkit": "cocoa", - "version": 30, + "version": 31, }, "pausedRanges": Array [], "processes": Array [ @@ -9431,7 +9431,7 @@ Object { "stackwalk": 1, "startTime": 1460221352723.438, "toolkit": "cocoa", - "version": 30, + "version": 31, }, "pages": Array [ Object { @@ -10971,7 +10971,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -12625,7 +12625,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "stackwalk": 1, @@ -14422,7 +14422,7 @@ Object { "misc": "rv:48.0", "oscpu": "Intel Mac OS X 10.11", "platform": "Macintosh", - "preprocessedProfileVersion": 50, + "preprocessedProfileVersion": 51, "processType": 0, "product": "Firefox", "stackwalk": 1, diff --git a/src/types/markers.js b/src/types/markers.js index dbba01fd08..db36d9c0b4 100644 --- a/src/types/markers.js +++ b/src/types/markers.js @@ -34,6 +34,17 @@ export type MarkerFormatType = /// This is effectively an integer, so wherever we need to display this value, we /// must first perform a lookup into the appropriate string table. | 'unique-string' + + // ---------------------------------------------------- + // Flow types. + // A flow ID is a u64 identifier that's unique across processes. In the current + // implementation, we represent them as hex strings, as string table indexes. + // A terminating flow ID is a flow ID that, when used in a marker with timestamp T, + // makes it so that if the same flow ID is used in a marker whose timestamp is + // after T, that flow ID is considered to refer to a different flow. + | 'flow-id' + | 'terminating-flow-id' + // ---------------------------------------------------- // Numeric types @@ -144,6 +155,17 @@ export type MarkerSchema = {| // if present, give the marker its own local track graphs?: Array, + + // If set to true, markers of this type are assumed to be well-nested with all + // other stack-based markers on the same thread. Stack-based markers may + // be displayed in a different part of the marker chart than non-stack-based + // markers. + // Instant markers are always well-nested. + // For interval markers, or for intervals defined by a start and an end marker, + // well-nested means that, for all marker-defined timestamp intervals A and B, + // A either fully encompasses B or is fully encompassed by B - there is no + // partial overlap. + isStackBased?: boolean, |}; export type MarkerSchemaByName = ObjectMap;