Closed Bug 626021 Opened 14 years ago Closed 10 years ago

ES5 getters/setters are ridiculously slow

Categories

(Core :: JavaScript Engine, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: pleasestand, Assigned: jorendorff)

References

()

Details

(Keywords: perf)

Attachments

(2 files)

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.
"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
Depends on: 559653
Blocks: WebJSPerf
Keywords: perf
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
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:
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
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.
Attached file world's dumbest getter microbenchmark (deleted) —
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.
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
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; });
Blocks: 639858
No longer blocks: 639858
Depends on: 639858
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
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
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...
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
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
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: