Closed Bug 1388709 Opened 5 years ago Closed 5 years ago

Console `originAttributes` attributes slow things down and is unused

Categories

(DevTools :: Console, enhancement)

enhancement
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
(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...
Assignee: nobody → poirot.alex
It looks like the following changeset removed the last usage of this property:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d07ca792049f
bug 1314361
Oh ok, I better understand. It was introduced in bug 1211665 for WebExtension purpose and removed in bug 1314361 by WebExtension changes...
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?
Attachment #8895347 - Flags: review?(wmccloskey)
Attachment #8895347 - Flags: review?(wmccloskey) → review?(kmaglione+bmo)
Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

https://reviewboard.mozilla.org/r/166542/#review171902
Attachment #8895347 - Flags: review?(kmaglione+bmo) → review+
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
Attachment #8895347 - Flags: review?(amarchesini)
Comment on attachment 8895347 [details]
Bug 1388709 - Remove console `originAttributes` property.

https://reviewboard.mozilla.org/r/166542/#review177256
Attachment #8895347 - Flags: review?(amarchesini) → review+
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f833a3744f6
Remove console `originAttributes` property. r=baku,kmag
https://hg.mozilla.org/mozilla-central/rev/9f833a3744f6
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.