Closed Bug 1359716 Opened 4 years ago Closed 4 years ago

Make test_render_perf.html more accurate

Categories

(DevTools :: Console, defect, P1)

defect

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Iteration:
55.4 - May 1
Tracking Status
firefox55 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Details

(Whiteboard: [console-html])

Attachments

(1 file)

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: nobody → nchevobbe
Priority: -- → P1
Whiteboard: [console-html]
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-
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.
Flags: needinfo?(odvarko)
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+
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0251b8095351
Make test_render_perf.html more accurate; r=Honza
https://hg.mozilla.org/mozilla-central/rev/0251b8095351
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Iteration: --- → 55.4 - May 1
Flags: qe-verify?
Flags: qe-verify? → qe-verify-
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.