Katelyn Gadd (:kael) Reporter
	Description • 12 years ago

While bug 626021 claims that SpiderMonkey is better than V8 at property getters/setters (possibly true, I haven't checked), it's unfortunately far from a winner for value properties ({ value: x } instead of { get, set }).

Normally this would be a case where you could say 'well don't use value properties then, just do foo.bar = x'. Unfortunately, the ES5 specification provides no way to actually do this in the case where a prototype in your prototype chain has defined a property named bar - there's no way to bypass that property and store a value into your this-reference. The only way to replace your prototype's definition of 'bar' is to use defineProperty on your this-reference to define a new property named 'bar' local to your instance.

In Opera and Chrome, a value-property is effectively as fast as a value stored with a regular assignment, regardless of whether writable is set. No matter what I do, value-properties in SpiderMonkey incur a tremendous performance hit.

An alternate solution would be to provide a cross-browser way to bypass properties from further up the prototype chain, I suppose... but value-properties are also tremendously useful for creating lazily evaluated (or memoized) properties, where you first defineProperty a property with a getter, and then when the getter is invoked, it replaces itself with a value property.

Obvious things I tried:

Object.defineProperty(instance, "propertyName", null) // Throws
delete instance.propertyName // Silently does nothing
delete instance["propertyName"] // Silently does nothing

Simple test case:

http://jsperf.com/defineproperty-vs-regular-property

This issue is responsible for a lot of slowness for JSIL demos in Firefox.

	Boris Zbarsky [:bzbarsky]
	Comment 1 • 12 years ago

I'll bet money that TI is involved somehow.  ;)

	Brian Hackett [Laid off!]
	Comment 2 • 12 years ago

(In reply to Boris Zbarsky (:bz) from comment #1)
> I'll bet money that TI is involved somehow.  ;)

Bingo.

When a constructor looks like this:

function foo() {
  this.f = ...
  this.g = ...
}
x = new foo();

We can pick up the new properties definitely held by foo objects and compile accesses to the objects later on in the manner of a Java or .NET compiler.  Otherwise we fall back to a PIC, which can be slow.  V8 is never able to compile accesses in this way, but is smarter about doing shape checks and generates faster code when they are required (IonMonkey is / will be doing similar tricks).

For peak performance, we'll want the engine to statically know the properties on the new objects, but it's not clear to me yet what your use case is and the best way to go about this.  Paragraph 2 of comment 0 is only correct when the prototype of the object has a setter for the property being written, e.g. writing x.f = 0 will create an own property on x even if x's prototype has a data property for f.  Are you creating setters on prototype chains, or allowing untrusted code to do so?

	Katelyn Gadd (:kael) Reporter
	Comment 3 • 12 years ago

This is all 'untrusted' code, I think; it's running in content. It's not cross-domain or cross-frame, though. Also I should point out that *most* of the uses of this don't occur in constructors; they occur in prototypes that inherit from other prototypes.

Is it possible for me to give the runtime the necessary info to optimize these properties the way it would normal ones set in the constructor? If I made sure to construct each prototype itself using a constructor that sets all the fields and defines the properties, would that allow TI to look at the constructor and generate the smarter code?

If writing to x.f creates an own property on x when f is a data property from the prototype, that's interesting - that may address some of this problem for me. Unfortunately, I do have getters up the prototype chain in various places - some of them don't have setters though.

A slightly contrived but real example is that all my instances have a special value property called '__ThisType__' that points to their type object. Each time I create a prototype, I define that value to point to the associated type object that contains the metadata. So, If I have a class A, class B's prototype is Object.create(A.prototype). You're saying that if __ThisType__ is a value property, I should be able to replace it in B.prototype by doing a regular assignment, but if it has a getter/setter I can't?

And for an example where it has to be a getter, not a value property - at least initially - is where I'm lazily evaluating the value of a constant property on an object. I evaluate it the first time it's used (inside a getter) and then replace that getter property with a constant value property. I'm not aware of a way to replace this with a non-defineProperty own property, since there's no way to delete that getter once it's created. The code that does this is here:

http://pastebin.mozilla.org/1699571

JSIL.Host.runLater is basically a slightly optimized setTimeout(1, callback), because Object.defineProperty from within a getter is broken in some JS runtimes (not spidermonkey, though, last I checked - so thanks for being awesome there :D). This has the net effect of replacing the inefficient getter with an efficient value property within a millisecond, so after a few hundred milliseconds, all the lazy expressions the app is using in its steady state are no longer lazy.

One of the concrete reasons I need lazy properties is that the .NET type system allows cycles - each type may have a static constructor that runs when the type is first actually used at runtime, and that static constructor may reference other types. As a result, if A's static constructor references B, and B's static constructor references A, the runtime is supposed to initialize them in the order they are accessed, and when it hits a cycle, silently continue as if that type were fully initialized (this is kind of nuts, but they basically tell you that you're shooting your own foot off by doing it). The lazy property mechanism allows me to emulate this kind of 'on-demand' initialization with the JS runtime, instead of having to manually anticipate every location where a type initialization could occur, and put an expensive function call there.

	David Mandelin [:dmandelin]
	Comment 4 • 12 years ago

Let's revive the discussion. Kevin, can you briefly restate the scenarios where you need to define value properties with Object.defineProperty? It sounds like some cases you had it's fine to just do a normal property set, and that the biggest place you need it is for initializing property values on demand. But I'm wondering if there's a simpler (for the engine) way to do it.

That said, it also seems worthwhile fixing this eventually even if it can be worked around. I checked an IM browser and it's still 4x slower than a "regular property".

	Katelyn Gadd (:kael) Reporter
	Comment 5 • 12 years ago

The core issue is that once an object has had a property defined with defineProperty, you have to keep using defineProperty. I run into this primarily when doing lazy evaluation, using this pattern:

https://github.com/kevingadd/JSIL/blob/master/Libraries/JSIL.Core.js#L260

The property has to start out with a getter, at which point the only way to turn it into a value is using defineProperty. I used to do 'delete' to try and remove the getter, but that doesn't work in every JS engine and deopts objects into hashtables in some engines too.

There are also cases where I have to use defineProperty, because an object in the prototype chain has a property with the same name that has a getter or setter. In that case, there is no way to create a property on the 'this' reference without invoking the getter/setter. If there were a way to do this it would probably also simplify the lazy evaluation case. I don't really need the features of defineProperty (configurable, enumerable, etc.) - I just need a way to replace a getter/setter (or property defined in the prototype chain) with a value property on the this-reference.

	Jeff Walden [:Waldo]
	Comment 6 • 12 years ago

(In reply to Kevin Gadd (:kael) from comment #5)
> The property has to start out with a getter, at which point the only way to
> turn it into a value is using defineProperty. I used to do 'delete' to try
> and remove the getter, but that doesn't work in every JS engine and deopts
> objects into hashtables in some engines too.

Redefining the property so that it's structurally different will deoptimize equally.  (Unless the property was the last one added, but I don't know if we optimize redefinition in that case, and it seems doubtful the general pattern fits that niche anyway.)

	David Mandelin [:dmandelin]
	Comment 7 • 12 years ago

(In reply to Kevin Gadd (:kael) from comment #5)
> The core issue is that once an object has had a property defined with
> defineProperty, you have to keep using defineProperty. I run into this
> primarily when doing lazy evaluation, using this pattern:
> 
> https://github.com/kevingadd/JSIL/blob/master/Libraries/JSIL.Core.js#L260
> 
> The property has to start out with a getter, at which point the only way to
> turn it into a value is using defineProperty. I used to do 'delete' to try
> and remove the getter, but that doesn't work in every JS engine and deopts
> objects into hashtables in some engines too.

Yeah, |delete| generally does nasty things to optimizations. And in a high-level sense, the way properties are optimized in JS engines generally don't want you to be changing properties around. Here's some alternatives and how they impact engines; I'm not sure how they impact your design:

A. Fixed set of properties. Initialize the object with properties in its constructor (or early in its lifetime) and don't change them later. The property accesses will most likely get highly optimized. This would mean that you're not doing lazy initialization of properties; I'm not sure what you'd be doing instead.

B. Fixed set of properties with getters. Initialize the object with properties that have getters. They won't be as fast as case A (and your getter will have a test and branch in it), but at least they won't get any slower from changing things around.

C. Lazy initialization pattern with defineProperty. IonMonkey should be able to get performance equivalent to Chrome for this case; the fact that it doesn't is a missing optimization. But Chrome's performance is only 0.66x as fast as IM on "regular property", so it will still cost you something compared to the "fixed set of properties" case (assuming the "fixed set of properties" is in fact possible).

> There are also cases where I have to use defineProperty, because an object
> in the prototype chain has a property with the same name that has a getter
> or setter. In that case, there is no way to create a property on the 'this'
> reference without invoking the getter/setter. If there were a way to do this
> it would probably also simplify the lazy evaluation case. I don't really
> need the features of defineProperty (configurable, enumerable, etc.) - I
> just need a way to replace a getter/setter (or property defined in the
> prototype chain) with a value property on the this-reference.

This just seems to be a missing optimization to me--it's a perfectly valid use case and should be able to go fast.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 8 • 12 years ago

Not sure on priority here yet -- either P1 or P2, depending on what strategy we take for .NET -> JS porting, and whether it affects that or emscripten.

	Till Schneidereit [:till]
	Comment 9 • 12 years ago

This blocks self-hosting of some functions. Specifically, the spec for Array#map requires using [[DefineOwnProperty]] in Step 8.c.iii., which requires us to either expose that as a native function, or, preferably, to use Object.defineProperty with the right flags.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 10 • 12 years ago

Till, is this still an issue for self-hosting?

	Jeff Walden [:Waldo]
	Comment 11 • 12 years ago

Pretty sure it would be.

Although it's worth noting, in some cases we really probably don't even want exactly this method to be fast.  For example, places that create an array and then initialize most or all of the elements of it might want different fast-paths to smack values directly into elements.  Object.defineProperty in general has to worry about property conflicts and all that, where in narrower situations we might know conflicts to be impossible.

	Till Schneidereit [:till]
	Comment 12 • 12 years ago

What Waldo says: we don't necessarily need this exact function to be fast. What we need to implement quite a few spec algorithms is a fast way of defining a propery we know not to collide with anything.

Maybe the best thing to do would be to add an intrinsic for putProperty.

	Katelyn Gadd (:kael) Reporter
	Comment 13 • 11 years ago

Either this is fixed, or something else (LICM maybe) has successfully optimized the related jsperf tests into no-ops.

	Guilherme Lima
	Comment 14 • 7 years ago

From https://jsperf.com/defineproperty-vs-regular-property/26 I think it's possible to say that DefineProperty is 7x slower than regular property