profiler icon indicating copy to clipboard operation
profiler copied to clipboard

Add a correct "Recording start time"(?) to Profile Info

Open squelart opened this issue 4 years ago • 3 comments

#3457 should rename "Recording started" to "Process started" or similar. Now, we may still want to show something like "Recording started".

I think the frontend has enough information already (meta.startTime being the process start time, and being able to find the earliest visible sample/marker to display) to compute the local time corresponding to the left-most side of full range. Though I don't think the name "Recording started" would be 100% correct, since it's possible some of the early recording was lost if the profile buffer got full. Maybe "Profiling range start" would work? Better suggestions welcome.

After that, Firefox could store the timestamp when the profiler was started (backend change required, please let me know if needed), and store it in a new field, e.g. meta.recordingStart. And this could be shown correctly as "Recording started". But I believe that we should still show "Profiling start range", especially if it's different.

Bonus idea: Show the local time corresponding to displayed profile-relative times (in seconds) in some places. E.g.: When hovering the band with the times at the top of the timeline area; When hovering a "Start" time in the Marker Table; Etc.

squelart avatar Jul 27 '21 02:07 squelart

Corresponding back-end bug to add the content start time: https://bugzilla.mozilla.org/show_bug.cgi?id=1692934

squelart avatar Jul 08 '22 00:07 squelart

Bug 1692934 has landed. Starting with Firefox Nightly 104 build 20220708214204, we now have three new timestamps per process, in ms relative to the start time:

  • meta.profilingStartTime: This is when the profiler session started.
  • meta.profilingEndTime: This is when the profiler session ended. If meta.shutdownTime is also present, it will be the same number.
  • meta.contentEarliestTime: This is the earliest time that data was recorded and is still present in the profile.

These should now always be present. But if something went wrong, the contentEarliestTime could be missing or null. And obviously older profiles won't have this information. So the front-end should still be able to work without them.

But if they're present, I think they should be presented to the user in the "Profile Info" box, both as timestamps, and as durations (something like "Profiling session: 2h 10m 5s", "Available data: Up to 13m 12s").

Also, contentEarliestTime and profilingEndTime could be used as the initial time range, instead of trying to guess it from the data. And I think profilingStartTime should be used as the origin of time -- unless it's too confusing?

Since these are relative to each process, if there is more than one process they'll need to be shifted accordingly to match the parent process, like other timestamps. When combining them, I think the following makes most sense:

  • meta.profilingStartTime and meta.profilingEndTime: I think the parent process information makes the most sense there, since that's where the whole session was initiated and then stopped. There shouldn't be earlier child processes. And if child processes live longer, their extra data would probably not be expected by most users anyway (I clicked "Capture" there, why is there more data after that?). Also some child processes end early, so using their end time would cut the profile too short.
  • meta.contentEarliestTime: Use the earliest. This is because of how old data is discarded by chunks, each process could have its earliest surviving data at different points. This way we show the widest useful range.

squelart avatar Jul 11 '22 01:07 squelart

Also: Some threads show a section to their left, in blue with stripes, the hovering tooltip there says "This buffer was empty. Either the profiler was still initializing for a new thread, or the profiling buffer was full. [...]" Now that we have meta.contentEarliestTime (for whole processes), combined with each thread's registerTime, we could distinguish between the two situations and display a more accurate message.

squelart avatar Jul 11 '22 05:07 squelart