Skip to content

Program created with createSemanticDiagnosticsBuilderProgram writes files to disk. #29176

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
0xorial opened this issue Dec 27, 2018 · 8 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@0xorial
Copy link

0xorial commented Dec 27, 2018

Search Terms:
incremental program watcher createSemanticDiagnosticsBuilderProgram

I tried example from here and the version with createSemanticDiagnosticsBuilderProgram still writes files to disk. Also it is much slower than version with createEmitAndSemanticDiagnosticsBuilderProgram.
(project ~140 files, size ~1MB. full compile ~8s, 'semantic' incremental compile ~3s, 'emitAndSemantic' incremental compile <0.5s)

Expected behavior:
No files written to disk. Performance is approximately same between createSemanticDiagnosticsBuilderProgram and createEmitAndSemanticDiagnosticsBuilderProgram

Actual behavior:
Files are written to disk and performance is much worse with createSemanticDiagnosticsBuilderProgram .

Related
tildeio/broccoli-typescript-compiler#57
#20234

@sheetalkamat
Copy link
Member

The default afterCreateProgram for the watchHost is emitFilesAndReportErrorUsingBuilder and that's the reason you are seeing this because the example calls on that (origPostProgramCreate!(program);) You would need to instead Report errors from program your self you do not want to emit the program. Note that SemanticDiagnosticsBuilder is only caching semantic diagnostics and does not handle emit and just redirects it to the original program.

  // Between `createEmitAndSemanticDiagnosticsBuilderProgram` and
  // `createSemanticDiagnosticsBuilderProgram`, the only difference is emit.
  // For pure type-checking scenarios, or when another tool/process handles emit,
  // using `createSemanticDiagnosticsBuilderProgram` may be more desirable.

@0xorial
Copy link
Author

0xorial commented Dec 28, 2018

Unfortunately, it seems that there is a performance-related issue linked to emitting. I tried to copy emitFilesAndReportErrors function from watch.ts and simply remove writing and I immediately had performance drop.
I noticed that there is some performance tracing inside the code. Is there some documentation regarding which tools I can use to benefit from that tracing?

@weswigham
Copy link
Member

The extendedDiagnostics flag enables collection of the data.

@0xorial
Copy link
Author

0xorial commented May 23, 2019

@sheetalkamat, I have spent some time debugging the performance-related part of the issue and found a difference in this method. For SemanticDiagnosticsBuilderProgram this method goes into emit method from program.ts, which ends up checking ALL files of the project, instead of only checking the modified ones.

Is it necessary to have different behavior in this method between EmitAndSemanticDiagnosticsBuilderProgram and SemanticDiagnosticsBuilderProgram ?

@sheetalkamat
Copy link
Member

sheetalkamat commented May 23, 2019

@0xorial This is on purpose. Please see https://github.com/microsoft/TypeScript/blob/master/lib/typescript.d.ts#L4376 to see how the emit works.

// Emits the JavaScript and declaration files.
// When targetSource file is specified, emits the files corresponding to that source file,
// otherwise for the whole program.
// In case of EmitAndSemanticDiagnosticsBuilderProgram, when targetSourceFile is specified,
// it is assumed that that file is handled from affected file list. If targetSourceFile is not specified,
// it will only emit all the affected files instead of whole program

Emit is optimized only for EmitAndSemanticDiagnosticsBuilderProgram which is used to emit and get optimized semantic diagnostics. If you don't want to emit or know which files to emit (like many tools already do eg. webpack loaders) you don't need to have emit logic in there and hence SemanticDiagnosticsBuilderProgram is provided to just optimize the diagnositcs and the emit on that builder is just redirect to actual program.

@0xorial
Copy link
Author

0xorial commented May 23, 2019

Sorry, I still do not understand - what is the purpose of not giving a performance optimization to SemanticDiagnosticsBuilderProgram ? It would work much faster this way, right?

BTW, the reason I dig into this so persistently is because I was working on a webpack integration and I believe this is the fastest integration there is, with performance similar to running tsc --watch. Unfortunately, to achieve that I had to hack around a some of compiler internals: use EmitAndSemanticDiagnosticsBuilderProgram and make a fake writeFile function among others. I would really love to see a public API that would allow integrations with same performance.

@sheetalkamat
Copy link
Member

@0xorial I am not sure what you are asking. If you need performant emit then use EmitAndSemanticDiagnosticsBuilder. Emit is only difference between those two builders and as I mentioned it was intentionally designed that way. Otherwise we could have had just one builder. We are giving control to api users with these two builders to either use builder to determine what files to emit and cache semantic diagnostics or just cache semantic diagnostics when they already know which files to emit by themselves.

@typescript-bot
Copy link
Collaborator

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes. If you're still waiting on a response, questions are usually better suited to stackoverflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants