Simplify component tree

RESOLVED FIXED in Firefox 55

Status

()

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

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---
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

(2 attachments)

(Assignee)

Description

8 months ago
We should only use components when we really need them since they have a performance cost.
If possible, we should only use functions (which we already do), without wrapping then with React.createFactory which add an unnecessary overhead.

In the same time, we should try to lower the number of elements we use so the rendering time could be improved.
(Assignee)

Updated

8 months ago
Assignee: nobody → nchevobbe
Blocks: 1308219
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [console-html]

Updated

8 months ago
Iteration: --- → 55.4 - May 1
Flags: qe-verify?

Updated

8 months ago
Flags: qe-verify? → qe-verify-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

8 months ago
mozreview-review
Comment on attachment 8862782 [details]
Bug 1358507 - Simplify component tree.

https://reviewboard.mozilla.org/r/134668/#review137820

This looks great to me - the code is simpler and I'm seeing a significant improvement in the local perf test with the patch applied.


With it applied:
2 INFO took 11354.05 ms to render messages
3 INFO took 11717.070000000002 ms to render messages
4 INFO took 10557.14 ms to render messages
5 INFO took 11161.700000000004 ms to render messages
6 INFO took 13667.740000000005 ms to render messages
7 INFO took 13927.880000000005 ms to render messages
8 INFO took 11816.815000000002 ms to render messages
9 INFO took 10759.275000000009 ms to render messages
10 INFO took 12676.939999999988 ms to render messages
11 INFO took 12189.040000000008 ms to render messages
12 INFO took 14489.315000000002 ms to render messages
13 INFO took 14881.609999999986 ms to render messages
14 INFO took 13816 ms to render messages

Without it applied:
2 INFO took 14200.875 ms to render messages
3 INFO took 15046.345000000001 ms to render messages
4 INFO took 19900.745000000003 ms to render messages
5 INFO took 20300.77500000001 ms to render messages
6 INFO took 17477.03 ms to render messages
7 INFO took 19967.425000000003 ms to render messages
8 INFO took 18042.67 ms to render messages
9 INFO took 17025.75 ms to render messages
10 INFO took 16110.329999999987 ms to render messages
11 INFO took 17595.130000000005 ms to render messages
12 INFO took 15180.570000000007 ms to render messages
13 INFO took 15910.709999999992 ms to render messages
14 INFO took 21218.889999999985 ms to render messages
Attachment #8862782 - Flags: review?(bgrinstead) → review+

Comment 4

8 months ago
mozreview-review
Comment on attachment 8862783 [details]
Bug 1358507 - Adapt tests to the new component architecture;

https://reviewboard.mozilla.org/r/134670/#review137822

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_timestamps.js:13
(Diff revision 1)
>  
>  "use strict";
>  
>  const {PrefObserver} = require("devtools/client/shared/prefs");
>  
> -const TEST_URI = "data:text/html;charset=utf-8,Web Console test for " +
> +// const TEST_URI = "data:text/html;charset=utf-8,Web Console test for " +

These commented lines can be removed
Attachment #8862783 - Flags: review?(bgrinstead) → review+
Comment hidden (mozreview-request)

Comment 6

8 months ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/44e82e1cf34a
Simplify component tree. r=bgrins
https://hg.mozilla.org/integration/autoland/rev/00e46048a3b4
Adapt tests to the new component architecture; r=bgrins
https://hg.mozilla.org/mozilla-central/rev/44e82e1cf34a
https://hg.mozilla.org/mozilla-central/rev/00e46048a3b4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.