Skip to content
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

Initial build mode build emits twice #43995

Closed
amcasey opened this issue May 7, 2021 · 1 comment
Closed

Initial build mode build emits twice #43995

amcasey opened this issue May 7, 2021 · 1 comment
Assignees
Labels
Domain: Performance Reports of unusually slow behavior Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@amcasey
Copy link
Member

amcasey commented May 7, 2021

From #42173 (comment)

In the course of investigating #42173, I noticed that a clean build of the largest project spent 50 seconds on emit. I picked out the slowest files and noticed that each occurred twice. It turns out that updateShapeSignature triggers a separate emit from the real one (fortunately, with no file writing), which accounts for 22 seconds of the emit time. Note that this was with TS 4.2 - 4.3 skips signature computation on a clean build.

It seems like we could potentially realize a very significant perf win if we could combine the two emit passes. I believe @sheetalkamat has already been thinking about ways to do this.

@amcasey amcasey added the Domain: Performance Reports of unusually slow behavior label May 7, 2021
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 11, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.4.0 milestone May 11, 2021
@andrewbranch andrewbranch added the Rescheduled This issue was previously scheduled to an earlier milestone label Aug 30, 2021
sheetalkamat added a commit that referenced this issue May 17, 2022
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label May 18, 2022
@sheetalkamat
Copy link
Member

Copying this from #49156 (comment)

I am having hard time reasoning about this change.
There are three places currently doing declaration emit/transforms:

  1. Builder: To get signature
  2. DeclarationDiagnostics: Does only transform and gets error
  3. Actual d.ts emit

As part of this i am caching either transform or text (preferring text over transform) and declaration diagnostics so we can reuse it.
I am unable to see lot of benefit of this though. Mainly because:

  1. By default when we everything is clean and not emitted, the first run: Builder is not going to do signature emit any more (Do not calculate signatures if old state is not used #43314)
  2. Change to store signature computed during emit helps with not having big working set for the next iteration: (During emit, if shape signature for the file is same as version, then update it with emitted d.ts file #48616) so this makes it as if in first build we did d.ts emit so number of files on which d.ts emit would be done to calculate their signature by the builder should be smaller. So perf win is possible only in next edits but that is not huge since the file list is smaller.
  3. noEmitOnError is only when we really get declaration diagnostics separately so thats the only place we would save d.ts transform cost.

I havent been able to find repo where this benefits significantly. Numbers remain more or less same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Performance Reports of unusually slow behavior Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants