Closed chowey closed 1 year ago
it slows down our parser by 5-9%
Wanted to clarify: Are you saying that the use of let
/const
slows down V8 due to the overhead of TDZ, which slows down the TypeScript compiler itself? You're not saying that you're running the TypeScript parser on esbuild's output, right?
I'm not fully sure how difficult it'd be, but it feels like all of the parts are there somewhere for other transformations
It's pretty complex. I'm guessing you're already familiar with the complexities since the TypeScript compiler needs to do the same thing. The main problem is that these bindings are captured differently by closures in loops, which Babel and TypeScript both transform by hoisting the loop body out into its own function. But that requires rewriting a bunch of other things including break
/continue
(potentially multiple levels), return
, yield
, and await
. Doing this requires a lot of care to get all of the edge cases right. I haven't done it because it's a legacy feature that I hoped was getting less and less relevant over time. Here's an example of some of the more complex cases:
async function *example() {
outer: for (let i = 0; i < 3; i++) {
inner: for (let j = 0; j < 3; j++) {
switch (check(k => i + j + k)) {
case 1: break inner
case 2: break outer
case 3: continue inner
case 4: continue outer
case 5: continue
case 6: return a
case 7: await b
case 8: yield c
}
}
}
}
There are ways of avoiding needing to transform loop bodies into functions to implement this. But they would be very strange and unexpected, and no one does them as far as I'm aware. They also likely don't offer any performance benefit. Here are the two ways that I'm aware of:
Injecting a new binding using with
:
// Input
let fns = [];
for (let i = 0; i < 3; i++) {
fns.push(() => i);
}
// Output
var fns = [];
for (var i = 0; i < 3; i++) {
with ({ i: i }) {
fns.push(function () {
return i;
});
}
}
This is terrible for obvious reasons. It would be super slow and wouldn't work in strict mode (which includes all ESM code).
Injecting a new binding using catch
:
// Input
let fns = [];
for (let i = 0; i < 3; i++) {
fns.push(() => i);
}
// Output
var fns = [];
for (var i = 0; i < 3; i++) {
try {
throw i;
} catch (i) {
fns.push(function () {
return i;
});
}
}
This is also terrible. Older versions of V8 (and probably other VMs) deoptimize functions containing catch
, which defeats the goal of improving performance. I also wouldn't be surprised if throw
followed by an immediate catch
doesn't turn into optimal bytecode.
I already have esbuild do a simple form of the let
/const
=> var
transform for top-level variables when bundling to avoid TDZ performance issues. These are trivial to remove because there are no loops involved, and JavaScriptCore has horrible performance with code that uses many let
/const
variables in the same scope due to an O(n^2) implementation of TDZ handling: #478.
I could maybe try to add a mode that just prefers var
when it's trivial to substitute it. So everything that isn't in a loop where the variable is captured would then be converted. I'm guessing that would fix the vast majority of cases. Actually: I wonder which is faster, that or what Babel does. I could see moving loop bodies into separate functions actually slowing things down vs. just using let
in those cases. Haven't done any performance measurements to see which way is slower though.
Wanted to clarify: Are you saying that the use of let/const slows down V8 due to the overhead of TDZ, which slows down the TypeScript compiler itself? You're not saying that you're running the TypeScript parser on esbuild's output, right?
Yes, this is slowing down the TypeScript compiler itself; I have a change set I've been working on for a good while which turns the TS project into modules with imports (rather than namespaces), and I am currently just running esbuild directly on the src
directory. Here's the build change commit on my WIP branch (though it is constantly changing so this commit will go stale).
It's pretty complex. I'm guessing you're already familiar with the complexities since the TypeScript compiler needs to do the same thing. The main problem is that these bindings are captured differently by closures in loops, which Babel and TypeScript both transform by hoisting the loop body out into its own function.
You're totally right; I didn't at all mean to minimize it in my comment. I actually completely forgot about the loop problem. My knowledge of our TS ES5 transform is not perfect without rereading it, and I was only guessing as to the effort given my own understanding of esbuild's internals. Apparently I forgot about things that weren't just variable name shadowing itself (which esbuild internally does have a way to handle via renaming, like foo
vs foo2
, or things like ??
).
At the end of the day, I'm still getting a major performance boost from scope hoisting; that really improves things on the TS compiler where so many helpers are declared in other files. I'd totally understand if you didn't want to tackle this, it's just unfortunate that even today, the TDZ performance hit is so bad.
In any case, it might actually be worth it to skip let
/const
lowering altogether just so we can have things like debugging work properly when transformed loops are involved; this is a problem I already experience today when debugging the TS compiler, where stepping stops working because I've stumbled on a loop that's been turned into a function and the source maps don't line up in quite the right way. In that sense, a mode that could do the easy replacements could be a good middle ground.
If you do have some performance changes you want me to test, I'm happy to do so; it should be straightforward to just go run
whatever branch you have.
EDIT: On second thought, a month later, I don't know if I actually want this; for a perf boost it's good, but, when emitting ESM, export var
is less than desirable for consumers since those would be live bindings. When inside a function scope, it makes more sense, but that is a little more complicated of an optimization.
Perhaps it would be better to simply consider ES2015 the new baseline. AFAIK there are no longer any actively supported browsers out there that do not support ES2015.
The fact it's in the top 3 issues in terms of 👍 says otherwise. If anything, we should have, at least, a branch that shows progress towards that goal.
Perhaps it would be better to simply consider ES2015 the new baseline. AFAIK there are no longer any actively supported browsers out there that do not support ES2015.
There are many quite detailed and nuanced examples found in the comments in this issue of situations and environments in which support for a baseline of ES2015 is not an option. Whether or not and to which degree implementing transforms for syntax that won't work in an ES5 environment is relevant to esbuild is the relevant discussion here, but it is important to have this discussion based on the understanding that there are many situations in which developers still need to target ES5-environments and are therefore forced to use other tools than esbuild, either in isolation or in combination. Being able to target browsers and/or JavaScript execution environments based on whether or not they are being actively supported is a luxury.
I can understand that there are users stuck with older platforms that they need to support. However, there are plenty of mature tools already out there that can compile all the way back to ES5 (such as Rollup). As mentioned before transpiling back to this older syntax incurs a lot of churn on the tooling.
Just saying if there are 2% of the users out there that need this, is it really worth adding all that extra baggage when there are plenty of alternatives to esbuild
for those legacy platforms? It's effort best spent on other things is all I am saying.
I’ll just chime in here and say this isn’t just limited to IE11, there are devices out there that use NetFront, based off WebKit, that are stuck on ES5.
Through the practice of using esbuild in some projects, it is feasible to use swc for es5 conversion, and the performance is slightly degraded, which is still acceptable. I published it as an npm package (https://www.npmjs.com/package/esbuild-plugin-es5).
I'm closing this issue because I do not plan on doing this. My apologies to everyone who wanted esbuild to do this. ES6 was released a long time ago at this point and the cases where you need to target ES5 are rare and getting rarer. I believe that it makes sense to focus esbuild's limited development effort on other things.
This issue being closed doesn't change anything about how esbuild behaves when targeting ES5. The ES5 target is not being removed, and this doesn't prevent esbuild from potentially lowering additional syntax to ES5 in the future. It only means that esbuild will never fully support lowering everything to ES5.
I recognize that there are legitimate cases for running your code as ES5. Some mentioned in the above thread are supporting the few people that still use IE, building code for old smart TVs with outdated JS VMs, or building code for custom JS VMs that are selective about which parts of the JS spec they implement (e.g. Goja or Hermes). If you still want to use esbuild when targeting these unusual environments, then you will have to use an additional tool after running esbuild that transforms esbuild's ES6 output to ES5. Production-quality tools to do this are already written, widely used, and work well. This is not an area that esbuild needs to innovate in.
This is an awesome project, by the way!
Do you have any longer-term plans to support ES5? I ask, because this is the only thing preventing me from using esbuild exclusively for all my transpiling needs.
Specifically, I am still required to support IE11, because it still has official support from Microsoft:
What's worse, the "version of Windows on which it is installed" is Windows 10, which is planned to be the last version of Windows. So IE 11 may not be going away any time soon.
Something like shimport can be used to get ES modules running on IE11, so the only thing missing is ES5 lowering.
Perhaps this is something the community can help with?