Bug 1820813 Comment 10 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Okay, I think I understand it better.  I think the fix for this is a bit more involved.  I think that, the Firefox Profiler shouldn't use clamped values ever, anywhere.  This would mean that Profiler performance markers wouldn't line up with (Web) performance API markers, but I think that's okay (?) because the Profiler wouldn't be showing or comparing the vales from the Web API, it will only be showing markers added via `profiler_add_marker`.  That call happens in the same control flow as adding Web API performance markers, so they look like they're the same, but conceptually we only show markers added via `profiler_add_marker` and those Markers are separate from the markers added in the peformance API.

[Here's a place](https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/dom/base/ChromeUtils.cpp#274-277) we're using a clamped timestamp for a profiler marker that's independent from the PerformanceMark/PerformanceMeasure objects.  [Here's a place](https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/RTCStatsReport.cpp#28) we're using clamped timestamps and I don't think this goes into the profiler, but it might be relying on precision/behavior that we don't have...  Fortunately I think that example I gave is the only other example of `profiler_add_marker` relying on a clamped timestamp - every other one I spot-checked because I thought it might was either using Timestamp::Now() (which is not fiddled) or getting the timestamp internally.

I think Bug 1820826 arises in more cases than described.   `profiler_add_marker` is using an internal (accurate) timestamp but no matter what is passed to the PerformanceMarker ctor, the Marker's startTime will be clamped.  It will either come from Performance::Now() (clamped) or it will come from another Mark's startTime (which eventually will have to be Performance::Now()).

I think both of these approach is going to be complicated, but that Option 2 is going to be a better approach.  Depending on the urgency of this need, I may be able to help prototype it.
Okay, I think I understand it better.  I think the fix for this is a bit more involved.  I think that, the Firefox Profiler shouldn't use clamped values ever, anywhere.  This would mean that Profiler performance markers wouldn't line up with (Web) performance API markers, but I think that's okay (?) because the Profiler wouldn't be showing or comparing the vales from the Web API, it will only be showing markers added via `profiler_add_marker`.  That call happens in the same control flow as adding Web API performance markers, so they look like they're the same, but conceptually we only show markers added via `profiler_add_marker` and those Markers are separate from the markers added in the peformance API.

~~[Here's a place](https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/dom/base/ChromeUtils.cpp#274-277) we're using a clamped timestamp for a profiler marker that's independent from the PerformanceMark/PerformanceMeasure objects.  [Here's a place](https://searchfox.org/mozilla-central/source/dom/media/webrtc/jsapi/RTCStatsReport.cpp#28) we're using clamped timestamps and I don't think this goes into the profiler, but it might be relying on precision/behavior that we don't have...  Fortunately I think that example I gave is the only other example of `profiler_add_marker` relying on a clamped timestamp - every other one I spot-checked because I thought it might was either using Timestamp::Now() (which is not fiddled) or getting the timestamp internally.~~

That's not correct, I had gotten confused thinking that `CreationTimestamp` was clamped, but it is not.  

I think Bug 1820826 arises in more cases than described.   `profiler_add_marker` is using an internal (accurate) timestamp but no matter what is passed to the PerformanceMarker ctor, the Marker's startTime will be clamped.  It will either come from Performance::Now() (clamped) or it will come from another Mark's startTime (which eventually will have to be Performance::Now()).

I think both of these approach is going to be complicated, but that Option 2 is going to be a better approach.  Depending on the urgency of this need, I may be able to help prototype it.

Back to Bug 1820813 Comment 10