Skip to content

really write output file only if needed #13776

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

Closed
wants to merge 2 commits into from
Closed

really write output file only if needed #13776

wants to merge 2 commits into from

Conversation

gdh1995
Copy link
Contributor

@gdh1995 gdh1995 commented Jan 31, 2017

This PR makes tsc only does really writing if an output file's content / BOM has been changed.

If one file is not updated, the log message will be:

  • TSFILE (no changes): R:/Google/a.js

Intention

Although this slows down the compiler a little, I thinks it's a bad design to always write disk a lot everytime I change a single little file in my project.

For example, I'm migrating a JavaScript project to TypeScript, and I want to keep most of the built code same as the old, so I need to call tsc often. Here tsc --watch is useful, but it changes files' mtime frequently and make diff tools and zip do some unnecessary works.

@msftclas
Copy link

Hi @gdh1995, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Jan 31, 2017

Has #6937 already implemented incremental building?

If I have read the comment and code in #6937 correct, output will be skipped iff 1. the complied result changed and 2. someone other than compiler touched the file, regardless of modification.

Indeed reading file again and computing hash will skip some emission in certain case of 2. But I'm not sure whether it will outweigh the cost of reading/hashing relative to a stat call.

@gdh1995
Copy link
Contributor Author

gdh1995 commented Jan 31, 2017

Updated secondly: My second commit makes watcher re-skip iff two fingerprints match, and the watcher will check content on the first time (on initing the fingerprint map).

Old content

@HerringtonDarkholme No, because #6937 only takes effects when --watch is enabled and it still write output files and update their mtime the first time a watching version tsc gets inited. That is, a tsc --watch will update all output files on initing, and then wait external / ts-file changes to select some files.

However, this PR makes --watch always use my new writeFileIfDiff to de-duplicate writing, and when --watch is not enabled, the check will still be done.

Updated: I made a mistake that --watch may not update the fingerprint's updated field, I'm writing a new commit.

Currently this PR has enabled all System instance to check diff, but if you want, we may disable this feature by a switch or on some special platforms.

1. reset fingerprint.updated to false if no more changes found
2. write directly without pre-reading if fingerprint is not matched
3. still check file content during the first time of compiling process
@RyanCavanaugh
Copy link
Member

What issue tracks this change?

In general the updating of an output file's timestamp is a good thing because it allows incremental build tools (e.g. make) to correctly determine which output files are out-of-date with respect to their inputs.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 31, 2017

@gdh1995, thanks for the contribution. as a general policy, please file an issue first before submitting a change. I do not think we would want to take this change.

Most build systems do check time stamps before calling the compiler, e.g. gulp, MSBuild, etc.. so this is redundant, and all users have to pay this price regardless if they care about this feature or not.

@mhegazy mhegazy closed this Jan 31, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants