Closed Bug 1667223 Opened 4 years ago Closed 4 years ago

mach perftest VIEW is broken on GS5 locally: replicate time just keeps increasing

Categories

(Testing :: mozperftest, defect)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Originally https://jira.mozilla.com/browse/FXP-1340

The values just keeping increasing over a given run:

"replicates": [342603, 367380, 392318, 417116, 442098, 466967, 492591, 517842, 542610, 567952, 592352, 617368, 642265, 666794, 691967, 717060, 742357, 767901, 792718, 817542, 842695, 868057, 893527, 917921, 943165]

From sparky:

Does it actually take 342s for a single trial? If not, then it's likely related to the log parsing - might be different for gs5 (android 10?) vs our p2/g5 devices


From acreskey:

Might be this: process creation time
https://github.com/sitespeedio/browsertime/blob/5a20f579f393451136f41dae8f74dc07a192d93a/lib/android/index.js#L277-L327

I have noticed that my GS5 says, "Start" instead of "START" in the logcat for the process start time.

To be explicit, I mean Galaxy S5, which is a fairly old device.

From acreskey:

Might be this: process creation time
https://github.com/sitespeedio/browsertime/blob/5a20f579f393451136f41dae8f74dc07a192d93a/lib/android/index.js#L277-L327

I confirmed that we're not getting reasonable process creation times. For 5 sequential perftest iterations, the processStartTime is:

1606847225770
1606847225762
1606847225776
1606847225767
1606847225773

These should be increasing but they're not (e.g. run #3 is the longest). navigationStartTime looks reasonable:

1606869308305
1606869327505
1606869345365
1606869363467
1606869381940

These increase by ~18k each time (if this is in ms, that's ~18 seconds which seems a little long but might be accurate).

The issue appears to be that the pid passed in is incorrect – pid 1 is given for each run. The root cause seems to be that the pidof shell command acts differently on my GS5 than it does on my P2 (from https://github.com/sitespeedio/browsertime/blob/5a20f579f393451136f41dae8f74dc07a192d93a/lib/android/index.js#L245).

Note to self: I'm able to use a local browsertime in ./mach perftest by running with:

pushd ../ && tar -czf browsertime.tar.gz browsertime && popd && rm -rf ~/.mozbuild/browsertime/ && ./run-perftest.sh

Basically, update python/mozperftest/mozperftest/test/browsertime/package.json to point at a local tar ball, tar up the browsertime directory to that path, delete the one in mozbuild, and run ./mach perftest.

Btw, if you're modifying things in browsertime, you can directly modify the code in ~/.mozbuild/browsertime or you might be able to make a link from the ~/.mozbuild/browsertime/node_modules/browsertime folder to your local repo.

I filed https://github.com/sitespeedio/browsertime/issues/1448 against browsertime so I can open a PR to fix the issue.

Assignee: tarek → michael.l.comella

(In reply to Greg Mierzwinski [:sparky] from comment #5)

you might be able to make a link from the ~/.mozbuild/browsertime/node_modules/browsertime folder to your local repo.

I confirmed that this worked correctly and is more performant than my option from comment 4. Unfortunately, it's easy to forget you have it enabled but hopefully the upstream browsertime is updated soon and update our dependency on it soon.

Oh nevermind, I must have had some other state present (the package.json changes?) when I was testing it.

(In reply to Michael Comella (:mcomella) [needinfo or I won't see it] from comment #7)

I opened a PR to fix the issue: https://github.com/sitespeedio/browsertime/pull/1449

The PR is closed. It's not yet in mozilla-central though so I filed bug 1683197 to update browsetime to the version that includes it. As such, I'll close this bug and we can track that fix getting into m-c in that bug.

Note: I'm getting a replacement device shipped to me that doesn't have this issue so fixing this bug is no longer as urgent.

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