Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Profile format changes for flows and stack-based markers #5186

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Profile format changes for flows and stack-based markers #5186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs-developer/CHANGELOG-formats.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Comment on lines +86 to +89
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note we also need to implement and bump the version on the gecko side fairly at the same time. Otherwise we might run into a situation where another change will want to bump the format version (eg paul bone's changes on the memory counters) and we wouldn't have the same version on the backend and frontend...

If all we want to do for now is experimenting, it might not be necessary to bump the gecko format version until we actually land it on the gecko side. Until then Jeff and yourself will have to remember you need to have an updated frontend.

(it's fine to bump the processed format version though).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put up a patch that bumps the Gecko version into https://bugzilla.mozilla.org/show_bug.cgi?id=1929913 . I think that's the cleanest way to approach it.


### 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.
Expand Down
4 changes: 2 additions & 2 deletions src/app-logic/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions src/profile-logic/gecko-profile-versioning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
8 changes: 7 additions & 1 deletion src/profile-logic/marker-schema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down Expand Up @@ -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 === '') {
Expand Down
9 changes: 9 additions & 0 deletions src/profile-logic/processed-profile-versioning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
};
Expand Down
4 changes: 4 additions & 0 deletions src/test/fixtures/profiles/processed-profile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[],
Expand Down Expand Up @@ -148,6 +151,7 @@ function _replaceUniqueStringFieldValuesWithStringIndexesInMarkerPayload(
}
}

// This is used in tests, with TestDefinedMarkers.
export function addMarkersToThreadWithCorrespondingSamples(
thread: Thread,
markers: TestDefinedMarkers
Expand Down
2 changes: 1 addition & 1 deletion src/test/integration/symbolicator-cli/symbolicated.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions src/test/store/__snapshots__/profile-view.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,15 +457,15 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Firefox",
"sourceURL": "",
"stackwalk": 0,
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [
Object {
Expand Down
48 changes: 24 additions & 24 deletions src/test/unit/__snapshots__/profile-conversion.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ Object {
"oscpu": undefined,
"physicalCPUs": undefined,
"platform": undefined,
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "ART Trace (Android)",
"sampleUnits": undefined,
Expand All @@ -479,7 +479,7 @@ Object {
"symbolicated": true,
"toolkit": undefined,
"updateChannel": undefined,
"version": 30,
"version": 31,
"visualMetrics": undefined,
},
"pages": Array [],
Expand Down Expand Up @@ -100220,7 +100220,7 @@ Object {
"oscpu": undefined,
"physicalCPUs": undefined,
"platform": undefined,
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "ART Trace (Android)",
"sampleUnits": undefined,
Expand All @@ -100230,7 +100230,7 @@ Object {
"symbolicated": true,
"toolkit": undefined,
"updateChannel": undefined,
"version": 30,
"version": 31,
"visualMetrics": undefined,
},
"pages": Array [],
Expand Down Expand Up @@ -382162,7 +382162,7 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Chrome Trace",
"profilingEndTime": 119159778.026,
Expand All @@ -382172,7 +382172,7 @@ Object {
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [],
"threads": Array [
Expand Down Expand Up @@ -430868,7 +430868,7 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Chrome Trace",
"profilingEndTime": 119159778.026,
Expand All @@ -430878,7 +430878,7 @@ Object {
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [],
"threads": Array [
Expand Down Expand Up @@ -479574,7 +479574,7 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Chrome Trace",
"profilingEndTime": 66155012.423,
Expand All @@ -479584,7 +479584,7 @@ Object {
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [],
"threads": Array [
Expand Down Expand Up @@ -482616,15 +482616,15 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Chrome Trace",
"sourceURL": "",
"stackwalk": 0,
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [],
"threads": Array [
Expand Down Expand Up @@ -485734,7 +485734,7 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Chrome Trace",
"profilingEndTime": 66155012.423,
Expand All @@ -485744,7 +485744,7 @@ Object {
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [],
"threads": Array [
Expand Down Expand Up @@ -486934,7 +486934,7 @@ Object {
"oscpu": undefined,
"physicalCPUs": undefined,
"platform": undefined,
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Firefox",
"sampleUnits": undefined,
Expand All @@ -486944,7 +486944,7 @@ Object {
"symbolicated": true,
"toolkit": undefined,
"updateChannel": undefined,
"version": 30,
"version": 31,
"visualMetrics": undefined,
},
"pages": Array [],
Expand Down Expand Up @@ -494459,7 +494459,7 @@ Object {
"oscpu": undefined,
"physicalCPUs": undefined,
"platform": undefined,
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Firefox",
"sampleUnits": undefined,
Expand All @@ -494469,7 +494469,7 @@ Object {
"symbolicated": true,
"toolkit": undefined,
"updateChannel": undefined,
"version": 30,
"version": 31,
"visualMetrics": undefined,
},
"pages": Array [],
Expand Down Expand Up @@ -513872,7 +513872,7 @@ Object {
"oscpu": undefined,
"physicalCPUs": undefined,
"platform": undefined,
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Firefox",
"sampleUnits": undefined,
Expand All @@ -513882,7 +513882,7 @@ Object {
"symbolicated": true,
"toolkit": undefined,
"updateChannel": undefined,
"version": 30,
"version": 31,
"visualMetrics": undefined,
},
"pages": Array [],
Expand Down Expand Up @@ -521390,7 +521390,7 @@ Object {
"oscpu": undefined,
"physicalCPUs": undefined,
"platform": undefined,
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "Firefox",
"sampleUnits": undefined,
Expand All @@ -521400,7 +521400,7 @@ Object {
"symbolicated": true,
"toolkit": undefined,
"updateChannel": undefined,
"version": 30,
"version": 31,
"visualMetrics": undefined,
},
"pages": Array [],
Expand Down Expand Up @@ -544990,15 +544990,15 @@ Object {
"oscpu": "",
"physicalCPUs": 0,
"platform": "",
"preprocessedProfileVersion": 50,
"preprocessedProfileVersion": 51,
"processType": 0,
"product": "target/debug/examples/work_log (dhat)",
"sourceURL": "",
"stackwalk": 0,
"startTime": 0,
"symbolicated": true,
"toolkit": "",
"version": 30,
"version": 31,
},
"pages": Array [],
"threads": Array [
Expand Down
Loading