1409097 - Remove the message-body-wrapper element from the DOM tree in the new console frontend

Status: RESOLVED WONTFIX

(DevTools :: Console, enhancement, P1)

Description

[Brian Grinstead [:bgrins]]

We should be able to get faster mounting times in react if we flatten the DOM that we render.

Here are some profiles with the test case: `for (var i = 0; i < 1000; i++) { console.log(i); }`

No changes:
https://perfht.ml/2kQRA1u

Removed message-body-wrapper:
https://perfht.ml/2kRaupb

And the if you hold the right arrow on the tree and expand it all the way to the end you should see a final call to setValueForProperty. In the second, we are 4 levels further left, meaning 4 less function calls (590px indent instead of 630px indent at 10px spacing each). Note that this is dependent on the deepest tree also being the slowest path, but even after alt+clicking and scanning the trees it does appear that the latter doesn't have anything deeper.  Also, flushBatchedUpdates does seem faster after doing a couple of profiles.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: Bug 1409097 - Remove unused scrollToMessage prop;

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

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: Bug 1409097 - Remove the message-body-wrapper element from the new console frontend;

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

Comment 3

[Brian Grinstead [:bgrins]]

Comment on attachment 8918952 [details]
Bug 1409097 - Remove unused scrollToMessage prop;

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

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8918952 [details]
Bug 1409097 - Remove unused scrollToMessage prop;

https://reviewboard.mozilla.org/r/189844/#review195304

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8919046 [details]
Bug 1409097 - Remove the message-body-wrapper element from the new console frontend;

Thanks for working on this Brian.
There seems to have an issue with "attachments" (stacktrace, table, …) due to this change (i'll attach a screenshot in the next comment).

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: console stacktrace and table misplaced

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Maybe grid layout would help in this case.

Comment 8

[Nicolas Chevobbe [:nchevobbe]]

Brian, didn't we say yesterday that we should close this ?

Comment 9

[Brian Grinstead [:bgrins]]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #8)
> Brian, didn't we say yesterday that we should close this ?

Yep, thanks