If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

ts_paint should use the process creation time instead of receiving a timestamp from the test script



4 years ago
4 years ago


(Reporter: vladan, Assigned: jmaher)


Firefox Tracking Flags

(Not tracked)



(1 attachment)



4 years ago
Right now there is a >50% ts_paint (startup time) regression on Talos because the test script launches the browser differently:


It seems the test script is passing ts_paint a timestamp, but this timestamp isn't the actual Firefox process creation time. This timestamp is the time when some Python call is made to launch the process(?)

The ts_paint test itself should just use the process creation time from nsIApppStartup

Comment 1

4 years ago

Comment 2

4 years ago
I can pull the startup time from startupInfo['process'] which is the earliest timestamp I can find.

I found on android we don't have firstPaint available, so sticking with our method of Date.now() when we pop into the mozafterpaint handler will keep with what we had previously and work on both platforms.

Comment 3

4 years ago
Created attachment 831520 [details] [diff] [review]
update tspaint to use start time from the browser, not python (1.0)

this patch gets us back to where we were prior to the mozprocess switch.  I am much happier getting this regression removed.

tested locally on android and desktop, tested 5 iterations on desktop builds here: https://tbpl.mozilla.org/?tree=Try&rev=f319712807b0.  Confirmed a ~500ms drop in times and that the times are mostly stable.
Assignee: nobody → jmaher
Attachment #831520 - Flags: review?(jhammel)

Comment 4

4 years ago
Comment on attachment 831520 [details] [diff] [review]
update tspaint to use start time from the browser, not python (1.0)

Review of attachment 831520 [details] [diff] [review]:

Looks fine if it is green on try, with nits.

::: talos/startup_test/tspaint_test.html
@@ +17,5 @@
> +  var startupTime = -1;
> +  if (useSpecialPowers) {
> +    // We need to define this still
> +    throw('NOT DEFINED');

I find this confusing. Is this to detect if special powers is defined? Should this be fixed before checkin?
Attachment #831520 - Flags: review?(jhammel) → review+

Comment 5

4 years ago
yes this is in the case if we have specialpowers defined which isn't defined or possible atm, but needs to happen at some point in time.  This is a really a placeholder for a future bug.

Comment 6

4 years ago
landed on talos:

There are a few other talos changes in the works, specifically an update to xperf to support blobber uploads of .etl files when we fail which I would prefer to hold off on deploying this until those are landed.


4 years ago
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.