984146 - Prototype mutation makes code run slowly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Performance, defect, P3)

Description

[:gerard-majax]

Since a couple of days, my Nexus S exposes this in logcat:

E/GeckoConsole(   77): [JavaScript Warning: "mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create" {file: "resource://gre/modules/Preferences.jsm" line: 378}]
E/GeckoConsole(   77): [JavaScript Warning: "TypeError: mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create" {file: "app://system.gaiamobile.org/js/browser_context_menu.js" line: 31}]

I'm wondering how much it related to bug 983624, bug 983626 and some DataCloneError that happens in email client.

Comment 1

[Dave Huseby [:huseby]]

Can you confirm that the above log warning does indeed impact performance?  It certainly looks like a scary warning.

Comment 2

[Nicolas B. Pierron [:nbp]]

The __proto__ is deadly for the type inference, basically, after doing that this means that Type Inference is unable to follow any of the properties of the object as you are changing the underlying object lookup behind his back.

This basically means that every lookup you do on this object or any object created with this constructor will fallback on a slow path for reading/writing properties to the object.  I might under estimate this issue, Eric would know better. (ni: efaust)

(In reply to Alexandre LISSY :gerard-majax from comment #0)
> E/GeckoConsole(   77): [JavaScript Warning: "mutating the [[Prototype]] of
> an object will cause your code to run very slowly; instead create the object
> with the correct initial [[Prototype]] value using Object.create" {file:
> "resource://gre/modules/Preferences.jsm" line: 378}]

See Bug 982856.

> E/GeckoConsole(   77): [JavaScript Warning: "TypeError: mutating the
> [[Prototype]] of an object will cause your code to run very slowly; instead
> create the object with the correct initial [[Prototype]] value using
> Object.create" {file:
> "app://system.gaiamobile.org/js/browser_context_menu.js" line: 31}]

See
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Introduction_to_Object-Oriented_JavaScript#Inheritance

Comment 3

[:Eli Perelman]

So you are saying that this warning only appears when fiddling with __proto__ and not when using .prototype?

Comment 4

[Jeff Walden [:Waldo]]

Yes.  It appears when you do |obj.__proto__ = ...|.  It does not appear when you have |function F() {} F.prototype = ...;| or |function G() {} G.prototype.foo = ...;|.

Comment 5

[Eric Faust [:efaust]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> The __proto__ is deadly for the type inference, basically, after doing that
> this means that Type Inference is unable to follow any of the properties of
> the object as you are changing the underlying object lookup behind his back.
> 
> This basically means that every lookup you do on this object or any object
> created with this constructor will fallback on a slow path for
> reading/writing properties to the object.  I might under estimate this
> issue, Eric would know better. (ni: efaust)

It's even worse than that.

When you set __proto__, not only are you ruining any chances you may have had for future optimizations from Ion on that object, but you also force the engine to go crawling around to all the other pieces of TI (information about function return values, or property values, perhaps) which think they know about this object and tell them not to make many assumptions either, which involves further deoptimization and perhaps invalidation of existing jitcode.

Changing the prototype of an object in the middle of execution is really a nasty sledgehammer, and the only way we have to keep from being wrong is to play it safe, but safe is slow. If possible, try to create directly with prototype chain you want. If we really have to swizzle prototype chains around dynamically, I'm not sure I have great advice as to how to minimize the performance implications of that action.

Comment 6

[:Eli Perelman]

What you are saying all jives well with me. There isn't a lot of use cases where you would want to use __proto__ over .prototype, and I believe that the warning is justified if you are stepping into that territory.

Would it be beneficial to change the wording of the warning so that it's more specific to __proto__ rather than [[Prototype]]? Just curious.

Comment 7

[Jeff Walden [:Waldo]]

Well, it specifically speaks to *mutation*.  The [[Prototype]] concept itself is general, and fine.  Did you skim over the mention of "mutation" when reading?  Maybe we need to speak more forcefully to making *changes* here, somehow.

Comment 9

[Josh Mize [:jgmize]]

According to Mike Bostock, the author of d3, in https://github.com/mbostock/d3/issues/1805#issuecomment-38385759 "... changing the prototype is effectively the only good way to create an array subclass in ES5...", and the "... warning is not helpful (and in fact misleading because Object.create is not applicable for arrays)."

Is there some way this warning could be made more helpful for the specific case of array prototype mutation?

Comment 10

[freddy03h]

Attachment: 984146.patch

use Object.create() instead of __proto__

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=44115cf32f72

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8443915 [details] [diff] [review]
984146.patch

Review of attachment 8443915 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Thanks for killing this annoying warning.

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/88970e36e090fdf471ce40a0e8fd4793f5f27725