Add a test case to test-perf.html to measure streaming time with a store containing {logLimit} messages

RESOLVED FIXED in Firefox 56

Status

()

Firefox
Developer Tools: Console
P1
normal
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

unspecified
Firefox 56
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [console-html])

MozReview Requests

()

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

Attachments

(1 attachment)

(Assignee)

Description

10 months ago
This will help us see if there are any differences from the simpler stream test.
(Assignee)

Updated

10 months ago
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Flags: qe-verify-
Priority: -- → P1
Whiteboard: [console-html]
Comment hidden (mozreview-request)

Updated

10 months ago
Whiteboard: [console-html] → [console-html] [triage]

Updated

10 months ago
Iteration: --- → 56.1 - Jun 26
Whiteboard: [console-html] [triage] → [console-html]

Comment 2

10 months ago
mozreview-review
Comment on attachment 8878459 [details]
Bug 1372814 - Add test case and performance measurement in test-perf.html .

https://reviewboard.mozilla.org/r/149816/#review155206

Those user timing are **very** useful in the profile, thanks!!

::: devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:155
(Diff revision 1)
>  
> +  const measureLabel = "stream - add single message";
> +  for (let i = 0; i < NUM_STREAMING; i++) {
> +    const measure = startMeasure(measureLabel);
> +    await addMessage(wrapper, testPackets[i]);
> +    measure.stop(false);

In other places the 'false' is implied

::: devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:179
(Diff revision 1)
>      {hud: EventEmitter.decorate({proxy: {}})},
>      {},
>      null,
>      document,
>    );
>    wrapper.init();

While we are at it, maybe an `await wait(500)` here before we start the profiler might give the test some extra startup time to reduce variability (haven't tested this myself, feel free to not add)
Attachment #8878459 - Flags: review?(bgrinstead) → review+
(Assignee)

Comment 3

10 months ago
mozreview-review-reply
Comment on attachment 8878459 [details]
Bug 1372814 - Add test case and performance measurement in test-perf.html .

https://reviewboard.mozilla.org/r/149816/#review155206

> In other places the 'false' is implied

The stop function takes a boolean whose defulat is true : 
```
    stop: (clear = true) => {
      performance.measure(label, startLabel);
      const entries = performance.getEntriesByName(label);
      if (clear){
        performance.clearMeasures(label);
      }
      return entries[entries.length - 1];
    }
```
This is the only case where we don't want the performance entries to be clear right away because we want to use them after the streaming iterations.
Ah, misread that. Carry on!
(Assignee)

Comment 5

10 months ago
mozreview-review-reply
Comment on attachment 8878459 [details]
Bug 1372814 - Add test case and performance measurement in test-perf.html .

https://reviewboard.mozilla.org/r/149816/#review155206

> While we are at it, maybe an `await wait(500)` here before we start the profiler might give the test some extra startup time to reduce variability (haven't tested this myself, feel free to not add)

I tried it and launched the test a few time, I think this might help, thanks !
Comment hidden (mozreview-request)

Comment 7

10 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ee03c4d8ee6c
Add test case and performance measurement in test-perf.html . r=bgrins
Backed out for eslint failures in test_render_perf.html:

https://hg.mozilla.org/integration/autoland/rev/ebd15598fd772fe8c1053b72a1c52adb65559453

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ee03c4d8ee6c25aeedcebcba266cfcb3d6c560ad&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=108264299&repo=autoland

 TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:65:17 | Missing space before opening brace. (space-before-blocks)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:70:4 | Missing semicolon. (semi)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:116:30 | Missing space before opening brace. (space-before-blocks)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:133:1 | Line 119 exceeds the maximum line length of 90. (max-len)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:133:7 | 'filterToggleTimeOff' is assigned a value but never used. (no-unused-vars)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:138:7 | 'filterToggleTimeOn' is assigned a value but never used. (no-unused-vars)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:159:1 | Line 145 exceeds the maximum line length of 90. (max-len)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/devtools/client/webconsole/new-console-output/test/chrome/test_render_perf.html:162:52 | Infix operators must be spaced. (space-infix-ops)
Flags: needinfo?(nchevobbe)
(Assignee)

Comment 9

10 months ago
Sorry about that. Let me fix those errors
Flags: needinfo?(nchevobbe)
Comment hidden (mozreview-request)

Comment 11

10 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/54ba22bded5e
Add test case and performance measurement in test-perf.html . r=bgrins

Comment 12

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/54ba22bded5e
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.