The tablewidget tests spend a lot of time printing CSS warnings to the console

RESOLVED FIXED in Firefox 43

Status

RESOLVED FIXED
3 years ago
5 months ago

People

(Reporter: sjakthol, Assigned: sjakthol)

Tracking

unspecified
Firefox 43
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
When those tests are executed locally following messages are printed to the output (example):
> 7654 INFO Console message: [JavaScript Warning: "Property contained reference to invalid variable.  Error in parsing value for 'color'.  Falling back to 'inherit'." {file: "chrome://browser/skin/devtools/widgets.css" line: 1272 column: 30380 source: " var(--theme-body-color)"}]

Those messages are printed as the variables are defined in theme-specific sheets which are not loaded in the test document and require the .theme-{light, dark} class to be set.

For a recent mozilla-central push [1] the runtimes are:
* browser_tableWidget_basic.js: 23988ms
* browser_tableWidget_keyboard_interaction.js: 7120ms
* browser_tableWidget_mouse_interaction.js: 6496ms

They are very slow considering that the tests are very simple.
[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux/1440171639/mozilla-central_ubuntu32_vm_test-mochitest-devtools-chrome-2-bm05-tests1-linux32-build5.txt.gz
(Assignee)

Comment 1

3 years ago
Created attachment 8651496 [details] [diff] [review]
bug-1197585-tablewidget-warnings.patch

Here's a patch that fixes the warnings by introducing the required theme sheet and class to the test document.

With this patch the times are following [1]:
* browser_tableWidget_basic.js: 1555ms
* browser_tableWidget_keyboard_interaction.js: 1239ms
* browser_tableWidget_mouse_interaction.js: 1336ms

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=63ae08917a29

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sjakthol@outlook.com-63ae08917a29/try-linux/try_ubuntu32_vm_test-mochitest-devtools-chrome-2-bm03-tests1-linux32-build36.txt.gz
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
Attachment #8651496 - Flags: review?(bgrinstead)
Comment on attachment 8651496 [details] [diff] [review]
bug-1197585-tablewidget-warnings.patch

Review of attachment 8651496 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch - thanks for the patch!

Relatedly, we should profile why it's that slow to print all those messages.  That's even without the browser console opened, so it might be a good way to capture backend slowness
Attachment #8651496 - Flags: review?(bgrinstead) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 4

3 years ago
Took a quick profile and 42% of the total time is spent in Tester.onConsoleMessage() [1] (self cost 8%) and most of that is spent in StructuredLogger._dumpMessage() (self cost 31%) [2].

So it's not the backend that is slow, it's just the dumping which is expensive.

[1] https://dxr.mozilla.org/mozilla-central/rev/ba43a48d3c528cc956335793e02504e5ca2c149f/testing/mochitest/browser-test.js#400
[2] https://dxr.mozilla.org/mozilla-central/rev/ba43a48d3c528cc956335793e02504e5ca2c149f/testing/mochitest/tests/SimpleTest/TestRunner.js#266
https://hg.mozilla.org/mozilla-central/rev/54ac9a920d85
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
(In reply to Sami Jaktholm from comment #4)
> Took a quick profile and 42% of the total time is spent in
> Tester.onConsoleMessage() [1] (self cost 8%) and most of that is spent in
> StructuredLogger._dumpMessage() (self cost 31%) [2].
> 
> So it's not the backend that is slow, it's just the dumping which is
> expensive.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/rev/
> ba43a48d3c528cc956335793e02504e5ca2c149f/testing/mochitest/browser-test.
> js#400
> [2]
> https://dxr.mozilla.org/mozilla-central/rev/
> ba43a48d3c528cc956335793e02504e5ca2c149f/testing/mochitest/tests/SimpleTest/
> TestRunner.js#266

Ah, good to know - thanks!

Updated

5 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.