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

Opportunity: Improve compiler performance by avoiding property accesses #39247

Closed
evanw opened this issue Jun 24, 2020 · 8 comments · Fixed by #51387
Closed

Opportunity: Improve compiler performance by avoiding property accesses #39247

evanw opened this issue Jun 24, 2020 · 8 comments · Fixed by #51387
Assignees
Labels
Domain: Performance Reports of unusually slow behavior Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@evanw
Copy link
Contributor

evanw commented Jun 24, 2020

TypeScript Version: 3.9.5

Search Terms: compiler performance namespace run-time property access

Code

I have looked into TypeScript compiler performance in the past and I've always wondered why the TypeScript compiler code uses namespaces heavily, since run-time property accesses are slower than statically-bound identifiers in JavaScript.

I finally got around to writing a proof of concept:

let fs = require('fs')
let tscPath = __dirname + '/node_modules/typescript/lib/tsc.js'
let tsc = fs.readFileSync(tscPath, 'utf8')

let vars = new Set()
tsc = tsc.replace(/\bts\.(\w+)/gm, (_, id) => {
  id = `ts_${id}`
  vars.add(id)
  return id
})
tsc = `var ${[...vars].join(',\n  ')};\n${tsc}`

fs.writeFileSync(tscPath, tsc)

This post-processes tsc.js to convert run-time property accesses into statically-bound identifiers. I ran this on the Rome code as a benchmark of a reasonably-large TypeScript code base and got a noticeable speed boost:

Before After Difference
Time to run tsc.js 29.5s 27.5s 2s faster

Each time is the best of 5 runs, and each run was time node node_modules/typescript/lib/tsc.js -noEmit -project rome.

The TypeScript compiler is leaving some performance on the table by using run-time property accesses where it could use statically-bound identifiers instead. I'm sure you are using namespaces for code organization for good reasons, but it does come at a cost. An alternative to namespaces that might have less performance overhead could be to use ES6 modules and bundler, for example.

I'm posting this issue because I think the results of this experiment are interesting. The issue tracker seemed like the most appropriate place to post this. Feel free to just close this issue if you'd like.

@steakscience
Copy link

Gotta go fast!

@DanielRosenwasser
Copy link
Member

@amcasey @ahejlsberg @weswigham, it'd be nice to see if #38510 gives any observable performance boost.

@mohsen1
Copy link
Contributor

mohsen1 commented Jun 25, 2020

#35561 is trying to use modules instead of namespaces

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 25, 2020

Whoops, that's the right link, thanks @mohsen1.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 25, 2020
@amcasey
Copy link
Member

amcasey commented Jun 25, 2020

Cool! @mohsen1 I assume that if you run the tests in the other order (i.e. no-namespaces first), you get the same result? I've gotten into the habit of discarding the first run whenever I do perf testing to minimize the effects of disk caching.

@amcasey
Copy link
Member

amcasey commented Jun 26, 2020

I did my own run against ant-design and saw a ~5% reduction in total compilation time. Pretty impressive. I have yet to compare the compiler output (i.e. to confirm it hasn't changed).

@DanielRosenwasser
Copy link
Member

This should go hand-in-hand with the modules work for 4.6. I would hope that a bundler with scope hoisting will allow us to avoid property accesses/function calls that serve no purpose.

@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Nov 2, 2022
@jakebailey
Copy link
Member

For those following this issue, #51387 will fix this (though it was later than 4.6 😄).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Performance Reports of unusually slow behavior Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants