Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let slows down performance by 5 times #5815

Closed
AlekseyMalyshev opened this issue Mar 20, 2016 · 8 comments
Closed

let slows down performance by 5 times #5815

AlekseyMalyshev opened this issue Mar 20, 2016 · 8 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@AlekseyMalyshev
Copy link

I wrote this simple performance test recently.

There are two versions of JavaScript:

euler35/euler35.js
euler35/euler35-es6.js

The only difference is that es6 contains "use strict"; on top and uses let instead of var. This simple change however decreased ES6 performance by more than 5 times! I do not believe this should be the case. Full details are here.

  • Version: v5.9.0
  • Platform: arwin Alekseys-MBP 15.3.0 Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; root:xnu-3248.30.4~1/RELEASE_X86_64 x86_64
@vkurchatkin vkurchatkin added the v8 engine Issues and PRs related to the V8 dependency. label Mar 20, 2016
@mscdex
Copy link
Contributor

mscdex commented Mar 20, 2016

This is a known issue, v8 just hasn't optimized some of the ES6 features yet. There's nothing node can really do about that.

@benjamingr
Copy link
Member

V8 hasn't even optimized most of the ES5 features yet.

@Trott
Copy link
Member

Trott commented Mar 21, 2016

for (let...) (which is used in this example) in particular is a problem in v8 (and probably other JS engines) at this time, and should be benchmarked against for (var...) before being used in performance-critical code.

@Trott
Copy link
Member

Trott commented Mar 21, 2016

I'm going to close this as this is an issue that would need to be resolved on the v8 side of things. Feel free to re-open or comment if anyone feels like closing is a mistake. Also, despite being closed, feel free to comment if there are additional questions or observations.

@Trott Trott closed this as completed Mar 21, 2016
@mscdex
Copy link
Contributor

mscdex commented Mar 21, 2016

FWIW there is a PR (#5819) that fixes this.

@Trott
Copy link
Member

Trott commented Mar 21, 2016

@mscdex Correct me if I'm mistaken, but I think that just fixes a few instances of for (let...) that are in Node.js core, but it won't make for (let...) any faster in userland code like in this issue. That will have to wait for v8 to do something awesome.

@AlekseyMalyshev
Copy link
Author

I run some tests and indeed replacing let with var in for loops significantly mitigates the issue. However a version of my test with let is still about 20% slower, than the one with var's. I think the loop somehow multiplies this delay when let it used to define the index.

@Trott
Copy link
Member

Trott commented Mar 22, 2016

@AlekseyMalyshev Here's the relevant issue in the v8 tracker: https://bugs.chromium.org/p/v8/issues/detail?id=4762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

5 participants