1388709 - Console `originAttributes` attributes slow things down and is unused

Status: RESOLVED FIXED

(DevTools :: Console, enhancement)

Description

[Alexandre Poirot [:ochameau]]

Based on bug 1382968 comment 42, it looks like the call to Cu.cloneInto()
has significant impact on console.log performances.
http://searchfox.org/mozilla-central/rev/0f16d437cce97733c6678d29982a6bcad49f817b/dom/console/ConsoleAPIStorage.js#132-135
    // Clone originAttributes to prevent "TypeError: can't access dead object"
    // exceptions when cached console messages are retrieved/filtered
    // by the devtools webconsole actor.
    aEvent.originAttributes = Cu.cloneInto(aEvent.originAttributes, {});

Commenting this line introduces a 30% win in console.log performances under heavy usage (see bug 1382968 comment 42).

I'm not an active contributor for the console, but I looked into its codebase, and I wasn't able to find any usage of this `originAttributes` object:
http://searchfox.org/mozilla-central/search?q=originAttributes&case=false&regexp=false&path=devtools%2F

The only one reference coming from webconsole code is this one:
http://searchfox.org/mozilla-central/rev/bd39b6170f04afeefc751a23bb04e18bbd10352b/devtools/server/actors/webconsole.js#1783
    delete result.originAttributes;

Where we immediately delete it as soon as we receive the console-api-log-event message...
It has been introduced in bug 1318006, as well as the cloneInto, in order to prevent Dead wrapper exceptions.

If that is really unused, it looks like we should remove it completely.

Comment 1

[Alexandre Poirot [:ochameau]]

(In reply to Alexandre Poirot [:ochameau] from comment #0)
> Commenting this line introduces a 30% win in console.log performances under
> heavy usage (see bug 1382968 comment 42).

I compared wrong numbers, it is actually 3 times faster rather than only 30%!
But this is profiler number, it may easier add some unreal overhead...

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1388709 - Remove console `originAttributes` property.

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

Comment 3

[Alexandre Poirot [:ochameau]]

It looks like the following changeset removed the last usage of this property:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d07ca792049f
bug 1314361

Comment 4

[Alexandre Poirot [:ochameau]]

Oh ok, I better understand. It was introduced in bug 1211665 for WebExtension purpose and removed in bug 1314361 by WebExtension changes...

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

Bill, :bz and :baku are both on PTO. Would you, or someone else you know, be confident reviewing this?

Comment 6

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b95b15bb3b7e81f7252bf58c231785e5c4748e5

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

https://reviewboard.mozilla.org/r/166542/#review171902

Comment 8

[Alexandre Poirot [:ochameau]]

Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

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

Comment 9

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 4 changes to 4 files
remote: 
remote: WebIDL file dom/webidl/Console.webidl altered in changeset f599bbbd037a without DOM peer review
remote: 
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: 
remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote: 
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.d_webidl hook failed
abort: push failed on remote

Comment 10

[Andrea Marchesini [:baku]]

Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

https://reviewboard.mozilla.org/r/166542/#review177256

Comment 11

[Alexandre Poirot [:ochameau]]

Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f833a3744f6
Remove console `originAttributes` property. r=baku,kmag

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9f833a3744f6