Closed Bug 984146 Opened 6 years ago Closed 6 years ago

Prototype mutation makes code run slowly

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S5 (4july)

People

(Reporter: gerard-majax, Assigned: freddy03h)

References

Details

(Keywords: perf, Whiteboard: [c=profiling p= s=2014.07.04.t u=])

Attachments

(1 file)

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.
Can you confirm that the above log warning does indeed impact performance?  It certainly looks like a scary warning.
Flags: needinfo?(nicolas.b.pierron)
Keywords: regression
Priority: -- → P3
Whiteboard: [c=profiling p= s= u=]
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
Flags: needinfo?(nicolas.b.pierron) → needinfo?(efaustbmo)
So you are saying that this warning only appears when fiddling with __proto__ and not when using .prototype?
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 = ...;|.
(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.
Flags: needinfo?(efaustbmo)
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.
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.
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?
Attached patch 984146.patchSplinter Review
use Object.create() instead of __proto__

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=44115cf32f72
Attachment #8443915 - Flags: review?(21)
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.
Attachment #8443915 - Flags: review?(21) → review+
https://github.com/mozilla-b2g/gaia/commit/88970e36e090fdf471ce40a0e8fd4793f5f27725
Assignee: nobody → freddy03h
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [c=profiling p= s= u=] → [c=profiling p= s=2014.07.04.t u=]
Target Milestone: --- → 2.0 S5 (4july)
You need to log in before you can comment on or make changes to this bug.