| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
After the upgrade of V8 to a version powered by TurboFan, the recommendation not to use `var` for `for` loops is obsolete.
There was a problem hiding this comment.
LGTM but do you have a reference to something that describes that this became unnecessary?
Sorry, something went wrong.
|
@BridgeAR Well there's https://v8project.blogspot.com/2017/05/launching-ignition-and-turbofan.html... Not sure if there's anything more I could use. |
Sorry, something went wrong.
|
Btw slight typo in the description (and commit):
(the above has var) Just so no one is confused about what this is addressing. |
Sorry, something went wrong.
|
I fixed the typo in the description. The reference to the launch of ignition and turbofan is not a proper sign that let is not causing any trouble anymore though. See 55f9c85 as a example that even const had issues until now. @bmeurer can you give us any further insight if there are any potential deopts left or is let definitely on par with var? |
Sorry, something went wrong.
If I remember correctly this was before v8 6.0 but to be honest I don't know if everything has been resolved in v8 6.0. |
Sorry, something went wrong.
👍 to @lpinca's comment. It was intended to be a hot patch until Crankshaft is no longer used as the main optimizing compiler. The effects of that patch has also been cancelled by 1b54371. |
Sorry, something went wrong.
That was only one of two of those.
When reading the description of the mentioned commit it does not sound like that to me. With the new Ignition+TurboFan pipeline, the instanceof fast-path can be missed if the right-hand side needs a TDZ check, i.e. is const declared on a surrounding scope. This doesn't apply to Node 8 at this point, where it's at V8 5.8, but it applies as soon as V8 5.9 rolls. There's work going on in Ignition (and TurboFan) to optimize those TDZ checks properly, but those changes will land in V8 6.1, so might not end up in Node 8. |
Sorry, something went wrong.
Oops, misread the description. We are at 6.1 now, but let's wait until @bmeurer can explain the performance implication a bit more clearly. |
Sorry, something went wrong.
|
for (let i...) and for (var i...) have different semantics, especially if i is used in a closure or otherwise escapes, like: for (let i = 0; i < 1000; ++i) {
array.forEach(o => o.i += i);
}This will be slow even with TurboFan. And it also has a different meaning than if you'd use var. So it might be a good idea to keep the rule in place, even if only to prevent the bugs. |
Sorry, something went wrong.
|
I think @bmeurer has made a good enough case against this, especially if escape analysis is to be disabled momentarily. Thus, closing for now. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
After the upgrade of V8 to a version powered by TurboFan, the
recommendation not to use let for for loops is obsolete.
Checklist
Affected core subsystem(s)
tools