Bugzilla

David Mandelin [:dmandelin]

Updated

•

13 years ago

Depends on: 559653

Updated

•

13 years ago

Blocks: WebJSPerf

Keywords: perf

Assignee

Comment 4

•

13 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

•

13 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

•

13 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

•

13 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

•

13 years ago

Attached file world's dumbest getter microbenchmark — Details

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.

Kevin Israel

Reporter

Comment 9

•

13 years ago

Attached file dumb getter microbenchmark, showing a problem — Details

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

•

13 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

•

13 years ago

Blocks: 639858

Assignee

Updated

•

13 years ago

No longer blocks: 639858

Depends on: 639858

Ryan VanderMeulen [:RyanVM]

Assignee

Comment 11

•

13 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.

Comment 12

•

13 years ago

world's dumbest getter microbenchmark hung and eventually crashed my nightly. Looks like there's problems here!

Ryan VanderMeulen [:RyanVM]

Comment 13

•

13 years ago

FWIW, we're faster than Chrome on all the JSPerf Rev. 4 tests.
http://jsperf.com/es5-getters-setters-versus-getter-setter-methods/4

Comment 14

•

12 years ago

And now this is showing up on robohornet.  We're 10x slower than Chrome on the "ES5 Property Accessors" test there... :(

Blocks: Robohornet

Yury Selivanov

Comment 15

•

11 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

Yury Selivanov

Comment 16

•

11 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.

Comment 17

•

11 years ago

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?

Comment 18

•

11 years ago

(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.

Updated

•

11 years ago

Depends on: 861759

Comment 19

•

11 years ago

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.)

Comment 20

•

11 years ago

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?

Comment 21

•

11 years ago

(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...

Guilherme Lima

Comment 22

•

9 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