ES5 getters/setters are ridiculously slow

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
9 years ago
4 years ago

People

(Reporter: pleasestand, Assigned: jorendorff)

Tracking

({perf})

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(2 attachments)

Reporter

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.13) Gecko/20101203 Firefox/3.6.13
Build Identifier: 4.0b10pre (2011-01-14)

ES5 getters/setters are extremely slow in both Firefox 3.6.13 and the 2011-01-14 nightly. A performance test case I created shows that they are over 20 times slower than normal methods.

This renders them unusable in such high-performance applications as emulators, precisely where they would be most useful. Safari, Chrome, and Opera also have this performance issue.

Reproducible: Always

Steps to Reproduce:
1. Open the linked jsPerf URL.
2. Click on "Run tests."
3. Observe the results.
Actual Results:  
In Minefield:
ES5 way: 2,638,017 ±0.25% 97% slower
Using methods: 123,929,793 ±0.26% fastest

Expected Results:  
There should be little difference between the two.

Comment 1

9 years ago
"where they would be most useful" ? really?
With bug 559653 fixed, we can get to tracing getters and setters after Firefox 4, for Firefox 5 (quarterly releases, yay). The performance will be a lot better where we can stay on trace. Where not, we can use JaegerMonkey inline caching for much better perf too.

This will happen, and soon. But getters and setters are simply not yet used enough to matter on the Web, and I agree with comment 1 -- they are not in any case especially useful or suitable for use in emulators. But I'd love to see a counterexample.

/be
This bug wants an owner.

/be
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: WebJSPerf
Keywords: perf
Assignee

Comment 4

8 years ago
Judging from the microbenchmarky nature of the ...microbenchmark, this is probably just a duplicate of bug 559813. We'll see.
Depends on: 559813
No longer depends on: 559653
Assignee

Comment 5

8 years ago
Transloc labs stumbled across this issue:
  http://labs.transloc.com/?p=21
I can reproduce the issues on jsperf.com, but in the shell with -j -m -p they pretty much go away:
Assignee

Comment 6

8 years ago
I meant to emphasize that Transloc found that getters stink too. But again, only in the browser. Which is rather strange. My money is on jsperf.com sucking, but it could just as easily be us.

Here are the shell -j -m -p speed numbers I meant to paste. Higher is better.

  Direct access: 381
  __defineGetter__: 137
  Get method defined on obj: 151
  ES5 getter: 136
  Prototype getter: 134

A getter isn't much slower than an actual method, then. In the shell.

Taking this bug.
Assignee: general → jorendorff
Assignee

Comment 7

8 years ago
The speed numbers in comment 6 are bogus; these are real.

Direct access: 176
__defineGetter__: 115
Old-fashioned getter: 115
ES5 getter: 115
Prototype getter: 116

Getters are as fast as methods ...in the shell.
Assignee

Comment 8

8 years ago
I get the same numbers in the browser, using this microbenchmark. A getter is as fast as a method.

However running extremely similar code on jsperf.com, we lose. So something in jsperf.com's special sauce somehow gets us to lose. I wonder if all the eval-ing going on there is inhibiting the tracer. Shouldn't.
Reporter

Comment 9

8 years ago
I changed the var t = new TestClass(); part to var t = { ... }; and so on (dropping the prototype getter benchmark as irrelevant in that case), and I get these results:

User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110307 Firefox/4.0b13pre

Direct access: 74
__defineGetter__: 5
Old-fashioned getter: 131
ES5 getter: 5
Assignee

Comment 10

8 years ago
OK, there is a profiler tuning problem. The shell testcase below is fast in the shell with -j, as expected, but with -m or -j -m -p the cases involving a getter are very slow.

----

function TestClass() {
    this._x = 1;
    this.getX = function() { return this._x; };
    this.__defineGetter__('x1', function() { return this._x; });
    Object.defineProperty(this, "x2", {get: function() { return this._x; }});
}
TestClass.prototype = {get x3() { return this._x; }};
var t = new TestClass();

function test(name, fn) {
    var t0 = new Date;
    fn();
    var dt = new Date - t0;
    print(name + ": " + dt.toFixed(1) + " msec");
}

test('_x', function () { var i$ = 1e6; while (i$--) t._x; });
test('getX()', function () { var i$ = 1e6; while (i$--) t.getX(); });
test('x1', function () { var i$ = 1e6; while (i$--) t.x1; });
test('x2', function () { var i$ = 1e6; while (i$--) t.x2; });
test('x3', function () { var i$ = 1e6; while (i$--) t.x3; });
Assignee

Updated

8 years ago
Blocks: 639858
Assignee

Updated

8 years ago
No longer blocks: 639858
Depends on: 639858
Assignee

Comment 11

8 years ago
Filed bug 639858 for the profiler issue. I'm hoping billm will take that.

I'll work on setters -- though I think the profiler issue has to come first.
world's dumbest getter microbenchmark hung and eventually crashed my nightly. Looks like there's problems here!
FWIW, we're faster than Chrome on all the JSPerf Rev. 4 tests.
http://jsperf.com/es5-getters-setters-versus-getter-setter-methods/4
And now this is showing up on robohornet.  We're 10x slower than Chrome on the "ES5 Property Accessors" test there... :(
Blocks: Robohornet

Comment 15

6 years ago
Any chance this will be fixed soon?  The performance of setters/getters is absolutely horrible, take a look at another performance test: http://jsperf.com/object-defineproperty-vs-definegetter-vs-normal

Comment 16

6 years ago
There is another test which eliminates possibility that this is an overhead of calling defineProperty in the loop: http://jsperf.com/object-defineproperty-vs-definegetter-vs-normal/33

Setters/getters are hundred thousand times slower than regular properties.
Something is weird here.  If I do this:

  function ctor() {
    this._x = 0;
  }
  ctor.prototype = {};
  var es5 = new ctor();
  Object.defineProperty(ctor.prototype, "x", {
    get: function() { return this._x; },
    set: function(value) { this._x = value }
  });
  var count = 1000;
  var start = new Date;
  for (var j = 0; j < count; ++ j) {
    for (var i = 0; i < 1000; i++) es5.x += 1;
    es5.x = 0;
  }
  var end = new Date;
  print((end - start) / (1000*count) * 1e6);

then I see that this takes about 15ns per iteration of the innermost loop.

But if I do the defineProperty call on es5 instead of the proto, which is what the linked-to testcase does, I get something closer to 640ns per iteration.

Do we end up not doing sane ICs in that case for some reason?
(In reply to Boris Zbarsky (:bz) from comment #17)
> Do we end up not doing sane ICs in that case for some reason?

When the getter/setter is on the prototype, we bake in a direct call to the getter/setter. We don't handle the case where the property is an own property and the object is not a singleton, so we fall back to an IC. I'm not sure but I don't think we have enough type information to inline a getter/setter call in this case.

Baseline has stubs for calling getters/setters, but these also don't handle the own-property case. However, if I comment out the "obj == holder" check in IsCacheableGetPropCall and IsCacheableSetPropCall, --no-ion becomes very fast. So what we can do is:

(1) Make Baseline's scripted getter/setter stubs handle the own-property case too.

(2) Have IonBuilder query the shape/holder/getter/setter information stored in the Baseline IC to inline a call to the getter/setter in Ion, guarded by a shape guard.
Depends on: 861759
So I've updated the testcase at http://jsperf.com/object-defineproperty-vs-definegetter-vs-normal/35 to include the prototype version; I assume the own property version will get to parity with the prototype version in bug 861759.

What I see is that the prototype version is about 50x faster than the own-prop version, which matches my numbers from comment 17.  It's still about 10x slower than the value-property version...

(Note that I have no idea where the "hundred thousand" times claim in comment 16 comes from: the difference I see between own value properties and own accessors is about 500x, which is bad enough.)
Or is one of the points of comment 18 that even in the prototype situation we still don't inline the getter/setter, so the 10x overhead is from having to do the call instead of just inline updating the slot?
(In reply to Boris Zbarsky (:bz) from comment #20)
> Or is one of the points of comment 18 that even in the prototype situation
> we still don't inline the getter/setter, so the 10x overhead is from having
> to do the call instead of just inline updating the slot?

Yeah, IonMonkey can't inline scripted getters/setters, best case it emits a direct call to the getter/setter. Maybe I can look into it after fixing bug 861759, if I have some spare cycles...

Comment 22

4 years ago
Nightly 38.0a1 (2015-02-13)
Direct access: 273
__defineGetter__: 109
Old-fashioned getter: 281
ES5 getter: 101

Chrome 40
Direct access: 227
__defineGetter__: 266
Old-fashioned getter: 266
ES5 getter: 266
Bug 1129382 improved this a lot (I get the slow script dialog on 35..), but we need bug 1128646 to make own getters as fast as getters on the prototype. Hopefully I can get to that soon.
Depends on: 1129382, 1128646

Comment 24

4 years ago
Direct access: 289
__defineGetter__: 289
Old-fashioned getter: 281
ES5 getter: 277

On http://jsperf.com/object-defineproperty-vs-definegetter-vs-normal/35:
Object.defineProperty - 316,669 ± 0.32%
__defineGetter__ - 318,250 ± 0.25%
Normal - 318,593 ± 0.23%
es3 - 312,843 ± 0.82%
Object.defineProperty on the prototype - 317,512 ± 0.34%
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.