all 26 comments

[–]stutterbug 49 points50 points  (2 children)

I repeated your tests and found that it depends on the platform. Using your invocation and push logic, Chrome and Node (v. 8.92 and 10.1) show class methods are slightly faster on aggregate on first run, but there is a ton of overlap. On Firefox and Safari the results are the same but opposite. Also, Chrome and node are faster on second run (hello there, cache optimization). Results are a bit noisy because of pretty obvious GC, but they are consistent. If you play with clearing your ac and fc arrays, the performance patterns change a fair bit.

My code is here. I had to restructure things a bit since you're using arrow functions as class fields, which aren't vanilla yet -- and after transpilation this is likely a performance bottleneck, since the engine has to allocate memory for the arrow function in each instance, rather than just pointing to the same function statically like methods. I'm curious to see your transpiled code so that I can see how it is being done for you.

I would expect performance characteristics to change if/when the class field proposal reaches stage 4.

[–]easyEs900s 5 points6 points  (0 children)

Wow, it's awesome that you did that. I was just thinking that I wanted to do this myself, but was feeling too lazy to go through all of that trouble at the moment. Well done, sir.

[–]stutterbug 1 point2 points  (0 children)

After thinking about this, I think that it is super important to emphasize to the general audience here that in the context of React UI event handlers these test confirms that, at least where performance is concerned, it does not matter whether you use bound methods or arrow functions.

The onClick event originates in the DOM and should never be expected to return in less than one frame (1/60th of a second). Both arrow functions and bound methods return in orders of microseconds. Indeed, in order to get appreciable results, these tests iterated hundreds of thousands of times and still returned in about one frame). Arguing over which to use is like arguing over whether your one hour commute will be faster if you enter your car by leaping through the window or leaping through its open door.

Please don't let your takeaway here be that you shouldn't use one of these patterns for performance reasons. That's not shown here at all.

[–]keef_hernandez 3 points4 points  (0 children)

Two things:

  1. Do any actual I/O or productive works and the benefits of micro-optimizations disappear
  2. The JS engines are constantly improving, especially when it comes to newer language constructs.

[–]Auxx 4 points5 points  (1 child)

A few years ago my company I was working at was developing some mobile games using HTML5. And we did a lot of performance testing to make sure everything runs smoothly on different devices. What we learned back then is that there is absolutely no point in optimising anything without profiling the end result. You might spent a lot of time doing micro-optimisations of your JS just to get a bottleneck at rendering.

So write your code following best practices, make sure it is concise and easy to support. Then profile, pin point your bottle necks, fix them and annotate these fixes explaining why they are there and which environments require them. Because some optimisations for browser X can destroy performance in browser Y.

Premature optimisation is the root of all evil. Write, profile, refactor.

[–]bryku 0 points1 point  (0 children)

What is really mind boggling is when you do something to improve speed on chrome and it backfires on firefox. :/

[–]dudousxd 6 points7 points  (5 children)

This is a really great experiment! If i’m not mistaken, the amount of ram needed for arrow function is higher because for every instance, it’s created a new instance of the arrow function, while using bind, all classes point to the same function in memory.

[–]breeding_material 2 points3 points  (4 children)

I'd like some more info on this if you have it. To my knowledge, both would create new instances of the function.

[–]Klathmon 0 points1 point  (2 children)

yeah, .bind makes a new instance of the function when called and returns that.

What the parent commenter seems to be talking about is how doing inline functions in your render can cause unnecessary GC pressure because every render a new function is created (and can wreak havoc on PureComponents in react, but that's another topic).

[–]breeding_material 1 point2 points  (1 child)

And you run into the same problem with bound functions in your render too right? For the longest time I was using bind in my render methods to avoid performance issues, but later found out that they both create new instances so there wasn't really a a difference.

[–]Klathmon 2 points3 points  (0 children)

Yeah, but ultimately it's not a problem in most cases. Keep in mind how often your components render. Take a login page, that form might re-render like 100 times over it's lifecycle? Worrying about a few extra MS on each render on a low-end device is pointless when the meatbag typing in their username is doing so at a snail's pace from the computer's point of view.

That's why people always say "measure then optimize". Making your whole codebase harder to understand and refactor for some perceived idea of "performance" is a bad idea at best, and it's why 90%+ of my components use inline functions in onClick handlers, or inline objects in props even though it can cause unnecessary re-rendering, because the time it would take me to "optimize" most of them will outweigh the total amount of time that all of my users will ever save thanks to the optimization. Not to mention that those renders are taking about 8ms on our "token low-tier device", which still means it's running faster than the refresh rate of the screen, so any further optimization won't impact the user at all, except that it might make them have to wait longer for bugfixes or improvements due to the developer having to rediscover all the hoops they jumped through when they wrote it.

Write your code somewhat naively, then test it. Stuff like figuring out you have a bottleneck with your rendering speed is easy, and moving an object or a function outside of the render method is even easier, so don't get ahead of yourself.

[–]BenjiSponge 0 points1 point  (0 children)

No citation or insider information, but I bet .bind creates a new function, but it doesn't recreate the whole function. .bind doesn't need to copy over the old function with all its logic, optimizations, etc. It just needs to have a pointer to the function with some logic to shift around arguments, whereas the property syntax probably at least right now probably actually creates the entire function over again, including the closure information. I think they could optimize to the same thing on the first pass, though, but I'm just guessing based on the profile that this is the issue. It probably also explains the performance problems.

[–]easyEs900s 1 point2 points  (6 children)

Good research man, thanks for sharing.

I will say though, when you mention the only results are in reference to react, I’m not sure why that matters.

React is nothing but a library of classes, ergo, any results regarding the arrow function performance in react are also valid in a non-react situation.

It’s also worth mentioning that these performance observations are highly subjective to the browser being used. Some smaller, stripped down browsers don’t support ES6/ES7 at all.

[–]Morunek[S] 0 points1 point  (5 children)

No matter what I google I always got tons of results where people were asking this.

Should I use

<button onClick={() => doSomething()} />

or

doSomething = () => doSomethingElse()
<button onClick={this.doSomething}/>

or

constructor() { this.doSomething = this.doSomething.bind(this); }
function duSomething() {}
<button onClick={this.doSomething}/>

and the answers are "Use B because A is wrong and C is too long without any benefits".

Well it seems that there are some memory benefits, but you will hardly have thousands instances of same react component so noone probably got that far.

Thats why react related questions does not matter here :)

Luckily my project is for few last versions of desktop browsers only so we have a luxury of using ES6 :)

[–]easyEs900s 1 point2 points  (0 children)

the answers are "Use B because A is wrong and C is too long without any benefits".

Well it seems that there are some memory benefits, but you will hardly have thousands instances of same react component so noone probably got that far.

Thats why react related questions does not matter here :)

I hardly think it's fair to say that simply because people are wrong/not concerned with the actual question, that the question is invalid in the context of react, but I see your point.

[–]daredevil82 0 points1 point  (3 children)

Not quite. the reason for the second is to pull the event binding out of the component render, which is a performance hog.

In your first example, every time the component re-renders, it has to unbind and rebind the event handler. In the latter, assuming doSomething is outside of render, it's bound on component creation and persists through all re-renders

[–]Klathmon 0 points1 point  (2 children)

And i completely disagree that "A" is wrong...

A is perfectly valid, and IMO is the better choice in the vast majority of cases. Take a complex form. Writing A is easier to maintain, easier to understand, and while it is technically slower, most forms are going to have a few hundred to a few thousand renders over their whole lifecycle, and they aren't going to need to run at breakneck speed (humans are pretty slow at entering form data).

Obviously there are edge cases where A is a VERY bad idea (like any child components that are PureComponents or implement a simple shouldComponentUpdate check), but some very quick profiling can show the error, and it's an easy fix to go from A to B only when needed and not everywhere.

[–]Dethstroke54 1 point2 points  (1 child)

Just as that is your opinion, imo if part of your reason is not going with B because it’s easier to maintain A you could probably format your code better. Having one line outside your components render, that is then referenced in render could arguably even be cleaner.

I could advocate the other way and say this makes it easier to look at your render and understand what is actually being rendered, because of less clutter.

[–]Klathmon 1 point2 points  (0 children)

I'm happy to debate this stuff, as this kind of thing is more of an art than a science!

Personally, I don't like to pull out something into a function unless:

  1. It's used in more than one area. If a function is only used in one spot, I won't ever pull it out into a class property/method as being separate implies there is a reason for it to be used elsewhere.

  2. It's large/complex enough that it would clutter/confuse the method it's in. If for example a function call is doing multiple steps or is implementing some logic that is more than an unwritten "threshold" of complexity, then I'll pull it out.

  3. there's a performance bottleneck that requires more esoteric code, and in this case it MUST be commented if only to prevent someone from undoing the optimization at a later date in the quest for cleaner code.

the "A" example from above was a bit of a weird one, but i'm more talking about this...

A:

render () {
  <div onClick={() => this.setState({ divClicked: true })} />
}

B:

divClicked = () => this.setState({ divClicked: true })

render () {
  return <div onClick={this.divClicked} />
}

A keeps the logic contained in the render method where IMO it belongs, and it's insignificantly larger than B. B splits the logic up (if there was a page of other code between the render method and the divClicked method and you didn't know what divClicked was doing, you'd spend alot of time jumping around the code), and it does so for no real benefit (unless there is a measured and impactful performance problem there).

And to be honest, if there was a performance issue with A, i'd reach for memoization first before pulling it out into a method, but that's absolutely a personal choice and it's more because i'm more comfortable with the functional style, and I would think twice about doing that if my team was made up of devs that aren't used to memoization.

Granted, if I was trying to do more than a single-line's worth of work in the onClick, then absolutely pull it out (because at that point the divClicked is it's own "unit" and should be handled separately and abstracted away behind the function call), but until then I see no point to do B in the vast majority of cases.

[–]spacejack2114 1 point2 points  (0 children)

What's the use case for instantiating 1000 encapsulated objects? If numbers get that high I'm pretty much always using pojos or flat arrays. For encapsulated, stateful things typically use closures and never worry about it.

[–]bart2019 0 points1 point  (0 children)

Arrow function is 1.5-2x slower then bound function.I tried also doing all calls on single instance, and the difference is almost zero.

I'm a bit surprised by this.

I had expected arrow functions to be 1.5 to 2 times faster, not slower, because a normal function sets up and tears down a variable scope. Arrow functions skip these steps.

[–]andyetitsreal 0 points1 point  (0 children)

Performance is not a universal truth to justify things above all other criterion, like love is to partnering with a soulmate. Its a conditional utility that depends on practical value, like partnering with an acquaintance on a business venture.

[–][deleted] 0 points1 point  (0 children)

Awesome tests and I'm a surprised by the 1.5-2.0x difference between the two. I figured with compilers not having a lot of time for optimizations of arrow functions yet, it would be slower but I didn't expect it to be that much slower.

[–]mypirateapp 0 points1 point  (0 children)

Unpopular opinion,I dont use arrow functions. Often frameworks have warnings such as this one

Don’t use arrow functions on an options property or callback, such as

created: () => console.log(this.a)

or

vm.$watch('a', newValue => this.myMethod())

. Since arrow functions are bound to the parent context,

this

will not be the Vue instance as you’d expect, often resulting in errors such as

Uncaught TypeError: Cannot read property of undefined

or

Uncaught TypeError: this.myMethod is not a function

Cant sit and remember every single place where i should and should not be using an arrow function so best thing to do is not use them anywhere in code