perf: use emit from swc instead of tsc #15118

dsherret · 2022-07-07T22:06:24Z

This change separates emitting from type checking and now only emits with swc. It also separates the emit cache from the type checking cache.

Figure out coverage issue... :( -- Maybe will be fixed by fix(source-map): better handling of multi-byte characters swc-project/swc#5153
Tests for deno run --no-check=remote / deno check --remote does not report remote errors if a local check was already performed successfully #14632
Tests for not type checking multiple times when switching configs

Part of #13302

Closes #14632
Closes #8706

cli/emit.rs

Co-authored-by: Nayeem Rahman <nayeemrmn99@gmail.com>

cli/emit.rs

kitsonk

a few thoughts

cli/cache/check.rs

kitsonk · 2022-07-12T00:29:39Z

cli/cache/check.rs

+}
+
+impl TypeCheckCache {
+  pub fn new(db_file_path: &Path) -> Result<Self, AnyError> {


meta: we have never managed the sizes of caches, and having an unbounded database it raises the question of quota management and how we would manage an LRU and size constraints.

I didn't think it was worth the extra complexity, but maybe it is? The previous tsbuildinfo cache was also unbounded.

I just wrote some tests for this and for the same entrypoint type checked multiple times with a unique change each time, the end result was this (the hashes don't take up a lot of room):

1,000 unique type checks - 92KB (size of sqlite db)

5,000 unique type checks - 316KB

10,000 unique type checks - 600KB

Also, I didn't notice any slowdown in speed as the database size grew.

However, once you start type checking different root entry points, then it starts to grow a lot because it stores the tsbuildinfo as well. For example, this is with unique entrypoints:

1,000 unique entry points - 6.1MB

5,000 unique entry points - ~30MB

10,000 unique entry points - 59.5MB

For this, I also didn't notice any slowdown as the database size grew... it remained exactly the same. SQLite seems quite fast.

I think it would be unlikely for someone to even have more than 100 root entrypoints before upgrading their CLI version. Let me know your thoughts.

Yeah, I don't disagree, it was more of a meta concern that this plus other stuff likely needs better quota management.

cli/cache/check.rs

cli/cache/common.rs

kitsonk · 2022-07-12T00:42:11Z

cli/cache/common.rs

+/// Runs the common sqlite pragma.
+pub fn run_sqlite_pragma(conn: &Connection) -> Result<(), AnyError> {
+  // Enable write-ahead-logging and tweak some other stuff
+  let initial_pragmas = "


question: how were these determined?

It's from here:

deno/ext/webstorage/lib.rs

Lines 69 to 81 in 56d0ca7

// Enable write-ahead-logging and tweak some other stuff.

let initial_pragmas = "

-- enable write-ahead-logging mode

PRAGMA journal_mode=WAL;

PRAGMA synchronous=NORMAL;

PRAGMA temp_store=memory;

PRAGMA page_size=4096;

PRAGMA mmap_size=6000000;

PRAGMA optimize;

";

conn.execute_batch(initial_pragmas)?;

conn.set_prepared_statement_cache_capacity(128);

Divy let me know about these in the last pr: #13462 (comment)

Ok, how were they determined?

cc @littledivy -- do you know?

cli/cache/emit.rs

cli/tsc/compiler.d.ts

nayeemrmn · 2022-07-12T02:00:26Z

cli/emit.rs

-        return false;
-      }
+      hasher.write(specifier.as_str().as_bytes());
+      hasher.write(code.as_bytes());


Would it be better to use precomputed hashes of the code here instead of the code itself?

Maybe. I wouldn't be surprised if retrieving and storing those hashes is slower than just doing the hash though. xxhash is very fast.

kitsonk

LGTM

kitsonk · 2022-07-12T03:01:42Z

Oh, I forgot, how is the output of deno info impacted?

dsherret · 2022-07-12T03:13:17Z

Oh, I forgot, how is the output of deno info impacted?

@kitsonk no, I don't believe so because only the tsbuildinfo file was moved here.

In an upcomming PR for #13302 the emit path will be removed (source will stay).

kitsonk · 2022-07-21T04:18:25Z

cli/emit.rs

        "experimentalDecorators": true,
        "incremental": true,
        "jsx": "react",
+        "jsxFactory": "React.createElement",


Adding these here appears to cause issues when "jsx" is set to "react-jsx".

Hmm… I can’t remember why I added these :/ (but it’s late here)

Ref: #15263

dsherret added 6 commits July 7, 2022 18:04

refactor: use emit from swc instead of tsc

20b137f

Maybe fix lint.

5a73fc5

dry

03c6ca6

Add test for type checking local then remote

8bab64a

Test switching configs.

a174c17

Merge branch 'main' into emit_swc_instead_of_tsc

824133a

nayeemrmn reviewed Jul 9, 2022

View reviewed changes

cli/emit.rs Outdated Show resolved Hide resolved

dsherret and others added 6 commits July 11, 2022 14:13

Merge branch 'main' into emit_swc_instead_of_tsc

ec1b63e

Merge branch 'main' into emit_swc_instead_of_tsc

5121037

Merge branch 'main' into emit_swc_instead_of_tsc

6aea776

Update files.

a3a6850

Update cli/emit.rs

7f0eed8

Co-authored-by: Nayeem Rahman <nayeemrmn99@gmail.com>

Always type check with --reload

d7059d4

dsherret marked this pull request as ready for review July 11, 2022 23:21

dsherret requested review from kitsonk and bartlomieju July 11, 2022 23:21

nayeemrmn reviewed Jul 11, 2022

View reviewed changes

cli/emit.rs Show resolved Hide resolved

dsherret added 3 commits July 11, 2022 20:23

Add back reload exclusions

76b08b9

Merge branch 'main' into emit_swc_instead_of_tsc

982fd4a

Format. Sigh...

ecd66ab

kitsonk reviewed Jul 12, 2022

View reviewed changes

dsherret added 2 commits July 11, 2022 21:04

Fix merge mistake

6182b3d

Copyright headers.

ed67569

nayeemrmn reviewed Jul 12, 2022

View reviewed changes

kitsonk approved these changes Jul 12, 2022

View reviewed changes

Simplify emitBuildInfo call.

a453882

nayeemrmn mentioned this pull request Jul 12, 2022

fix(cli): Expand tsc roots when using checkJs #15164

Merged

dsherret changed the title ~~refactor: use emit from swc instead of tsc~~ perf: use emit from swc instead of tsc Jul 12, 2022

dsherret merged commit 0c87dd1 into denoland:main Jul 12, 2022

dsherret deleted the emit_swc_instead_of_tsc branch July 12, 2022 22:58

dsherret mentioned this pull request Jul 13, 2022

Fix bundle type checking benchmark #15178

Closed

justjavac mentioned this pull request Jul 13, 2022

feat(semver): add "semver" module denoland/std#2434

Merged

dsherret mentioned this pull request Jul 13, 2022

fix: fallback to no type checking cache when db file can't be created #15180

Merged

kitsonk reviewed Jul 21, 2022

View reviewed changes

ayame113 mentioned this pull request Jul 30, 2022

Uncaught SyntaxError: The requested module 'https://deno.land/std@0.144.0/http/file_server.ts' does not provide an export named 'ServeDirOptions' denoland/deploy_feedback#252

Open

kt3k mentioned this pull request Aug 30, 2022

fix(typescript): update type-checking info denoland/manual#379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: use emit from swc instead of tsc #15118

perf: use emit from swc instead of tsc #15118

dsherret commented Jul 7, 2022 •

edited

Loading

kitsonk left a comment

kitsonk Jul 12, 2022

dsherret Jul 12, 2022 •

edited

Loading

kitsonk Jul 12, 2022

kitsonk Jul 12, 2022

dsherret Jul 12, 2022

kitsonk Jul 12, 2022

dsherret Jul 12, 2022

nayeemrmn Jul 12, 2022

dsherret Jul 12, 2022

kitsonk left a comment

kitsonk commented Jul 12, 2022

dsherret commented Jul 12, 2022

kitsonk Jul 21, 2022

dsherret Jul 21, 2022

kitsonk Jul 21, 2022

	// Enable write-ahead-logging and tweak some other stuff.
	let initial_pragmas = "
	-- enable write-ahead-logging mode
	PRAGMA journal_mode=WAL;
	PRAGMA synchronous=NORMAL;
	PRAGMA temp_store=memory;
	PRAGMA page_size=4096;
	PRAGMA mmap_size=6000000;
	PRAGMA optimize;
	";

	conn.execute_batch(initial_pragmas)?;
	conn.set_prepared_statement_cache_capacity(128);

perf: use emit from swc instead of tsc #15118

perf: use emit from swc instead of tsc #15118

Conversation

dsherret commented Jul 7, 2022 • edited Loading

kitsonk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsherret Jul 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kitsonk left a comment

Choose a reason for hiding this comment

kitsonk commented Jul 12, 2022

dsherret commented Jul 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsherret commented Jul 7, 2022 •

edited

Loading

dsherret Jul 12, 2022 •

edited

Loading