-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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 builds with .tsbuildinfo-files are not faster than non incremental builds #44305
Comments
I have seen similar results with Pyright's codebase (@jakebailey). It seems like TypeScript still ends up doing type-checking and taking the same amount of time when enabling When I brought this up with @sheetalkamat, she mentioned that TypeScript/src/compiler/builderState.ts Line 108 in 391f9ff
could be the culprit, but we'd need to investigate. |
While it's certainly true that incremental build isn't always faster than clean build (we're working on it), it's not clear to me that that's the issue here. In particular, those two trace (thanks!) do look suspiciously similar. When an incremental build fails to out-perform a clean build, it is generally because the incremental build did different work than the clean build (i.e. it spent too long determining what updates were needed) and not because it did the exact same work. Neither of the traces in the description appears to show buildinfo emit. That seems strange to me, but I remember there were some changes in that area in 4.3 (lazier and possibly hash-based?). It's possible we're just using hashing because noEmit is set - I can't remember the specific rules. Any chance you can share one of your projects? |
If you can't share your code, I would find it helpful to see your tsconfig (feel free to censor names from your project) and tsc commands. Also, the output of |
Thanks for looking into this! I'm sure it has been working before but I can't get it to work in any of my projects any more. I'm unable to share any of the code, so I hope the following helps. Here is the tsconfig for the project I used to create the flame graphs, I just removed some of my customers paths and typings: {
"compilerOptions": {
"baseUrl": ".",
"esModuleInterop": true,
"experimentalDecorators": true,
"importHelpers": true,
"incremental": true,
"jsx": "react-jsx",
"lib": [
"dom",
"es6"
],
"module": "commonjs",
"moduleResolution": "node",
"noUnusedLocals": true,
"noUnusedParameters": false,
"paths": {
"*": [
"node_modules/*",
"node_modules/@types/*",
"typings/*"
],
"Api/*": [
"Api/*"
],
"ClientModel/*": [
"ClientModel/*"
],
"PageComponents/*": [
"PageComponents/*"
]
},
"skipDefaultLibCheck": true,
"skipLibCheck": true,
"strict": true,
"strictNullChecks": false,
"suppressImplicitAnyIndexErrors": true,
"sourceMap": true,
"target": "es5",
"types": [
"bootstrap.native",
"hammerjs",
"jasmine",
"jquery",
"jquery.validation",
"jquery-validation-unobtrusive",
"modernizr",
"slick-carousel"
],
"typeRoots": [
"./node_modules/@types/",
"./typings/"
]
},
"ts-node": {
"transpileOnly": true
}
} Here's two runs with extended diagnostics of the same project.
|
Some initial observations:
|
Time checks are only done with build and not project build |
@sheetalkamat Ah, because we can't know the list of files to check? What are we checking instead? The hashes of the input files listed in buildinfo? |
The path mappings aren't so much importing "Foo" rather than "./Foo" but more like "Foo" rather than "../../../../../Bar/Foo" which is both hard to read and a lot harder to type and different for every file depending how deep it is in the folder hierarchy. I prefer common framework imports to be rooted in a known path. |
I tried the same test with https://github.com/ant-design/ant-design, which is roughly the same size, and the second build stopped after binding (i.e. no check time). @DavidZidar Do you want to give that a shot locally and confirm that it's specific to your project(s) and not (e.g.) your file system? https://github.com/amcasey/ant-design/tree/GH44305
|
@amcasey I tried but I can't seem to install the dependencies.
However, as I stated originally I have tried to observe a measurable difference on two different machines and in Windows and in Linux (in WSL 2) on a different project on the second machine. But maybe I'm doing the same thing that's causing this issue in all my projects? |
@DavidZidar Weird. I thought maybe I was pulling in something from my npm cache, but it works with I don't really see what you could be doing wrong, but I have seen file systems cause up-to-date check issues (e.g. you might have been working on a mounted samba share or something). Since I can't play with your code (totally reasonable) I was hoping to establish a baseline in another way. That branch was based on an older version of ant-design because I happened to know it was in a state where I could build it with 4.3.2. Let me try again with something newer - maybe then you'll be able to install. |
Turns out ant-design |
@amcasey i think |
@DavidZidar Never mind about ant-design - sounds like @sheetalkamat figured it out. 😄 |
@DavidZidar can you try #44394 (comment) when it becomes available to see if that fixes issue for you. Thanks. |
@sheetalkamat Sure, I'd be happy to. But how do I install that particular version? |
Bot will report back on the PR with steps once the build is ready. |
@sheetalkamat It looks like the build failed. |
* Add test showing how setting strict is not preserved in tsbuildinfo Test for #44305 * Handle strict flag when writing tsbuildinfo Fixes #44305 * Apply suggestions from code review Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com> Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Component commits: b6754e4 Add test showing how setting strict is not preserved in tsbuildinfo Test for microsoft#44305 d3b479e Handle strict flag when writing tsbuildinfo Fixes microsoft#44305 cee9f40 Apply suggestions from code review Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
@DavidZidar you can try nightly instead when its published later today |
Component commits: b6754e4 Add test showing how setting strict is not preserved in tsbuildinfo Test for #44305 d3b479e Handle strict flag when writing tsbuildinfo Fixes #44305 cee9f40 Apply suggestions from code review Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com> Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
@sheetalkamat Seems very promising, I did one quick test with a project at home and went from about 2.8 seconds to 1.8 second on the second build. I'll test more on the bigger project on Monday. |
@sheetalkamat Overall performance seems to have improved quite a bit, the second build is also a lot faster now, so I think it seems to work great. Thank you! This is the same machine and same project as previously but with TypeScript version 4.4.0-dev.20210605.
|
Bug Report
TypeScript version 4.3.2
I am unable to see any measurable performance difference using incremental builds using
tsc --noEmit
with or without"incremental": true
incompilerOptions
or the--incremental
CLI argument. Builds take the same amount of time before and after thetsconfig.tsbuildinfo
is created.I have tried this in serveral different projects on both Windows and Linux, the results are the same. I also used the performance tracing feature and the flame graphs look pretty much identical with or without the tsbuildinfo file.
One project takes about 14.5 seconds on my machine without incremental build and about 14.5 seconds with incremental.
Another project takes 3.5 seconds on Windows without and 3.5 seconds with. The same project took 3 seconds on Linux without and 3 seconds with.
First flame graph with no tsbuildinfo
Second build with tsbuildinfo present
🔎 Search Terms
incremental
tsbuildinfo
performance
🕗 Version & Regression Information
⏯ Playground Link
Not possible.
💻 Code
N/A
🙁 Actual behavior
Incremental builds perform the same as non incremental builds.
🙂 Expected behavior
I expect incremental builds to be at least a little bit faster.
The text was updated successfully, but these errors were encountered: