Partition based on samples instead of traceEvents when importing a sample-based chrome trace (#460)

In the existing code, if `traceEvents` did not have any importable events, the profile would be marked as empty. This was a bug, as the dev Hermes profiles I was testing with had one X event which made the code work. However we do not need to guarantee (and the spec doesn't seem to) any traceEvents being present as long as we have samples and stack frames. 

When I tested a production profile taken from Hermes it did not have any importable events, just a metadata event with a thread name. This PR updates the implementation to iterate over partitioned samples instead of traceEvents so we construct profiles for all thread + process pairs referenced in the samples array.

| Before | After |
| --- | ----- |
| <img width="1454" alt="Screenshot 2024-01-03 at 9 58 56 AM" src="https://github.com/jlfwong/speedscope/assets/9957046/78c098a7-b374-4492-ad13-9ca79afdb40c"> | <img width="1450" alt="Screenshot 2024-01-03 at 9 58 17 AM" src="https://github.com/jlfwong/speedscope/assets/9957046/d2d5e82b-fa3e-43db-bf8a-e8c3b84cd13a"> |
This commit is contained in:
Zachary Marion 2024-01-03 13:04:22 -08:00 committed by GitHub
parent a3c0b15935
commit a6d700194f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 48 additions and 34 deletions

View File

@ -21,16 +21,6 @@
"args": {
"name": "mqt_js"
}
},
{
"name": "mqt_js",
"cat": "mqt_js",
"ph": "X",
"dur": 0,
"pid": 7512,
"ts": "11550183666",
"tid": "7695",
"args": {}
}
],
"samples": [

View File

@ -349,14 +349,50 @@ function frameInfoForEvent(
}
}
/**
* Constructs an array mapping pid-tid keys to profile builders. Both the traceEvent[]
* format and the sample + stack frame based object format specify the process and thread
* names based on metadata so we share this logic.
*
* See https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.xqopa5m0e28f
*/
function getProfileNameByPidTid(
function getProfileName(
processName: string | undefined,
threadName: string | undefined,
pid: number,
tid: number,
): string {
if (processName != null && threadName != null) {
return `${processName} (pid ${pid}), ${threadName} (tid ${tid})`
} else if (processName != null) {
return `${processName} (pid ${pid}, tid ${tid})`
} else if (threadName != null) {
return `${threadName} (pid ${pid}, tid ${tid})`
} else {
return `pid ${pid}, tid ${tid}`
}
}
function getProfileNamesFromSamples(
events: TraceEvent[],
partitionedSamples: Map<string, Sample[]>,
): Map<string, string> {
const processNamesByPid = getProcessNamesByPid(events)
const threadNamesByPidTid = getThreadNamesByPidTid(events)
const profileNamesByPidTid = new Map<string, string>()
partitionedSamples.forEach(samples => {
if (samples.length === 0) return
const pid = Number(samples[0].pid)
const tid = Number(samples[0].tid)
const profileKey = pidTidKey(pid, tid)
const processName = processNamesByPid.get(pid)
const threadName = threadNamesByPidTid.get(profileKey)
const profileName = getProfileName(processName, threadName, pid, tid)
profileNamesByPidTid.set(profileKey, profileName)
})
return profileNamesByPidTid
}
function getProfileNamesFromTraceEvents(
events: TraceEvent[],
partitionedTraceEvents: Map<string, TraceEvent[]>,
): Map<string, string> {
@ -373,19 +409,9 @@ function getProfileNameByPidTid(
const profileKey = pidTidKey(pid, tid)
const processName = processNamesByPid.get(pid)
const threadName = threadNamesByPidTid.get(profileKey)
const profileName = getProfileName(processName, threadName, pid, tid)
if (processName != null && threadName != null) {
profileNamesByPidTid.set(
profileKey,
`${processName} (pid ${pid}), ${threadName} (tid ${tid})`,
)
} else if (processName != null) {
profileNamesByPidTid.set(profileKey, `${processName} (pid ${pid}, tid ${tid})`)
} else if (threadName != null) {
profileNamesByPidTid.set(profileKey, `${threadName} (pid ${pid}, tid ${tid})`)
} else {
profileNamesByPidTid.set(profileKey, `pid ${pid}, tid ${tid}`)
}
profileNamesByPidTid.set(profileKey, profileName)
})
return profileNamesByPidTid
@ -616,7 +642,7 @@ function eventListToProfileGroup(
): ProfileGroup {
const importableEvents = filterIgnoredEventTypes(events)
const partitionedTraceEvents = partitionByPidTid(importableEvents)
const profileNamesByPidTid = getProfileNameByPidTid(events, partitionedTraceEvents)
const profileNamesByPidTid = getProfileNamesFromTraceEvents(events, partitionedTraceEvents)
const profilePairs: [string, Profile][] = []
@ -647,10 +673,8 @@ function eventListToProfileGroup(
}
function sampleListToProfileGroup(contents: TraceWithSamples): ProfileGroup {
const importableEvents = filterIgnoredEventTypes(contents.traceEvents)
const partitionedTraceEvents = partitionByPidTid(importableEvents)
const partitionedSamples = partitionByPidTid(contents.samples)
const profileNamesByPidTid = getProfileNameByPidTid(contents.traceEvents, partitionedTraceEvents)
const profileNamesByPidTid = getProfileNamesFromSamples(contents.traceEvents, partitionedSamples)
const profilePairs: [string, Profile][] = []