Closed Bug 1244837 Opened 4 years ago Closed 4 years ago

Bug 1238744 caused a massive slowdown in Marionette performance starting with Firefox 46.0a1

Categories

(Testing :: Marionette, defect, major)

46 Branch
defect
Not set
major

Tracking

(firefox45 unaffected, firefox46 fixed, firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- unaffected
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: whimboo, Assigned: yzen)

References

Details

(Keywords: perf, regression)

Attachments

(1 file)

Today I did a full run of our firefox-ui-tests and noticed that testing got so painful slow. While version 38.0 was e.g. opening and closing tabs that fast that you nearly weren't able to see it, it takes about 500ms for such a single action now. I could come with better numbers in a bit.

I will also do some testing and maybe I can clearly see in which version we have this decrease. Given that the client I'm using is the same, the reason should be somewhere in Marionette or maybe in Firefox.
Even the test location has been changed, the output below is the identical set of tests. Here we have a difference in execution time of factor 6!

$ time firefox-ui-functional --binary /mozilla/bin/esr38/firefox firefox_puppeteer/tests/test_tabbar.py
real	0m7.472s
user	0m6.060s
sys	0m0.640s

$ time firefox-ui-functional --binary /mozilla/bin/nightly/firefox firefox_ui_tests/firefox_ui_tests/puppeteer/test_tabbar.py
real	0m43.660s
user	0m14.384s
sys	0m2.268s
Severity: normal → major
Keywords: perf
Ok, Aurora is not that bad:

real	0m12.333s
user	0m12.248s
sys	0m0.832s

So this is a really recent regression in Firefox 47.0a1.
[Tracking Requested - why for this release]:
Execution time for tests exploded by a factor of 4-6 times. This would also cause a huge bottle neck on testing machines.

I wonder if we should have Talos tests for Marionette.
Version: 45 Branch → 47 Branch
Summary: Marionette testing got painful slow in recent versions of Firefox compared to 38.0 → Marionette testing got painful slow in Firefox 47.0a1
lets determine the cause- I assume mozregression could help us bisect?  There is value in tracking our test times and looking for any job (build/test) that increases by >10%.
I just noticed that my local Aurora was not up2date and I was still running a 45.0a2 version. After upgrading it I can see the same slowness. So this is a regression in early 46.0a1 then.
Version: 47 Branch → 46 Branch
I run mozregression and the builds have been nailed down to bug 1238744.
Blocks: 1238744
Flags: needinfo?(yzenevich)
Summary: Marionette testing got painful slow in Firefox 47.0a1 → Bug 1238744 caused a massive slowdown in Marionette performance starting with Firefox 46.0a1
Flags: needinfo?(ato)
You can run the above tests also via:

mach firefox-ui-test testing/firefox-ui/tests/firefox_ui_tests/puppeteer/test_tabbar.py
The slowdown seems to be when the interactions.Interactions#isVisible method is called: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/interactions.js#258
Flags: needinfo?(ato)
Sorry, I spoke too soon.  I believe it’s related to the retries to get the accessibility object: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/accessibility.js#112

It appears that we are always doing accessibility checks now, but that they only surface when the raiseAccessibilityExceptions capability is set to true.  I’m not sure this is necessary.  Perhaps we can disable accessibility checks altogether if the capability is not set?
(In reply to Andreas Tolfsen (:ato) from comment #9)
> true.  I’m not sure this is necessary.  Perhaps we can disable accessibility
> checks altogether if the capability is not set?

As what I got from the discussion last week on IRC that is how it should work. And in fx ui tests we definitely don't have enabled the accessibility capability.
Ill take a look at it today/tomorrow.
Assignee: nobody → yzenevich
Status: NEW → ASSIGNED
Flags: needinfo?(yzenevich)
---
 testing/marionette/accessibility.js                  | 18 ++++++++++++++++++
 .../marionette/tests/unit/test_accessibility.py      | 20 ++++++++++++++++++--
 testing/marionette/driver.js                         |  8 ++++++--
 3 files changed, 42 insertions(+), 4 deletions(-)

Review commit: https://reviewboard.mozilla.org/r/33467/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33467/
Attachment #8715463 - Flags: review?(ato)
Comment on attachment 8715463 [details]
MozReview Request: Bug 1244837 - only performing multiple attempts to retrieve an accessible object iff raisesAccessibilityExceptions capability is set to true. r=ato

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/33467/diff/1-2/
Attachment #8715463 - Attachment description: MozReview Request: Bug 1244837 - adding an explicit flag for multiple accessible object retrieval when performing a11y tests in marionette. r=ato → MozReview Request: Bug 1244837 - only performing multiple attempts to retrieve an accessible object iff raisesAccessibilityExceptions capability is set to true. r=ato
Attachment #8715463 - Flags: review?(ato) → review+
Comment on attachment 8715463 [details]
MozReview Request: Bug 1244837 - only performing multiple attempts to retrieve an accessible object iff raisesAccessibilityExceptions capability is set to true. r=ato

https://reviewboard.mozilla.org/r/33467/#review30285
https://hg.mozilla.org/mozilla-central/rev/2564c91f9a5c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
This works fine in latest Nightly builds. Thank you for the fix!

As noted this is a huge performance regression in Marionette test-only code. So we need this uplifted to mozilla-aurora.
Whiteboard: [checkin-needed-aurora]
No longer blocks: 1244255
You need to log in before you can comment on or make changes to this bug.