Make test_render_perf.html more accurate

RESOLVED FIXED in Firefox 55

Status

DevTools
Console
P1
normal
RESOLVED FIXED
a year ago
a month ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

unspecified
Firefox 55
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [console-html])

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
At the moment, we only measure the time it takes to render 1000 messages (because of the cap), 10 times, which on my machine takes ~2s. With such a low time, it is difficult to be certain that your patch made the console faster.
Bumping up the number of messages could take longer and thus can show better if a patch has some benefits on perf.
Also, displaying the median time to render in addition to the average might prevent some unusual low/high spikes.
(Assignee)

Updated

a year ago
Assignee: nobody → nchevobbe
Priority: -- → P1
Whiteboard: [console-html]
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8861830 [details]
Bug 1359716 - Make test_render_perf.html more accurate;

https://reviewboard.mozilla.org/r/133820/#review136790

I am experiencing time out on my machine (Win10):

17 INFO TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html | Test timed out. 

Honza

::: devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:54
(Diff revision 1)
>  }
>  
>  window.onload = async function () {
>    try {
> +    const Services = browserRequire("Services");
> +    Services.prefs.setIntPref("devtools.hud.loglimit", numMessages);

What is the `devtools.hud.loglimit.console` pref for?
Attachment #8861830 - Flags: review?(odvarko) → review-
(Assignee)

Comment 3

a year ago
mozreview-review-reply
Comment on attachment 8861830 [details]
Bug 1359716 - Make test_render_perf.html more accurate;

https://reviewboard.mozilla.org/r/133820/#review136790

Oh yeah, that make sense since it takes a lot of times.
So for now, the test isn't runned on TRY: http://searchfox.org/mozilla-central/source/devtools/client/webconsole/new-console-output/test/chrome/chrome.ini
I don't know if we want it to be run at some point, I was thinking that we were going to use it only locally to see if a patch has some benefits on performance.
What do you think ?

> What is the `devtools.hud.loglimit.console` pref for?

It the maximum number of messages we show in the console.
The default is 1000, and here I am setting it as the same number of messages we add in the console so it takes longer and we can better see if there's a performance boost/regression.
(Assignee)

Updated

a year ago
Flags: needinfo?(odvarko)

Comment 4

a year ago
mozreview-review
Comment on attachment 8861830 [details]
Bug 1359716 - Make test_render_perf.html more accurate;

https://reviewboard.mozilla.org/r/133820/#review136838

Ok, if the test isn't running on try, the timeout isn't big problem.

Thanks for the patch Nicolas!

Honza
Attachment #8861830 - Flags: review- → review+
Comment hidden (mozreview-request)

Comment 6

a year ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0251b8095351
Make test_render_perf.html more accurate; r=Honza

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0251b8095351
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.4 - May 1
Flags: qe-verify?
Flags: needinfo?(odvarko)
Flags: qe-verify? → qe-verify-

Updated

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