Audit undeclared marker fields #5281
Comments
|
I identified the following classes of markers with undeclared fields:
There are also some fields with special handling which are respected on all marker types: Markers with custom handling in the front-endNetwork markers have a whole lot of fields that aren't mentioned in any schema: GCSlice, GCMinor, GCMajor markers have custom code for their tooltips. IPC markers are handled with custom code, too. Even modern versions of Firefox don't emit a schema for IPC markers: https://share.firefox.dev/4fAkzwQ CompositorScreenshot markers have no schema, and their Old markers which weren't cleaned during upgradingProfile with old Profile with (Profiles with Markers with fancy labels / tooltips
The The
Markers with marker graphsExample: https://share.firefox.dev/400DXgC Markers with hopes for new front-end capabilitiesUrl / ChannelMarker markers (from D200055) have a Unintentional omissionsThe The Imported Chrome tracesImported profile with Useless data objectsThe following markers get a special shout out because they have data objects which are fully useless, because they only have a
|
Isn't there already a custom front-end handling in the tooltip for the |
|
Indeed! I wasn't aware of this. profiler/src/components/tooltip/Marker.js Lines 191 to 195 in 6fa213a I will edit my comment to correct this. I also left out some other details which I will add. |
|
I've filled in the missing details. This investigation is complete. |
|
I used the following checker function (see #5274), applied to a local tree which already had some other format changes and upgraders: function _isKnownUndeclaredSchema(
schemaName: string,
_markerName: string
): boolean {
switch (schemaName) {
case 'Network':
case 'VsyncTimestamp':
case 'IPC':
case 'CompositorScreenshot':
case 'MainThreadLongTask':
case 'CONTENT_FULL_PAINT_TIME':
case 'CONTENT_FRAME_TIME':
case 'BHR-detected hang':
case 'CompleteTraceEvent':
return true;
default:
return false;
}
}
function _isKnownUndeclaredField(
schemaName: string,
fieldKey: string
): boolean {
if (fieldKey === 'docShellId') { // 17a65e573bf902ec4fd8f68b2098b56c046c70d7
return ['DOMEvent', 'Styles', 'tracing', 'Text'].includes(schemaName);
}
if (fieldKey === 'stack') { // 1y01189smkgcmh64p7x4r7j4qmtddhrge5f849g
return ['FileIO', 'Text'].includes(schemaName);
}
switch (schemaName) {
case 'tracing':
case 'tracingCCFrom52Upgrader':
return fieldKey === 'interval';
case 'Network':
return [
'startTime',
'endTime',
'secureConnectionStart',
'requestMethod',
'requestStart',
'responseStart',
'responseEnd',
'id',
'status',
'cache',
'pri',
'URI',
'RedirectURI',
'count',
'domainLookupStart',
'domainLookupEnd',
'connectStart',
'tcpConnectEnd',
'connectEnd',
'contentType',
'httpVersion', // missing schema?
'redirectType', // missing schema? 094cy56cgcc4q0602h768zy6zbrbzg7nn959bp8
'classOfService', // missing schema?
'isHttpToHttpsRedirect', // missing schema? 16x915xng64f9ac3heeykzp71af4am78rzf8vt0
'redirectId', // missing schema? 16x915xng64f9ac3heeykzp71af4am78rzf8vt0
'isPrivateBrowsing', // 66xtsa0cwv80zg3h3qxtgf5e7txaft80ew7crng
].includes(fieldKey);
case 'Url':
case 'ChannelMarker':
return fieldKey === 'channelId';
case 'GCMinor':
return fieldKey === 'nursery' || fieldKey === 'nurseryTimings';
case 'GCSlice':
case 'GCMajor':
return fieldKey === 'timings';
case 'CC':
return fieldKey === 'mFreedJSZones';
case 'ProcessingTime':
case 'ProcessEnergy':
case 'WakeUpCount':
return fieldKey === 'label';
case 'ThreadCpuUse':
return fieldKey === 'label';
case 'Timer':
return fieldKey === 'prefix';
case 'DOMEvent':
return (
fieldKey === 'category' ||
fieldKey === 'phase' ||
fieldKey === 'interval'
);
case 'WindowProc':
return fieldKey === 'messageLoop' || fieldKey === 'name';
case 'CPU':
return true;
case 'Phase':
return fieldKey === 'phase';
case 'ContextSwitch':
return fieldKey === 'name';
case 'DebugBreak':
return fieldKey === 'name';
case 'IPC':
return [
'startTime',
'endTime',
'otherPid',
'messageSeqno',
'side',
'direction',
'messageType', // missing schema? 04zbwayx368gzwhta58f8af15ne7vrd2d44g0vg
'phase', // missing schema?
'sync', // missing schema?
].includes(fieldKey);
case 'Styles':
return fieldKey === 'category';
case 'UserTiming':
return fieldKey === 'name' || fieldKey === 'startMark' || fieldKey === 'endMark';
case 'CPUSample':
return true;
case 'CompositorScreenshot':
return ['url', 'windowID', 'windowWidth', 'windowHeight'].includes(fieldKey); // 16x915xng64f9ac3heeykzp71af4am78rzf8vt0
case 'MainThreadLongTask':
return fieldKey === 'category';
default:
return false;
}
}
function checkProfileUndeclaredMarkerFields(
profile: any,
_hash: string
): Set<string> {
const { meta, threads } = profile;
const { markerSchema } = meta;
const markerSchemaByName = new Map(markerSchema.map((s) => [s.name, s]));
// const namesForEmptyDataWithoutType = new Set();
// const namesForNonEmptyDataWithoutType = new Set();
const namesForUnknownTypes = new Set();
const unknownTypes = new Set();
const undeclaredFieldKeys = new Set();
const namesForUndeclaredFields = new Set();
const outcomes = new Set();
if (profile.meta.importedFrom === 'Chrome Trace') {
outcomes.add('is Chrome trace');
return outcomes;
}
for (let threadIndex = 0; threadIndex < threads.length; threadIndex++) {
const thread = threads[threadIndex];
const { markers, stringTable } = thread;
for (let markerIndex = 0; markerIndex < markers.length; markerIndex++) {
const nameIndex = markers.name[markerIndex];
const data = markers.data[markerIndex];
const name = stringTable.getString(nameIndex);
if (!data) {
continue;
}
if (!data.type) {
// const dataIsEmpty = Object.keys(data).length === 0;
// if (dataIsEmpty && !namesForEmptyDataWithoutType.has(name)) {
// console.log(
// `Found marker with name "${name}" and empty but non-null data but no data.type, thread index ${threadIndex}, marker index ${markerIndex}`
// );
// namesForEmptyDataWithoutType.add(name);
// outcomes.add(
// 'has empty but non-null data'
// );
// }
// if (!dataIsEmpty && !namesForNonEmptyDataWithoutType.has(name)) {
// console.log(
// `Found marker with name "${name}" and non-null data but no data.type, data: ${JSON.stringify(data)}, thread index ${threadIndex}, marker index ${markerIndex}`
// );
// namesForNonEmptyDataWithoutType.add(name);
// outcomes.add(
// 'has non-empty data without data.type'
// );
// }
continue;
}
const schema = markerSchemaByName.get(data.type);
if (schema === undefined) {
if (
!_isKnownUndeclaredSchema(data.type, name) &&
(!namesForUnknownTypes.has(name) || !unknownTypes.has(data.type))
) {
console.log(
`Found marker with data.type "${data.type}" and name "${name}" for which there is no schema, thread index ${threadIndex}, marker index ${markerIndex}`
);
namesForUnknownTypes.add(name);
unknownTypes.add(data.type);
outcomes.add('unknown schema for data.type');
}
}
const schemaFields = schema !== undefined ? schema.fields : [];
for (const fieldKey in data) {
if (
fieldKey === 'type' ||
fieldKey === 'innerWindowID' ||
fieldKey === 'threadId' ||
fieldKey === 'cause'
) {
continue;
}
const fieldValue = data[fieldKey];
const fieldSchema = schemaFields.find((f) => f.key === fieldKey);
if (fieldSchema === undefined) {
if (
!_isKnownUndeclaredField(data.type, fieldKey) &&
(!namesForUndeclaredFields.has(name) ||
!undeclaredFieldKeys.has(fieldKey))
) {
console.log(
`Found marker with data.type "${data.type}" and name "${name}" with undeclared field "${fieldKey}": "${fieldValue}", thread index ${threadIndex}, marker index ${markerIndex}`
);
namesForUndeclaredFields.add(name);
undeclaredFieldKeys.add(fieldKey);
outcomes.add('unknown field');
}
}
}
}
}
return outcomes;
} |
Profiles with lots of markers are too big.
A lot of space in the stringified JSON is taken up by the field keys in each marker's
dataobject. Example:{ "type": "Task", "name": "Document::NotifyIntersectionObservers", "priority": 4, "priorityName": "Normal", "task": 3944 }It would be nice if we didn't have to repeat those string keys (
"name","priority"etc) for every marker.#4494 proposes moving the
"type": "Task"part outside the data object.Once that's done, we could change the data object into a data array:
["Document::NotifyIntersectionObservers", 4, "Normal", 3944]In order to know which value belongs to which field, we would check the marker schema.
However, this can only work if all fields are declared in the schema.
What are the cases in which markers have fields which are not declared in the schema? I'll use this issue to research and write down the answer to this question.
┆Issue is synchronized with this Jira Task
The text was updated successfully, but these errors were encountered: