-
-
Notifications
You must be signed in to change notification settings - Fork 433
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
Incremental build performance dropped #78
Comments
Hmm... interesting. Thanks for reporting, I'll take a look. |
I did a little profiling with node-inspector and it seems that time is spent calling |
Could be this commit: dc53ef6 Before it was checking for "visited" modules. EDIT: it runs it for every TS file in the project and even |
Yes, seems a reasonable assumption. Also, in my case there were only ~20 files or so, because the rest are still ye olde' JS files, but in a larger project, this could sum up considerably to the overall incremental build time. |
We ran into this very recently. When bundling very large projects with ts-loader, it takes a very long time, which makes iterative development very difficult. |
@use-strict I'm working on reproducing locally. I'm seeing something similar to you, but I just wanted to confirm whether or not the numbers you posted were for the initial build or for a change in watch mode. Here's some numbers that I'm seeing:
In other words, for the initial build both "build modules" and "optimize assets" takes a long time, but a change in watch mode results in a small "build modules" but still a large "optimize assets" |
Alright, so after playing with this a little bit more my hopes at it being a quick-fix are gone. I was able to get the ~800ms "optimize assets" down to ~500ms in my test. That time is spent generating errors, and I'm not sure how much more I can reduce that (but I haven't given up yet). However, I did want to point out there are multiple scenarios & timings to take into account here. There is the "first build" vs an "incremental build" in watch mode, and there is the time to emit output for the modules and time to generate errors. As things stand right now the first build scenario performance is pretty abysmal. In my test it takes 2600ms to generate modules for the first build, but only 85ms to generate modules for an incremental build. At my job we have a project that takes around 20s to generate the first build but less than a second for incremental builds. The point here is that if you're not already using watch mode I highly suggest you do so. That said, I think there are definitely performance gains to be made for the first build and I will look deeper into that. Regarding the time to generate errors, it is a constant for both the first build and incremental builds, but it's also not a great constant. So to recap:
|
Also, I wanted to point out that in my test running |
@jbrantly , initial build time is irrelevant. It's expected to take a long time and it's not a problem as long as I don't have to kill the watch and restart it all the time. Now some remarks and questions:
|
You're probably right, but this is something I want to look into more. I don't know if maybe TS has some optimizations in this area that I'm simply missing out on because I'm doing something silly.
Consider app.ts -> dep.ts. Somewhat related is #52 which deals with the dependency tree. If I'm thinking about things correctly, once that's fixed in the scenario above webpack would consider
That's what I tried yesterday. It took the time in my test from 800ms to 500ms, so while it was better it was not a magical fix out of the box. But like I mentioned, it's possible I'm doing something silly elsewhere that's causing TS not to be optimized for this case. FWIW, I'm using this for my test. |
Just brainstorming potential fixes here: It might be possible to use |
I will investigate more today. Looks like |
I'm still not 100% sure of how the entire process works and there is little documentation to go on, so please just go along me: Webpack starts with an entry point and recursively includes dependencies, following Now if we add ts-loader, this basically means that every *.ts file spawns a ts-loader instance. Even so, ts-loader itself caches a single typescript instance per project. At this point typescript dependencies are different from webpack dependencies. Typescript needs to follow any dependencies for type-checking, and that's including any files that are not necessarily referenced at runtime via a After ts-loader is done with a file, a *.js file results and webpack follows again the dependencies that are For incremental builds, webpack watches all files in the project, so detecting a change is not a problem. The problem is figuring out which files were actually changed since the last incremental build, which webpack cannot help us with because it have no knowledge of them. So to sum things up, we need to build the list of changed files ourselves, using only information coming from the compiler API and/or filesystem, maybe using webpack just to get the paths that are being watched. And we need to do this faster than it takes to get all errors from all files in the project. Or, we need to find another way of getting the errors in a cheaper way. |
Yes
Roughly. To be pedantic the "loader" function is called for each .ts file.
Yes. (per the "instance" loader option and per compiler instance)
Right. Simple case would be a
Yup. By the time the second file is put through the loader TypeScript generally has all of the files for the project loaded. This is actually kind of a bad thing though for various reasons.
Well, to be clear, webpack watches all files that were put through the loader or explicitly added as a dependency using the
I'm not sure this is a problem. Webpack can tell us which files were actually changed. For instance it does so today for .d.ts files. The problem is detecting errors in files that were not actually changed but depended on files that were. And webpack could possibly help us there if we instruct it to do so by explicitly adding the deep tree of dependencies that TypeScript knows about to each file. In other words, instead of calling a blanket As an example, suppose app.ts -> dep.ts. If dep.ts changes, we need to call getDiagnostics on both app.ts and dep.ts even though only dep.ts changed. However, if app.ts changes then we only need to call getDiagnostics on app.ts. This helps the best-case scenario but doesn't address the worst-case. If you change a file that nothing depends on it will be very fast. But if you change a file that everything depends on then it'll be the same performance as today. A downside to this approach is that this list of files will be pushed through the loader again which can increase the "build modules" time, but we might be able to put in some optimizations for that. |
When you refer to using "watch", do you mean |
We are still seeing long load times even when using both
etc, etc. |
From what you are saying, my understanding is that this forces ts-loader to recompile the whole project tree whenever one file changes because of the dependencies? |
@SonofNun15 , both I am actually using a forked dev-server which writes files to disk and only use HTTP to get the stats (errors and warnings) for integrating them with the IDE. So it's basically the same thing. |
OK, I see. So |
Yes it is very quick. One other thing you should keep an eye on, and I don't know what you use in your project, is |
@SonofNun15 When talking about
Kind of, not exactly. ts-loader does get diagnostics from the entire program for every change, but that isn't the fault of how you've got your project structured. |
Current run time numbers are: Bundle size is 1.85 MB We were using |
This is without minification, which reduces the bundle size to ~638KB |
@jbrantly , I opened this issue: microsoft/TypeScript#5192 |
Interesting, thanks. Good to know they're aware of the problem of discovering deep dependencies. I still think there is a good possibility we can fix this now, but long term using an official API would clean things up. I haven't had the time to really explore this yet due to other obligations but I definitely do want to fix. |
FWIW, I've been thinking on this a lot and I believe I know a way to improve the initial build time. I'm planning on trying it out sometime this week. I realize this issue is more about incremental build time but initial build time is important too. I still have ideas on the incremental build time but they're not as fleshed out. I'm also planning on exploring that more. |
It's good to optimize both if possible, although the main bottleneck for the initial build time is probably not ts-loader, but webpack itself. |
Actually there is quite a bit of bottleneck in ts-loader right now. Using tsc to generate js and then just webpacking the js is significantly faster than using ts-loader for large projects. The issue (I believe) is that we are forcing TypeScript to re-synchronize everything pretty much for each file when we should only need to do it once in the common case. That's what I'm planning to address. The idea for improving incremental build time is somewhat dependent on the resulting behavior of the above, so that's why I want to investigate that afterward. But the basic idea hasn't changed: we need to somehow track dependencies and then only get diagnostics for the changed files and their deep dependents. The trick is that "somehow" 😁 |
As a potentially distracting aside, I recently switched from awesome-tsloader to ts-loader and for the same project, my build increased by a minute using ts-loader for an initial build. |
@raybooysen Still a long time 😦 Hopefully I can cut it down a lot more. Do you happen to have a loader that executes before ts-loader? |
Yes we have a few loaders. I wish webpack would output timings for this. |
Actually saying that, I need to confirm if the loaders run before ts-loader. I think not. But this is a full build, I don't have a breakdown of how much time each loader is taking. |
Can we add an option to skip error checking at after-compile? |
Pop up a PR. :) |
@Karabur , the IDE probably shows you errors in the file being currently edited, it won't show you errors in other files, caused by changes in the current file. In other words, it only helps with syntactic errors, not semantic errors. You will get those only by incremental build/ recompilation of the entire project. Because of this, I'm inclined to think that skipping error reporting provides little to no benefit. It may even prove detrimental if you discover errors later, because it will take you more time to figure out when and where you did something wrong. |
@raybooysen ok, will try to make a proper one. I am not sure about all IDE's but my (WS) shows errors for all files, not only for which I am currently edit. I dont need webpack output at all and have no benefit from loader forcing me to to wait 6-15 seconds on every single change to see app updated, but drawback is huge. Anyway it is matter of workflow, and since impact could be significant, it is always better an option to choose. |
IMHO, in the end, it still comes down to the performance of the incremental compilation. Anything larger than a few seconds is not acceptable, for me anyway. Skipping error reporting is not the right solution here. Getting the errors in an efficient way is. I'm not sure in your case if those 6-15 seconds are just typescript compilation or overall time, including time spent by webpack. The usual bottleneck is dependency resolving, so there may be other things causing the large build time. On a side-node, something to watch out for: WS has its own "language service" / compilation logic. It doesn't use the API exposed by typescript and they said it will never do so, for commercial reasons. Although rare, you might find yourself with compilation errors when the IDE says everything is fine and viceversa. WS10 at least was full of this kind of bugs and it was one of the reasons why I moved away from it. |
@Karabur Have you tried using the |
@use-strict of course it is workaround, not the solution. 6-14 is error processing step only. so, having all incremental time about 7 seconds I have like 1 second without. For relativeliy small project. Other people reported more time on that step. |
I've checked code in more details and with |
Right. With |
I should note (probably in the documentation somewhere) that From here
|
Any news or workarounds to make latest version of ts-loader as fast as 0.5.2? |
@panKt Not yet. I believe initial build time is actually faster now than in 0.5.2, but incremental build time still has the issue outlined here. |
@panKt, As a workaround you could use |
I am having this issue too :( incremental compile gets slower and slower as the project gets bigger, I've resorted to using transpileOnly for now but it's bad because I keep having to remember to toggle it to check for errors :( |
Is it getting slower in relation to how much code you're adding? This On 20 January 2016 at 09:40, Wael notifications@github.com wrote:
|
Had the same issue, setting "sourceMap": false in tsconfig.json helped tremendously (much more than expected). |
Setting "sourceMap": false doesn't change much in my case. But setting transpileOnly brings down the compile time from 2.5s to 700ms. This might now seem much, but it is actually 3 times faster. |
FWIW, both |
Hi,
I noticed that our incremental builds started to be considerably slower than they used to, so I ran webpack with
--profile --progress
and older versions of ts-loader to see if there are differences. It seems that v0.5.2 is twice as fast as the latest version. I'm unsure how to interpret the profiling data and the webpack source code is a complete mess, so it doesn't help much in that regard.Here are the stats:
It looks like there is a lot of time spent in "optimize assets". I don't know what's causing this, but I have a feeling that ts-loader is using some extra dependencies now which may have the side-effect of enabling some webpack optimizations that are meant for production, not development.
The text was updated successfully, but these errors were encountered: