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

import('typescript') is 3x slower than require('typescript') #35574

Closed
AviVahl opened this issue Oct 9, 2020 · 9 comments
Closed

import('typescript') is 3x slower than require('typescript') #35574

AviVahl opened this issue Oct 9, 2020 · 9 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@AviVahl
Copy link

AviVahl commented Oct 9, 2020

  • Version: 14.13.1
  • Platform: Linux. Fedora 32.
  • Subsystem: modules?

What steps will reproduce the bug?

mkdir slow-esm
cd slow-esm
npm init -y
npm i typescript

Now create two files:

a.js:

(async () => {
    const start = Date.now();
    require('typescript')
    console.log(`requiring typescript took ${Date.now() - start}ms`);
})()

a.mjs

(async () => {
    const start = Date.now();
    await import('typescript');
    console.log(`requiring typescript took ${Date.now() - start}ms`)
})()

node a.js outputs:
requiring typescript took 147ms

node a.mjs outputs:
requiring typescript took 440ms

How often does it reproduce? Is there a required condition?

The numbers change +-10ms, but the slowdown is pretty noticable.

What is the expected behavior?

Have a somewhat similar evaluation speed?

What do you see instead?

3x slowdown.

Additional information

I actually converted a commonjs app to native esm... the slowdown was clear, so I decided to make the naive check.

EDIT: Is it because typescript is a commonjs module? tokenization kicks in to find named exports? If so, is there a way to turn it off so I can check the speed without it?

@AviVahl
Copy link
Author

AviVahl commented Oct 9, 2020

Alright, I checked several versions. 14.13.0 has the speed regression. 14.12.0 works properly (~150ms on both files).
#35249 causing a speed regression is my guess.
Not sure if this is an acceptable/known tradeoff.

I'm considering to use commonjs as long as I'm importing other commonjs libraries. Don't want that extra slowdown. :(

@AviVahl
Copy link
Author

AviVahl commented Oct 9, 2020

It should be noted that I have a pretty speedy laptop (i7-7700HQ w/ 32GB of RAM) and I could notice the slowdown even before actually measuring. I wonder how it feels on slower machines.

@aduh95
Copy link
Contributor

aduh95 commented Oct 9, 2020

@nodejs/modules-active-members

@AviVahl
Copy link
Author

AviVahl commented Oct 9, 2020

lodash is also at least 3x slower on my machine (from ~20ms to ~75-90ms).

@MylesBorins
Copy link
Contributor

/cc @guybedford

@MylesBorins MylesBorins added the esm Issues and PRs related to the ECMAScript Modules implementation. label Oct 10, 2020
@guybedford
Copy link
Contributor

Firstly it's worth noting this is only for very large JS files (TypeScript is a single 8.9MB file). Most imports of ESM don't hit this kind of perf.

The issue here is not actually the performance of the JS parser itself, but the fact that when you run a cold start against TypeScript, the v8 optimizer hasn't yet kicked in so the v8 execution of the parser is running in slow mode for the entire 8.9MB TypeScript file.

Running a full warmup cycle against TypeScript before I get:

typescript/lib/typescript.js (8.9MB): 161ms (require)
typescript/lib/typescript.js (8.9MB): 436ms (esm parse cold)
typescript/lib/typescript.js (8.9MB): 228ms (esm parse warm)

Interestingly this points to a workaround - in any large application, import the small dependencies first, then the larger dependencies. This way the parser is warmed up by the time it gets to the larger dependencies.

This is also where using native / Wasm code would provide a benefit because not only do we get a ~50% speed improvement but the code is then also effectively always "warm".

We would have used the Wasm implementation by default, but Node.js cannot support Wasm in core when using the new --jitless flag.

@AviVahl
Copy link
Author

AviVahl commented Oct 10, 2020

Thanks for looking into it. I'll follow the PR...

@MylesBorins
Copy link
Contributor

MylesBorins commented Oct 12, 2020

The WASM update has significantly improved perf:

v14.12.0

test CJS

real	0m0.161s
user	0m0.141s
sys	0m0.018s

test ESM

real	0m0.164s
user	0m0.145s
sys	0m0.019s

v14.13.1

real	0m0.169s
user	0m0.150s
sys	0m0.019s

test ESM

real	0m0.403s
user	0m0.387s
sys	0m0.021s

master /w wasm change


test CJS

real	0m0.163s
user	0m0.142s
sys	0m0.019s

test ESM

real	0m0.249s
user	0m0.238s
sys	0m0.026s

@MylesBorins
Copy link
Contributor

I'm going to go ahead and close this as the wasm change has landed on master and will go out in the next v14.x release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

No branches or pull requests

4 participants