Improve the import of profiles generated from clang -ftime-trace=file.json

[fqueze]

2 changes:

• make the content of event.args.detail visible on markers (this shows which source file is being processed)
• isMainThread = true when pid == tid (will work only on Linux, but will still be a big relief there to have the actually useful thread at the top)

Example profile: https://share.firefox.dev/4rW0x7q

[codecov]

Codecov Report

❌ Patch coverage is 70.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.64%. Comparing base (3358235) to head (38eb6ad).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/profile-logic/import/chrome.ts	70.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5714      +/-   ##
==========================================
- Coverage   85.66%   85.64%   -0.02%     
==========================================
  Files         313      313              
  Lines       30985    31013      +28     
  Branches     8519     8449      -70     
==========================================
+ Hits        26543    26562      +19     
- Misses       4012     4021       +9     
  Partials      430      430              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[canova]

Thanks for the PR!

See my comment below about the beginEventDetail map, I don't think we need it, and ideally it should be gone, but also the current importer is very buggy, and because of it the marker payload/category logic is not working well. That's why it's unfair to ask a lot of changes that are not related to this patch in this PR. I'll file an issue and work on it as a follow-up.

[canova]

Nit: Can we extract this key generation logic and use it in all 3 places?
Edit: After thinking more about that I wanna remove all these as per my other comment, but they are for a follow-up.

[canova]

I was very surprised that we needed a map here, because normally we merge the payloads of start and end markers:

profiler/src/profile-logic/marker-data.ts

Lines 588 to 604 in 2ce8ab6

	// In the case of separate markers for the start and end of an interval,
	// merge the payloads together, with the end data overriding the start.
	function mergeIntervalData(
	startData: MarkerPayload | null,
	endData: MarkerPayload | null
	): MarkerPayload | null {
	if (startData === null) {
	return endData;
	}
	if (endData === null) {
	return startData;
	}
	return {
	...startData,
	...endData,
	};
	}

And this should have been enough. So locally I tried to remove it and noticed that it actually regressed the state.

Then I accidentally opened a can of worms, because it looks like even though we don't have marker payloads in a chrome event, we still assign a dummy payload to it here:

profiler/src/profile-logic/import/chrome.ts

Lines 1006 to 1010 in 2ce8ab6

	const newData = {
	...argData,
	type: name,
	category: event.cat,
	};

It looks like it's just for the category, but wait, we don't actually show the category anywhere in the UI since we don't have a marker schema!
And the marker category itself is assigned to Other by default!

So because of the fact that we always assign a payload even when it doesn't have a payload is breaking this marker payload merge logic...

So even though I don't like this beginEventDetail map, since the chrome importer was broken before this PR, it's unfair to ask for a bigger change from you. I will merge this PR and then I will follow-up myself with proper fixes that:

1. Put the category properly to the marker categories.
2. Remove the dummy payload if the event doesn't have any data.
3. Remove this beginEventDetail all together with all the logic around it.

---

Another issue I had with this map is that it wouldn't work well with the nested markers. I don't know if it's possible to have any from clang or any other tool, but I think that's a source of bugs too. But considering that this map will go away, we shouldn't really spend a lot of time on it.