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

Compilation significantly slower than TSC, and massive regression in 1.2.2? #393

Closed
prencher opened this issue Nov 30, 2016 · 34 comments
Closed

Comments

@prencher
Copy link

prencher commented Nov 30, 2016

With TypeScript 2.0.10's TSC, our compilation takes around 30 seconds, with an incremental time around 13 seconds, using the following configuration, and outputting individual files with source maps:

{
    "compilerOptions": {
        "experimentalDecorators": true,
        "forceConsistentCasingInFileNames": true,
        "isolatedModules": true,
        "jsx": "react",
        "lib": ["dom", "es2016"],
        "module": "commonjs",
        "moduleResolution": "node",
        "noImplicitAny": true,
        "sourceMap": true,
        "suppressImplicitAnyIndexErrors": true,
        "target": "es5",
        "typeRoots": [
            "./node_modules/@types",
            "./typings"
        ]
    },
    "exclude": [
        "node_modules"
    ]
}

However, when using ts-loader, the build takes around 70(!) seconds, with incrementals anywhere from 20-30 seconds.

That's bad enough, but with 1.2.2, it now takes around 98 seconds for both the initial compilation, and the incremental.

For reference, here's webpack configuration:

const webpack = require('webpack');
const packageJson = require('./package.json');

module.exports = {
    entry: {
        app: './source/main.ts',
        vendor: Object.keys(packageJson['dependencies']),
    },
    output: {
        path: 'build',
        filename: '[name].js'
    },
    devtool: 'source-map',
    resolve: {
        extensions: ['', '.js', '.ts', '.tsx', '.webpack.js', '.web.js'],
    },
    module: {
        loaders: [
            { loader: 'json-loader', test: /\.json$/ },
            { loader: 'ts-loader', test: /\.tsx?$/, exclude: /node_modules/ },
        ]
    },
    plugins: [
        new webpack.optimize.CommonsChunkPlugin('vendor', 'vendor.js'),
    ],
};

This is on Windows 10 using Node.js 6.9.1 and webpack 1.13.3.

Any idea? I've been completely unable to determine why.

@prencher prencher changed the title Much slower than TSC, and massive regression in 1.2.2? Compilation significantly slower than TSC, and massive regression in 1.2.2? Nov 30, 2016
@johnnyreilly
Copy link
Member

Is it exactly 1.2.2 where the change occurred? What's performance with the previous release?

@prencher
Copy link
Author

Exactly 1.2.2 yes; The numbers above are for 1.2.1 and 1.2.2 respectively.

@johnnyreilly
Copy link
Member

It may be down to this: #377

@johnnyreilly
Copy link
Member

If it's raw speed you're after do you want to try transpile mode?

@prencher
Copy link
Author

Have tried that but it loses all worthwhile type checking. It still takes around 11 seconds on 1.2.1, which is the same speed as tsc doing everything.

@johnnyreilly
Copy link
Member

Ts-loader will never be faster than tsc - that's a guarantee! I don't have suggestions you're going to love I'm afraid; I'd say fixing on 1.2.1 is probably your best bet since we're not planning to pull the Feature added in 1.2.2. it's possible in the future we may look at having 2 processes; one for type checking and one for transpilation but I couldn't say when that might land. PRs that improve the performance of ts-loader are always welcomed though!

@prencher
Copy link
Author

I understand it won't be as fast but 1.2.1 is already 1.5-2x slower than tsc.

1.2.2 is 50% slower than 1.2.1 for the initial compilation and gets seemingly no speed up from incremental at all, making incrementals nearly 5x slower than 1.2.1.

@johnnyreilly
Copy link
Member

I am curious as to what aspect of your project makes performance degrade so much between 1.2.1 and 1.2.2. Certainly my own projects seem OK and they have a significant size. If you can supply a simple repro which demonstrates the significant performance drop off when you use that switch it could be helpful.

@prencher
Copy link
Author

prencher commented Nov 30, 2016

I don't really have any way to do that, our project is relatively large and proprietary. Trying to figure out what causes anything in a build system is a needle in haystack situation.

Here's the diagnostics for raw tsc:

> node_modules\.bin\tsc --diagnostics --watch
Files:            1435
Lines:          242490
Nodes:         1184618
Identifiers:    377157
Symbols:        378205
Types:           96382
Memory used:   593214K
I/O read:        0.16s
I/O write:      15.07s
Parse time:      5.13s
Bind time:       1.01s
Check time:      5.96s
Emit time:      17.88s
Total time:     29.99s
9:34:51 AM - Compilation complete. Watching for file changes.
9:35:00 AM - File change detected. Starting incremental compilation...
Files:            1435
Lines:          242490
Nodes:         1184618
Identifiers:    377157
Symbols:        378205
Types:           96382
Memory used:   865522K
I/O read:        0.00s
I/O write:       0.30s
Parse time:      3.62s
Bind time:       0.00s
Check time:      5.24s
Emit time:       2.92s
Total time:     11.78s
9:35:12 AM - Compilation complete. Watching for file changes.

If there's any easy way to do profile or dump diagnostics for ts-loader itself, I'd be happy to provide that, but it doesn't appear to be instrumented in any way?

@johnnyreilly
Copy link
Member

Having played around a little more it does seem that we may have a performance regression on our hands. However, given that the choice is between losing performance and losing correctness I'm a little torn.

cc @smphhh @jbrantly

BTW we should probably introduce some kind of benchmark tests to catch this sort of thing. Not sure how this would best be implemented; we'd want to cover initial compilation and subsequent compilations. Something to think about.

@smphhh
Copy link
Contributor

smphhh commented Dec 7, 2016

Yeah 1.2.2 is certainly somewhat slower as in general it needs process more files for any given change compared to earlier versions. However, the only way I'd imagine the initial and incremental build times could be the same would be if somehow all the files in the project depend on all other files (directly or indirectly). @prencher does the incremental build time depend on which file you modify at all? Can you try creating a test file that possibly only one other file depends on and see what the incremental build performance is when modifying that? Also, if you enable the Webpack option that lists the files that are built for each recompilation you can see if all the files always end up getting rebuild for some reason.

@johnnyreilly
Copy link
Member

Thanks for chiming in @smphhh - I'd be interested to hear @prencher s input. Having worked with this for a while I am noticing the lag. It would be great if we could improve this somehow.

@prencher
Copy link
Author

prencher commented Dec 7, 2016

On 1.2.1, the compile time -- while still much slower than TSC directly -- does vary based on the file being saved.

On 1.2.2, there seems to be no effect, and incrementals are almost as slow as initial.

We do have a very import heavy code base with rollup files, which I'm sure contributes to having a ton of files for any single incremental. TSC itself seems to be able to handle it pretty well, though.

@johnnyreilly
Copy link
Member

Thanks for commenting @prencher. Although I'm really not keen on the idea I am considering rolling back the change in 1.2.2. The extra correctness is great but the performance cost is heavy; perhaps too heavy.

I need to think about this some more but it's definitely something I'm considering. @smphhh do you have any ideas for how we could improve performance without reverting?

@smphhh
Copy link
Contributor

smphhh commented Dec 7, 2016

My hunch is that there is no quick and easy fix for this, especially if we don't have a repro case. For one of my bigger projects the initial compilation time is around 25 seconds while the incremental compilation time is just 1-2 seconds depending a bit on the file that is modified (and roughly double that with devtool: "source-map". This is with babel loader wired after ts-loader. So clearly there must be something particular about the project of @prencher. I'd still be interested in the output of Webpack when run with --display-modules, that way you can see which files actually get rebuild for each change.

Reverting the change is probably not too bad of an idea, ts-loader users have been able to live with the not-perfectly-correct output since the beginning (I guess) after all. I think improving the performance of ts-loader might require a bit more of an holistic approach with focus on overall architecture etc, which is probably best considered along with the merge with awesome-typescript-loader. BTW @prencher have you tried ATL? Should be pretty easy to test out the performance with that.

P.S. As a bit of a disclaimer, I'm currently not even using ts-loader for the main project I'm working on because the setup is rather complex with React+Relay and the related GraphQL schema stuff. Instead I just have several different watch processes that watch each other's output.

@johnnyreilly
Copy link
Member

This is with babel loader wired after ts-loader. So clearly there must be something particular about the project of @prencher. I'd still be interested in the output of Webpack when run with --display-modules, that way you can see which files actually get rebuild for each change.

My own projects are also using babel-loader to transpile the emitted ES2016 code. @prencher could you provide the output @smphhh suggests please?

Reverting the change is probably not too bad of an idea, ts-loader users have been able to live with the not-perfectly-correct output since the beginning (I guess) after all.

That's true. We have made improvements along the way (I think @Strate added the initial reverse dependency graph stuff which helped).

@prencher
Copy link
Author

prencher commented Dec 7, 2016

ATL for Webpack 1, and the current stable version for Webpack 2 are both about the same as ts-loader, while the current 3.0.0-beta.9 seems to be a whole lot faster, but currently crashes on incrementals on Windows (s-panferov/awesome-typescript-loader#287).

As for the output, we invoke the Webpack API directly and I've not been able to find heads or tails in their docs about making this output show up. If you can assist there, I'd be able to enable it; I can't share the output for the same reason I can't share source code, but I would at least be able to confirm if a large number of files are being rebuilt.

Edit: modules on stats seems to do the trick - it's actually only showing 6 of the 971 modules as built for the chunk when I change a file that causes a 30 second incremental on 1.2.1.

I'll try with 1.3.0 next.

@johnnyreilly
Copy link
Member

As for the output, we invoke the Webpack API directly and I've not been able to find heads or tails in their docs about making this output show up.

I think this is what you need:

https://github.com/webpack/docs/wiki/node.js-api

stats.toString specifically. Some examples of usage here: https://github.com/johnnyreilly/proverb-signalr-react/blob/master/gulp/webpack.js

@johnnyreilly
Copy link
Member

Oh and based on an initial look the revert would be a one line change; so simple to implement if we go that route. (and also would have to disable the constEnumReExportWatch test)

@prencher
Copy link
Author

prencher commented Dec 7, 2016

So with 1.3.0 it builds 833 modules on incremental, compared to 6 on 1.2.1, out of 971 total.

@johnnyreilly
Copy link
Member

Okay that's pretty poor. I'll look to revert the change in the next release. I'll post once it's released (not sure when)

@smphhh
Copy link
Contributor

smphhh commented Dec 8, 2016

@prencher If the performance with stable ATL is about the same as with ts-loader 1.3.0, I assume the number of modules rebuilt is also about the same? What if you only modify the entry file which presumably no other file depends on? You don't happen to have circular dependencies in your project?

My current (somewhat speculative) interpretation of the issue is that:

  • Any performance advantage earlier versions of ts-loader might have had over ATL was due to ts-loader's incomplete handling of deep file dependencies
  • Any advantage development versions of ATL have over current ts-loader is due to the significant work they seem to have done optimizing and re-architecting the loader for performance
  • Dense dependency structure is always gonna cost you in terms of incremental build performance no matter what tool you are using.

@prencher
Copy link
Author

prencher commented Dec 8, 2016

@smphhh ATL pre-3.0 is roughly the same speed as ts-loader 1.2.1. 3.0 appears to be significantly faster from initial build times but again, it fails on incrementals on windows at the moment, so I can't confirm.

I can check on the number of files affected with entry point, as well as ATL for webpack 1 tomorrow.

As for the dense dependency structure, while that is true, TSC itself seems to be able to handle it very well; it's not fast at sub-30 initial and ~12 second incrementals, but it's a far cry from ts-loader which 2-3x slower with 1.2.1. ATL 3.0 seems to be getting close from the numbers I recall.

@smphhh
Copy link
Contributor

smphhh commented Dec 8, 2016

@prencher Ok then I misinterpreted you saying that stable ATL is about the same as current ts-loader, and the above interpretation doesn't hold. As, AFAIK, stable ATL handles deep dependencies properly, this issue might very well be something specific about ts-loader's implementation.

BTW, tsc's watch implementation itself is currently far from optimal, see microsoft/TypeScript#10879

@prencher
Copy link
Author

prencher commented Dec 8, 2016

@smphhh Yeah I am waiting with bated breath for improvements in TSC as well, but even in its current state, it is much faster than any loader - be it ts-loader, atl, or browserify with tsify.

Unfortunately using it directly is not really an option, other than to get an idea of the raw performance of TSC and what the various build systems are adding in terms of overhead.

That all being said, while I would love for improvements to ts-loader, this is more about restoring the performance level of 1.2.1.

@smphhh
Copy link
Contributor

smphhh commented Dec 8, 2016

One more thing, I noticed you are compiling with isolatedModules: true, and ATL seems to handle dependencies a bit differently in that case, see https://github.com/s-panferov/awesome-typescript-loader/blob/master/src/index.ts#L79. That's one thing that probably makes a difference for the performance when comparing ts-loader to ATL, maybe we should do a similar check in ts-loader @johnnyreilly ?

@smphhh
Copy link
Contributor

smphhh commented Dec 8, 2016

So I believe the way to resolve this would be to not define any Webpack dependencies when compiling with isolatedModules: true. This would essentially revert ts-loader's behavior back to pre-1.2.2 in that case, and still allow correct deep dependency handling when compiling with isolatedModules: false. I don't have time to completely think this through right now though so might be missing something.

@johnnyreilly
Copy link
Member

I'd be happy if you wanted to introduce that similar check. If you want to submit a PR to that effect let's give it a whirl. Hopefully @prencher could give it a try on his codebase and confirm. In the meantime I've discovered I need to do a little housekeeping around the test pack now that 2.1 has shipped.

Whilst I'm doing that I'll plan to comment out the 1.2.2 approach and re-add the 1.2.1 approach. This will get us back to having reasonable performance. If your PR bears fruit then we can move to it instead. Hope that's all clear.

@smphhh
Copy link
Contributor

smphhh commented Dec 8, 2016

@johnnyreilly Sounds reasonable. Not sure when I'll have time to work on that though.

@johnnyreilly
Copy link
Member

That's fine - whenever you get a chance we'll appreciate it! :Smile:

@johnnyreilly
Copy link
Member

And I've just learned emoji are case sensitive. 😋

@johnnyreilly
Copy link
Member

#406

@prencher
Copy link
Author

prencher commented Dec 8, 2016

@smphhh Editing the entry point is indeed much faster . We do have some circular dependency chains here and there due to rollup files.

@johnnyreilly Can confirm that 1.3.1 has restored performance for our project. Thanks!

Please feel free to reach out if you need me to test performance in the future, we seem to have a pretty extreme case. 😃

@Venryx
Copy link

Venryx commented Apr 21, 2017

Just thought I'd list my own compile times, for anyone finding this page through search (as I did):

tsc version: 2.1.4
ts-loader version: 1.3.3
webpack version: 1.13.3

Initial compile

TSC + Webpack: (tsc outputs to folder, which webpack watches)

  • 9s (tsc step) + 32s (webpack step)

Webpack with ts-loader:

  • 59s

Subsequent (incremental) compiles

TSC + Webpack:

  • 5s (tsc step) + 3s (webpack step)

Webpack with ts-loader:

  • 5s

So in my case, ts-loader is preferable for dev-mode, since most compiles are going to be recompiles.

However, the same is not true for deploys, since you can't use webpack incremental compile there -- so I use tsc + webpack for deploys, which about halves the total deploy time there. (I keep a background console running with tsc, for incremental compiling whose output is then used by the deploy scripts)

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

No branches or pull requests

4 participants