Last Comment Bug 782913 - Using property getters/setters is dramatically slower than calling the getters/setters directly
: Using property getters/setters is dramatically slower than calling the getter...
Status: RESOLVED WORKSFORME
[games:p?][ion:p?]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 16 Branch
: x86_64 Windows 7
-- major with 6 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
: Jason Orendorff [:jorendorff]
Mentors:
http://jsperf.com/property-overhead
: 954778 (view as bug list)
Depends on: 765454 861759 1128646
Blocks: WebJSPerf gecko-games 885526 JSIL
  Show dependency treegraph
 
Reported: 2012-08-15 03:36 PDT by K. Gadd (:kael)
Modified: 2017-01-24 17:42 PST (History)
31 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Shell benchmark (1007 bytes, text/plain)
2013-06-13 09:47 PDT, Till Schneidereit [till]
no flags Details
Shell benchmark part 2: the rise of the functions (1.54 KB, text/plain)
2013-06-13 11:55 PDT, Till Schneidereit [till]
no flags Details
Setter microbenchmark (974 bytes, text/plain)
2013-07-18 15:53 PDT, Eric Faust [:efaust]
no flags Details
Reduced property performance JSIL test case (1.35 MB, application/zip)
2013-07-18 23:42 PDT, K. Gadd (:kael)
no flags Details

Description User image K. Gadd (:kael) 2012-08-15 03:36:29 PDT
At present, if you create a property (using Object.defineProperty) that has a getter/setter, accesses to that property are tremendously slow - 10-100x slower than calling the getter/setter function directly, if not worse.

Having some amount of performance penalty for the indirection is reasonable, but at present this means that getters are basically unusable in code that needs to be performant.

In particular, it's important for getters/setters to be performant because they allow writing better code. For example:

foo.Property++
instead of
foo.set_Property(foo.get_Property() + 1);

getters/setters are also of value for lazy evaluation of expressions because they provide a simple way to expose lazy evaluation without changing the interface of a type. The alternative (using a function) means that now the lazily evaluated expression must always be a function call, even if it doesn't always require lazy evaluation and can be a constant.
Comment 1 User image Luke Wagner [:luke] 2012-08-15 10:35:42 PDT
I've also seen heavy getter usage in other hand-written JS game frameworks.  It is even reasonable to assume that they are fast since the identity of the callee is determined by the shape which is actually better than a normal callprop (at least, w/o branding).
Comment 2 User image David Mandelin [:dmandelin] 2012-09-04 12:27:45 PDT
Some data:

I just took another look at the JSPerf benchmark. The archived results show Chrome 21 (18kops/s, 2x slower than vanilla property), Chrome 22 (28kops/s, 3x slower than vanilla), and Firefox 18 (21kops/s, 2x slower than vanilla) doing the best at getters and setters.

(I note that Firefox 18 shows a lower rate overall than Fx 16, which may be something to look into, although Fx18 is only one sample.)

I tested with IM and got 20kops/s, which is 2x slower than vanilla. 

Remarks/Questions:

Kevin, what do you think browsers, and Firefox in particular, should aim for with getters/setters? All other browsers are very slow at these tests, and probably will be so for some time. Would you just go ahead and target Chrome and Firefox more if they were the only browsers good at this, or would you avoid it in order to work in more browsers?

The second question, for the JIT people, is how fast can we hope to make this? In this case (which is probably about the usual), the property is defined in the prototype and uses a getter, which is a function that just returns a property. With perfect static information, it seems we could reduce this to the same thing as a property get. Can we get perfect static information? If not, how many checks do we need and how fast can they be made?
Comment 3 User image David Anderson [:dvander] 2012-09-04 12:35:56 PDT
Neither CS nor IM can inline through getters/setters yet, but both boil away the property lookup and bake in a call. Inlining through them in IM is blocked on bug 765454.
Comment 4 User image K. Gadd (:kael) 2012-09-04 13:41:49 PDT
I think some performance penalty is fine here, as there are many cases where you can just replace your property getter/setter accesses with ordinary function calls. But there are use cases where you absolutely need them - lazy evaluation, and interfaces that originally had properties instead of getter/setter functions, for example. I think it's important to support those use cases in the long run.

In the short term it may help to try and educate people about this particular performance issue, since it's hard to spot from profiling in V8 or SpiderMonkey.
Comment 5 User image Vladimir Vukicevic [:vlad] [:vladv] 2012-09-10 12:25:25 PDT
This would be really nice for API design purposes -- getters/setters can be pretty clean instead of writing functions, so it would be nice to have no perf loss here.
Comment 6 User image Anthony Pesch 2013-04-11 11:03:07 PDT
I just ran into this last night and it took a while to figure out.

I'm working on a game and I'd recently swapped out some of my hand translated libraries with emscripten compiled libraries along with a views lib that wrapped emscripten's heap. The JavaScript views objects contained a lot of Object.defineProperty's to handle reading / writing at the appropriate place in the heap.

This worked fine in the latest Chrome, but ran _terrible_ in FF nightly. After swapping out all of the property definitions with with get_*() and set_*() everything worked great, however it's not very elegant for this application.
Comment 7 User image K. Gadd (:kael) 2013-04-11 11:09:21 PDT
I should also mention that I am relying on getters/setters for a partial Binary Data shim that I'm using in some of my experiments with emscripten interop and typed array heaps. It's much harder to eliminate the use of getters/setters in this code because I need a way for packed structs backed by a typed array to look and behave like packed structs that live entirely in a JS object. 

It's not possible for my codegen to just figure out 'oh, these are getters/setters' and insert method calls - the only alternative would be to introduce getters/setters for *all* values and then always call the methods, which would likely be a perf loss overall and would complicate the generated code.

In SPS profiles most of the problem seems to be the transition out of the jitcode into GetPropertyOperation/SetPropertyOperation and kin, then back into the actual getter/setter code. I see a lot of time spent there. Inlining would be great too, but I'm not even sure it's necessary - perf might be adequate without it.
Comment 8 User image Michael Bebenita [:mbx] 2013-06-13 01:10:37 PDT
This is also very important for Shumway since it makes heavy use of getters/setters to emulate ActionScript3 semantics.
Comment 9 User image Till Schneidereit [till] 2013-06-13 09:47:16 PDT
Created attachment 762109 [details]
Shell benchmark

I did some more - shell-based - benchmarking of this.

Using a getter is, for me, exactly 1.5x as slow as either using a method or directly accessing the field: with the attached script, I get 28ms for direct access and method call vs. 42ms for getter access.

This isn't surprising, given that, as per comment 3, method calls are inlined, while getters/setters are not. Once bug 765454 is fixed, we should have speed parity for inlineable (i.e., most) getters/setters.
Comment 10 User image Till Schneidereit [till] 2013-06-13 11:55:50 PDT
Created attachment 762196 [details]
Shell benchmark part 2: the rise of the functions

So, there's more to this story - and it's not pretty.

The attached version of the benchmark simply wraps the entire script into a function and executes that. Well, and it unrolls the loops 10x to reduce the influence of loop overhead.

Output on my machine:
direct access: 4
getter access: 1537
method access: 7

That is, the getter version is more than 380x as slow as the field version and more than 200x as slow as the method version.

If I move just the definition of obj to the toplevel, the getter version goes down to 32ms, with the others staying the same.

I'm sure there're some optimizations going on, for the field and method versions. I guess they just hoist the (inlinde, in the method case) field access, leaving only a register access in the loop. Or something.

I don't get why the getter version gets so much slower, however.

I'm sure that there's some optimization going
Comment 11 User image Nicolas B. Pierron [:nbp] 2013-06-17 03:07:17 PDT
(In reply to Kevin Gadd (:kael) from comment #7)
> In SPS profiles most of the problem seems to be the transition out of the
> jitcode into GetPropertyOperation/SetPropertyOperation and kin, then back
> into the actual getter/setter code. I see a lot of time spent there.
> Inlining would be great too, but I'm not even sure it's necessary - perf
> might be adequate without it.

This should not happen, otherwise this means that instead of producing a call, we go through the VM function, which is extremely bad.


(In reply to Till Schneidereit [:till] from comment #10)
> Created attachment 762196 [details]
> Shell benchmark part 2: the rise of the functions
> 
> Output on my machine:
> direct access: 4
> getter access: 1537
> method access: 7

FYI, this is likely related to the fact that IonMonkey does not yet inline getters/setters.  The problem is that we do not have a good way to represent a call issued from a getter/setter and resume it correctly in the baseline code / interpreter.

If we make any modification, they should be near IonBuilder::getPropTryCommonGetter, and identically for setters.
Comment 12 User image Eric Faust [:efaust] 2013-07-17 16:26:52 PDT
With the WIP patch I posted in bug 765454, I am seeing the following numbers from the microbenchmarks above:

Shell Benchmark: The first:

Before:
direct access: 27
getter access: 39
method access: 26

After (with patch):
direct access: 26
getter access: 27
method access: 26


Shell Benchmark Part 2: Electric Boogaloo:

Before:
direct access: 3
getter access: 26
method access: 3

After:
direct access: 3
getter access: 5
method access: 3

Till, the third benchmark you passed me (referenced in comment 10) is so slow because it's interacting with the Ion ICs the whole way, which means vm-based adjudication of the getter at every call. I will investigate that as my next priority.
Comment 13 User image Till Schneidereit [till] 2013-07-17 16:52:56 PDT
I've long gone to bed and am fast asleep. That can't keep me from quickly saying: Horray!
Comment 14 User image K. Gadd (:kael) 2013-07-17 18:50:15 PDT
Those are really great numbers. Once this lands, V8 and SpiderMonkey will both have performant properties, a bunch of my data structures will be faster, and I can turn off a whole pessimization pass in my compiler. (:

Let me know if you'd like me to build with that patch applied and test some apps against it.
Comment 15 User image Eric Faust [:efaust] 2013-07-18 15:53:35 PDT
Created attachment 778133 [details]
Setter microbenchmark

With the new WIP in bug 765454, we add setters!

With the microbenchmark attached, we see the following numbers:

Before:

direct access: 11
setter access: 35
method access: 11

After:

direct access: 11
setter access: 12
method access: 11

So, it seems that we also get good setter times after inlining.

Kevin, can you take the patch for a spin and see what you see? I hope it's stable enough to do whatever you want with it, and I'm interested to see how often we reap benefits in other codespaces that our simple microbenchmarks.
Comment 16 User image K. Gadd (:kael) 2013-07-18 17:41:05 PDT
Hi, I did a release build of the latest m-c revision with this patch applied, but performance is absolutely miserable (really long GC pauses, too). What mozconfig should I use to test this? Do I need to force PGO on or something?
Comment 17 User image K. Gadd (:kael) 2013-07-18 23:42:12 PDT
Created attachment 778324 [details]
Reduced property performance JSIL test case

Here's a simple property getter performance test case using JSIL. The results are... weird.
Comment 18 User image Till Schneidereit [till] 2013-12-30 02:57:26 PST
*** Bug 954778 has been marked as a duplicate of this bug. ***
Comment 19 User image Boris Zbarsky [:bz] (still a bit busy) 2017-01-24 10:01:58 PST
kael, is this still an issue?  I tried running your big .zip testcase, but I'm not sure what the output from it means or what output is expected....
Comment 20 User image K. Gadd (:kael) 2017-01-24 16:20:36 PST
This has been fixed, AFAIK.
Comment 21 User image Boris Zbarsky [:bz] (still a bit busy) 2017-01-24 17:42:25 PST
Thank you for double-checking!

Note You need to log in before you can comment on or make changes to this bug.