1359716 - Make test_render_perf.html more accurate

Status: RESOLVED FIXED

(DevTools :: Console, defect, P1)

Description

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1359716 - Make test_render_perf.html more accurate;

Set the logLimit to the number of messages we add.
Bump up the number of messages to add to 4000.
Iterate 25 times instead of 10.
Compute the median to limit the impact of low/high spikes.

Review commit: https://reviewboard.mozilla.org/r/133820/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/133820/

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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?

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

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.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8861830 [details]
Bug 1359716 - Make test_render_perf.html more accurate;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133820/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0251b8095351
Make test_render_perf.html more accurate; r=Honza

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0251b8095351