GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Build improvements by sheetalkamat · Pull Request #48784 · microsoft/TypeScript

Conversation

@sheetalkamat

@sheetalkamat Sheetal Nandi (sheetalkamat) commented Apr 20, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Commits: 16cef4a, 59ad6ef, c8327da

  • During tsc --build and other scenarios where we would do existence check as well as stat which resulted in two stat calls, we just check modified time

Commits: cb7aca3

  • In tsc --build while determining upto date ness status, we now check if referenced projects have errors so that we dont need to go through input and output timestamps in that case

Commits: 89d2d4c

  • During watch especially during polling file watch and in some scenarios during native file-system based watch we stat on files and that time is now passed through as part of file watcher event so tsc --build can use it.

Commits: 6e0c916

  • Apart from this in watch mode we save timestamps for all the watched files. These get updated through watcher event callback or are repopulated if watcher event didn't query the time. So if 2 files changed out of 20 input files then at max we will requery timestamps for those two files (if watch callback didn't already give us updated time)
    Fixes Build mode explicitly checks file stamps in watch mode rather than using file watcher events #45082

Commits: 7cb0f40, 1a8abac, 59f2b5c, dc21283, 5bccee8,

  • If its incremental build, tsbuildInfo alone determines upto date ness of the build. For this we now store changeFileSet, dtsChangeTime and emitSignatures for the output files in the buildInfo when in multi file emit scenario.
  • We store fileVersions, outSignature, dtsChangeTime in single file output scenario.
  • The emitSignatures and outSignature are the d.ts hash of the output that last changed to the disk. Since any composite project already has declaration as must we can store this easily and save having to read files on emit to see if the d.ts file has changed. This makes sure that whether you are in tsc --build or tsc --build --watch mode the state is persisted so we know exactly if we need to really build or just update timestamps.
  • In case of multi file emit scenario, tsbuildinfo timestamp, changeFileSet and errors in the buildInfo determine whether we need to rebuild project or not. This way we dont have to update timestamps for anything except buildInfo when thing appear uptodate but timestamp doesnt reflect that. (eg upsteam project changed but didnt change d.ts files)
  • In case of single file emit scenario, tsbuildInfo timestamp is enough because we cant emit tsbuildinfo without emitting other output files.
  • Earlier we checked input file timestamps and found newest file, now in case of incremental build, we first check buildInfo as missing it, or errors is clear indication of requiring rebuild. After that we check each input file timestamp against buildInfo so that the number of checks can be smaller.
  • Fixes Experiment with storing output timestamps in .tsbuildinfo #46677
  • Fixes tsc --build --watch (with or without --incremental) touches all files causing problems for downstream tooling #46661

Commits: 2f2e370, fcf07f8, 15fe24e

  • In non incremental project, we still need to update timestamps for output files that didnt change as there is no other way to tell if project is upto date or not. But we dont need to read d.ts files to see if .d.ts files have chaned since these projects cannot be referenced by other projects. (composite also means incremental which is must for referenced proejcts)

Commits: 6198fa3,

  • With so much depending on buildInfo, it is now cached and passed through emit as additional data so we dont have to reparse the json or identify which file is tsbuildinfo

Commits: 62c687b

  • We store the hash of files written in bundle emit to ensure that when we try to manipulate the sources for pseudo update (because d.ts of referenced project didnt change so only prepends need to concatenated in right order), we can check if file hasnt changed. This is now needed because bundle buildInfo also stores .d.ts change time and if incremental was toggled between compilations we might use wrong file text and get incorrect d.ts Look at 62c687b for details and 62c687b#diff-6b83fa9d09e38f88c61b24a70aec8668c68e0d8322e8404dbd85c90120f14eeeL1016 shows test that failed without this change

Commits: 4fb6773

  • We also cache output file timestamps so that we dont have to requery as we are the one who write those files.

Commits: 0f7903d, 437619e

  • This also has some refactoring of BuilderState, BuilderProgramState etc structures so its correctly reflected.
  • With this change we also have flags for emitting compiler option key into the buildInfo based on multi file or single file emit

Commits: e6a3ee8

  • tsc --build --force now doesnt query input file stamps (it never did query output file stamps)

Commits: b32d2eb

  • In builder, earlier we use to copy too many structures as part of backup which is internal method to ensure we can restore state if declaration emit fails. So now we are only storing emit state instead of reference map etc

Commits: 303824e

  • Other change in builder is that earlier we use to maintain new delta of changes for affected files new signatures as well as new exported modules map. We use to commit it once all the affected files for the changed file is handled. (This was due to our API usage where we could cancel the operations in between and would still need to be able to reuse the state as much as possible). Now i have changed this to instead maintain copy of old signatures and old export map so that all we do is clear these these maps instead of having iterate through each and update them into the latest copy on commit

Commits: 28a9ff3

  • During tsc --build if file timestamp is changed but its text has not, we read the file and do pseudo update (only in incremental build).
  • Fixes --watch compilation with no change takes 3 seconds #31932
Commits: e4e6672 - Delete only tsbuildinfo instead of all output files when doing clean on incremental project

Here are the numbers from run on solution with large (1994) number of projects.

  main PR Delta % Projects Built Timestamps Updated Total projects
tsc -b 120.21 93.38 -26.83 -22.32% 255 0 1994
tsc -b (Fix one error) 39.13 31.50 -7.63 -19.50% 22 0 1994
tsc -b (Fix multiple errors in files) 43.96 35.74 -8.22 -18.70% 43 0 1994
tsc -b -w 373.25 326.22 -47.03 -12.60% 484 0 1994
Fix error 19.69 17.76 -1.93 -9.80% 11 0 1994
Fix error 61.48 60.71 -0.77 -1.25% 66 0 1994
Fix error 11.53 8.97 -2.56 -22.20% 3 0 1994
Fix error 7.19 6.18 -1.01 -14.05% 2 0 1994
tsc -b 620.07 480.23 -139.84 -22.55% 796 0 1994
tsc -b No change 26.78 19.59 -7.20 -26.87% 6 0 1994
Local Change 36.70 20.14 -16.57 -45.13% 7 477 1994
D.ts change Average 74.60 54.47 -20.12 -26.98% 138 346 1994
tsc -b -w 630.56 557.41 -73.15 -11.60% 796 0 1994
Local Change 8.98 7.52 -1.46 -16.21% 5 131 1994
D.ts change Average 89.21 70.56 -18.65 -20.90% 136 133 1994
tsc -b -w no change 37.58 26.37 -11.21 -29.83% 6 0 1994

On Typescript Code base:

Total Projects In the main PR Delta % Projects Built Timestamps Updated Bundles updated Total Project
tsc -b 60.73 49.03 -11.70 -19.26% 18 0 0 19
tsc -b no change 0.15 0.09 -0.06 -41.56% 0 0 0 19
tsc -b local change 33.42 26.17 -7.25 -21.68% 1 4 9 19
tsc -b dts change 58.87 45.63 -13.24 -22.48% 14 0 0 19

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

DanielRosenwasser, matthandlersux, johannschopplich, electron-space, bhavitsharma, ssnielsen, cherryblossom000, azu, samhh, VladSez, and 14 more reacted with heart emoji U-C-S, IWANABETHATGUY, SchroederSteffen, Julien-Marcou, VladSez, pp0rtal, kasperskei, Mimikkk, auwtch, a11delavar, and 6 more reacted with rocket emoji
All reactions
  • ❤️ 24 reactions
  • 🚀 16 reactions
Sheetal Nandi (sheetalkamat) added 30 commits April 18, 2022 11:41
@sheetalkamat
Use fixed time for vfs so baselining is consistent
a3b5207
@sheetalkamat
Baseline buildinfos
c20613f
@sheetalkamat
Write new file text in baseline even if the file wasnt read on the sh…
c79af2c
…adow
@sheetalkamat
Remove unnecessary debugger statement
40a8c7f
@sheetalkamat
Make sure that incremental correctness is checked with correct writeF…
7377470
…ile so we know buildInfo was written

Also baseline these so its easy to verify the changes
@sheetalkamat
More baselines for the tsbuildinfo
c206ff0
@sheetalkamat
If we are writing dts file and have used file text as version, we can…
fc91aff
… update the signature when doing actual emit
@sheetalkamat
Make WriteFileCallback Api ready for future
396e6c8
@sheetalkamat
Assert that there is only single source file when emitting d.ts file
7084221
@sheetalkamat
Add test
5f02cf3
@sheetalkamat
Renames
c984298
@sheetalkamat
More refactoring
e4bf58a
@sheetalkamat
If we are updating dts of any of the file and it affects global scope…
87a7112
…, everything needs update in signature and dts emit

Fixes #42769
@sheetalkamat
Stacktrace optimization for getModified time in anticipation of using…
16cef4a
… it more than fileExists wherever possible
@sheetalkamat
Baseline getModifiedTime, setModifiedTime, fileExits and directoryExi…
7e65cd3
…ts for experiment
@sheetalkamat
Remove unnecessary write file finger print code since its not used at…
0ff8cb3
… all
@sheetalkamat
Use modified time instead of file existence check
59ad6ef
@sheetalkamat
Remove unnecessary getModifiedTime
c8327da
@sheetalkamat
No need to check for file existence before reading the d.ts file
7817fbf
@sheetalkamat
Do project reference errors before doing input/output file checks
cb7aca3
@sheetalkamat
Dont call getModifiedTimes if dts change
dd96e33
@sheetalkamat
Passdown modified time if queried
89d2d4c
@sheetalkamat
Use modified time passed through the file watching in tsbuild
6e0c916
@sheetalkamat
Handle force build as separate upto date status
e6a3ee8
@sheetalkamat
uptodate status worker to read buildinfo and use it to determine upto…
7cb0f40
… date ness
@sheetalkamat
No need to update output timestamps if buildinfo will determine uptod…
1a8abac
…ateness
@sheetalkamat
Store change file set instead of hasPendingChange to be able to reuse…
59f2b5c
… the information
@sheetalkamat
Add test that shows input file is not present
5c12067
@sheetalkamat
No need to check input time stamp before buildinfo
7734528
@sheetalkamat
Keep buildinfos for lifetime of the solution builder and project
6198fa3
@sheetalkamat
Options solutionDiagnostics instead so that its not too verbose when …
0cf9e30
…printing diagnostics
@sheetalkamat

Sheetal Nandi (sheetalkamat) commented Jun 1, 2022

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) pack this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jun 1, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Sheetal Nandi (@sheetalkamat), I've started to run the tarball bundle task on this PR at 0cf9e30. You can monitor the build here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jun 1, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Hey Sheetal Nandi (@sheetalkamat), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/127230/artifacts?artifactName=tgz&fileId=CCF2A5B002D81E471C51A8DB3E8BAD826F3033E0889AB91ACB94E055FA25C8E002&fileName=/typescript-4.8.0-insiders.20220601.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sheetalkamat
When tsc --build --clean, only remove tsbuildinfo if its incremental …
e4e6672
…build
@sheetalkamat Sheetal Nandi (sheetalkamat) changed the title [WIP] Build improvements Build improvements Jun 2, 2022
@sheetalkamat
Sheetal Nandi (sheetalkamat) marked this pull request as ready for review June 2, 2022 23:44
@sheetalkamat
Sheetal Nandi (sheetalkamat) requested review from Daniel Rosenwasser (DanielRosenwasser), Ryan Cavanaugh (RyanCavanaugh), Andrew Casey (amcasey) and Andrew Branch (andrewbranch) June 2, 2022 23:44
@sheetalkamat

Sheetal Nandi (sheetalkamat) commented Jun 2, 2022

Copy link
Copy Markdown
Member Author

Andrew Casey (@amcasey) Andrew Branch (@andrewbranch) i have highlighted important commits and the change in the description and also have left some comments during the change. Let me know if you need more details.
This PR is basically aggregation of lot of changes that mostly needs to be together.. So minor refactoring can be a separate PR but rest needs to be together for everything to work so cant make this into smaller PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sheetalkamat
Merge branch 'main' into timestamps
842de49
@amcasey

Andrew Casey (amcasey) commented Jun 7, 2022

Copy link
Copy Markdown
Member

Sheetal Nandi (@sheetalkamat) That table's great! When you ran those scenarios, did you do any checks to determine whether the output was the same as before? Also, do you have a similar (or simpler) table for compiling TypeScript itself?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Sheetal Nandi (sheetalkamat) added 4 commits June 7, 2022 12:13
@sheetalkamat
Revert "Options solutionDiagnostics instead so that its not too verbo…
5c07c77
…se when printing diagnostics"

This reverts commit 0cf9e30.
@sheetalkamat
Revert "Report aggregate statistics for solution as well as some solu…
31427f6
…tion perf numbers"

This reverts commit 405d8e9.
@sheetalkamat
Merge branch 'main' into timestamps
0f898f2
@sheetalkamat
Revert "When tsc --build --clean, only remove tsbuildinfo if its incr…
5cd883a
…emental build"

This reverts commit e4e6672.
amcasey
Andrew Casey (amcasey) approved these changes Jun 7, 2022
View reviewed changes

@amcasey Andrew Casey (amcasey) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your offline explanation made sense and I appreciate your thorough perf testing. I added some notes on naming and comments, but mostly I think we should check this in ASAP and see what happens.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/compiler/builder.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/builder.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/builder.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/builder.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/builder.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/diagnosticMessages.json Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/diagnosticMessages.json Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/tsbuildPublic.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/tsbuildPublic.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/compiler/tsbuildPublic.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Sheetal Nandi (sheetalkamat) added 2 commits June 7, 2022 17:20
@sheetalkamat
Comments in the code
5cd5d57
@sheetalkamat
Feedback
624c182
andrewbranch
Andrew Branch (andrewbranch) approved these changes Jun 8, 2022
View reviewed changes
@sheetalkamat
Sheetal Nandi (sheetalkamat) merged commit b8f6488 into main Jun 8, 2022
@sheetalkamat
Sheetal Nandi (sheetalkamat) deleted the timestamps branch June 8, 2022 16:48
@dylang Dylan Greene (dylang) mentioned this pull request Aug 30, 2022
4.8 hasSameBuildInfo can randomly crash tsc.js with "Cannot read properties of undefined (reading 'path')" #50545
Closed
@NickHeiner

Nick Heiner (NickHeiner) commented Aug 30, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

I think this might have caused a regression. In my project, when I introduce errors and build with tsc -p, I get errors, as expected. When I build with tsc --build, it exits very quickly, and shows no errors.

λ ./tools/tvui/node_modules/.bin/tsc -p packages/tsconfig.json 
# ... expected error messages omitted

Found 3 errors in 3 files.

Errors  Files
     1  node_modules/relay-hooks/lib/RelayHooksTypes.d.ts:56
     1  packages/@tvui/fx2-transition/src/utils.tsx:173
     1  packages/@tvui/redux/testing/unitTestingEnhancer.ts:8

tvui on  nth/ts-4.8 [!⇡] via  v16.16.0 took 10s 

λ time ./tools/tvui/node_modules/.bin/tsc --build packages/tsconfig.json --verbose
[3:52:58 PM] Projects in this build: 
    * packages/tsconfig.json

[3:52:58 PM] Project 'packages/tsconfig.json' is up to date but needs to update timestamps of output files that are older than input files


________________________________________________________
Executed in  550.41 millis    fish           external
   usr time  437.83 millis   50.00 micros  437.78 millis
   sys time  194.10 millis  616.00 micros  193.49 millis

--build does seem to be faster, but perhaps it's so fast that it's not doing any work. 😄

I noticed this when doing the TS 4.8 upgrade. I observe this with version 4.8.2.

I'm happy to open a new issue for this if that's preferable.

I first noticed this after using yarn to update type-fest in node_modules.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pp0rtal

Patrick Portal (pp0rtal) commented Aug 31, 2022

Copy link
Copy Markdown

Nick Heiner (@NickHeiner)

I'm happy to open a new issue for this if that's preferable.

New issue is definitively better, check if there is no similar issue.
If you have a standalone minimal project to reproduce it will be gold for maintainers 🤗
Thank you!

NickHeiner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@NickHeiner Nick Heiner (NickHeiner) mentioned this pull request Aug 31, 2022
tsc --build sometimes exits quickly without reporting errors, even when the TS being built is invalid #50568
Closed
@glasser David Glasser (glasser) mentioned this pull request Sep 27, 2022
4.8 regression with tsc --build: error correctly reported, then another execution succeeds #50959
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@andrewbranch Andrew Branch (andrewbranch) andrewbranch approved these changes
@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Awaiting requested review from DanielRosenwasser
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Awaiting requested review from RyanCavanaugh
+1 more reviewer
@amcasey Andrew Casey (amcasey) amcasey approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@sheetalkamat Sheetal Nandi (sheetalkamat)

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Experiment with storing output timestamps in .tsbuildinfo tsc --build --watch (with or without --incremental) touches all files causing problems for downstream tooling Build mode explicitly checks file stamps in watch mode rather than using file watcher events --watch compilation with no change takes 3 seconds

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@sheetalkamat @typescript-bot @amcasey @NickHeiner @pp0rtal @andrewbranch
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.