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

Incremental compilation for TypeScript #6428

Merged
merged 24 commits into from
Jun 24, 2020

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Jun 22, 2020

This PR adds incremental compilation capabilities to internal TS compiler.

Instead of using ts.createProgram() for "compilation" step (during deno startup) ts.createIncrementalProgram() API is used. Due to problems with source map (that are not fully understood) I changed compilation settings to inline source map (inlineSourceMap instead of sourceMap).

Thanks to .tsbuildinfo file that already stores all necessary metadata for compilation I was able to remove our own invention that is .graph files. .tsbuildinfo file is stored alongside compiled source and is used to cache-bust outdated dependencies - it's facilitated by version field. This value is computed in Rust during loading of module graph (basically a hash of file contents).

Please keep in mind that incremental compilation is only used for initial compilation (or dynamic imports compilation) - bundling and runtime compiler APIs haven't been changed at all.

Based on #5094
Ref #4323
Fixes #6194

Did some tests using oak. Changing mod.ts back and forth by adding a comment I got following results:

v1.1.1

▶ time DENO_DIR=./dir1 deno cache mod.ts
Compile file:///Users/biwanczuk/dev/oak/mod.ts
DENO_DIR=./dir1 deno cache mod.ts  5.74s user 0.21s system 192% cpu 3.088 total

▶ time DENO_DIR=./dir1 deno cache mod.ts
Compile file:///Users/biwanczuk/dev/oak/mod.ts
DENO_DIR=./dir1 deno cache mod.ts  5.72s user 0.20s system 193% cpu 3.067 total

this PR

▶ time DENO_DIR=./dir2 ../deno/target/release/deno cache mod.ts
Compile file:///Users/biwanczuk/dev/oak/mod.ts
DENO_DIR=./dir2 ../deno/target/release/deno cache mod.ts  1.59s user 0.09s system 184% cpu 0.913 total

▶ time DENO_DIR=./dir2 ../deno/target/release/deno cache mod.ts
Compile file:///Users/biwanczuk/dev/oak/mod.ts
DENO_DIR=./dir2 ../deno/target/release/deno cache mod.ts  1.58s user 0.09s system 186% cpu 0.899 total

CC @ry @kitsonk

@nayeemrmn
Copy link
Collaborator

Fixes #6194?

@bartlomieju
Copy link
Member Author

Not sure why, but using createIncrementalProgram makes it so // sourceMapURL is not present in emitted files...

@kitsonk
Copy link
Contributor

kitsonk commented Jun 22, 2020

A couple thoughts...

What does it look like for a small workload? What is the initial build for both?

It might not make sense to do this all the time, especially if there is a "small workload" tax, which the previous PR demonstrated there was, or if there is an initial build cost.

@bartlomieju
Copy link
Member Author

A couple thoughts...

What does it look like for a small workload? What is the initial build for both?

It might not make sense to do this all the time, especially if there is a "small workload" tax, which the previous PR demonstrated there was, or if there is an initial build cost.

Checking benchmarks I get
v1.1.1

  "cold_relative_import": {
      "min": 0.5408938322600001, 
      "max": 0.5712274492600001, 
      "system": 0.03312095999999999, 
      "user": 0.75551072, 
      "stddev": 0.009965001714141277, 
      "mean": 0.5583690491600001
    }, 
    "cold_hello": {
      "min": 0.5186779422600001, 
      "max": 0.5689711632600001, 
      "system": 0.03236036, 
      "user": 0.73321252, 
      "stddev": 0.018407609952175195, 
      "mean": 0.5441590419600002
    }, 

this PR

    "cold_relative_import": {
      "min": 0.21213297139500004, 
      "max": 0.23388305439500004, 
      "system": 0.024014207307692312, 
      "user": 0.2871624796153846, 
      "stddev": 0.007451360862256499, 
      "mean": 0.22225263331807693
    }, 
    "cold_hello": {
      "min": 0.213824696395, 
      "max": 0.23650528739500004, 
      "system": 0.027202591923076915, 
      "user": 0.2919498642307693, 
      "stddev": 0.00723504476891191, 
      "mean": 0.22464954816423083
    },

Looks like it's twice as fast 🚀

cli/js/compiler.ts Outdated Show resolved Hide resolved
@bartlomieju
Copy link
Member Author

@kitsonk please review

@ry ry changed the title [WIP] Incremental compilation for TypeScript Incremental compilation for TypeScript Jun 23, 2020
@bartlomieju bartlomieju requested a review from ry June 23, 2020 20:43
cli/js/compiler.ts Outdated Show resolved Hide resolved
serde_json::to_string(self)
}
}

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - waiting on @kitsonk's approval before landing.

@bartlomieju
Copy link
Member Author

bartlomieju commented Jun 24, 2020

Blocked by #6434

@bartlomieju
Copy link
Member Author

@kitsonk I wanted to land #6434 before this PR, but it turns out it can't really be used for ts.createIncrementalProgram:

$deno$/compiler.ts:1367:26 - error TS2345: Argument of type 'EmitAndSemanticDiagnosticsBuilderProgram' is not assignable to parameter of type 'Program'.
  Type 'EmitAndSemanticDiagnosticsBuilderProgram' is missing the following properties from type 'Program': getRootFileNames, getTypeChecker, getNodeCount, getIdentifierCount, and 9 more.

1367       performanceProgram(program);
                              ~~~~~~~

That's a bit unlucky as I really wanted to get in-depth stats for incremental compilation 😢

@bartlomieju bartlomieju added cli related to cli/ dir perf performance related labels Jun 24, 2020
@ry ry mentioned this pull request Jun 24, 2020
4 tasks
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit 3cbd107 into denoland:master Jun 24, 2020
@bartlomieju bartlomieju deleted the incremental_compile branch June 24, 2020 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir perf performance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential performance issue related to the current deps.ts/mod.ts convention
4 participants