Skip to content

Conversation

jbrantly
Copy link
Member

@jbrantly jbrantly commented Nov 6, 2015

This PR improves initial build performance. Times vary but I've seen a 4s second build go to 2.5s and a 22s build go to 12s.

The issue is that ts-loader too aggressively increases each file's version number which was invalidating TypeScript's cache. There is a TS method called synchronizeHostData that is very expensive to run and is called all over the place. However, it has two ways to short-circuit. One is using getProjectVersion and the other is by reading all of the files' version property. This PR handles both. When the initial call to getEmitOutput is run for the entry file, TypeScript essentially loads up the entire program at that point. All of the files are read from disk, put in memory, parsed, etc. After the entry file is done, webpack calls the loader for each of the dependent files. Previously we would increase the file's version number at this point. However, in most cases, the content of the file is exactly the same. By checking if the content is the same and not increasing the file's version we do not invalidate TypeScript's cache and it can emit the file much more efficiently. Additionally getProjectVersion is used to gain a small performance increase (TS doesn't need to iterate each file's version).

The early call to getCompilerOptionsDiagnostics was also causing synchronizeHostData to run. I moved this to later in the process so it can use the cached data.

One test broke with this change, but I believe I'm just going to update the test. The scenario is that a .d.ts is changed which causes errors in the program. Previously the bundle would be re-emitted in this case, but with these changes it won't be. I don't think this is a problem because nothing in the emit actually changes. It's actually a little more efficient.

This partially addresses some of the points regarding initial build time in #78. It also addresses #56.

@blakeembrey

@use-strict
Copy link
Contributor

Very nice. Good to have this stuff documented.

@blakeembrey
Copy link
Member

LGTM 👍

I've also seen the perf time issues be around 20% slower with ts-loader than typescript-simple-loader, but I haven't looked into it 100% yet. Might be something worth profiling against too, since other loaders haven't implemented these things but seem to still be a bit quicker.

@jbrantly
Copy link
Member Author

jbrantly commented Nov 7, 2015

Thanks!

Might be something worth profiling against too, since other loaders haven't implemented these things but seem to still be a bit quicker.

True. Off the top of my head I bet that's (at least partially) due to program.getDiagnostics vs getting diagnostics for each individual file. Something to look into.

jbrantly added a commit that referenced this pull request Nov 7, 2015
Improve initial build performance by reducing calls to `synchronizeHostData`
@jbrantly jbrantly merged commit d467e59 into master Nov 7, 2015
@jbrantly
Copy link
Member Author

jbrantly commented Nov 8, 2015

Published in v0.6.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants