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

perf: cache swc dependency analysis and don't hold onto ParsedSources in memory #15502

Merged
merged 10 commits into from
Aug 22, 2022

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Aug 19, 2022

This PR caches swc dependency analysis and changes it so that we don't hold onto ParsedSources anymore while running a program. This means that memory usage is drastically reduced and we have a significant performance improvement because we no longer need to parse sources files on every run.

Memory Usage

Given the following code:

import { Application } from "https://deno.land/x/[email protected]/mod.ts";
import * as test from "https://deno.land/[email protected]/node/module_all.ts";
import chalk from "npm:chalk";

console.log(Application);
console.log(chalk);
console.log(test);

setInterval(() => console.log(5), 5000);

Before: 185MB
After: 39MB

Speed

In the example above, but removing the setInterval:

Before (after first run): ~940ms
After (after first run): ~460ms (removing dep analysis cache, brings it up to 970ms on first run)

For something like just importing ts-morph:

import { Project } from "https://deno.land/x/ts_morph/mod.ts";

console.log(Project);

Before (after first run): ~1080ms
After (after first run): ~225ms

@dsherret dsherret requested a review from kitsonk August 21, 2022 20:36
@dsherret dsherret marked this pull request as ready for review August 21, 2022 20:36
cli/tools/doc.rs Outdated

let mut doc_nodes = if source_file == "--builtin" {
struct LibDenoDtsLoader {
Copy link
Member Author

@dsherret dsherret Aug 21, 2022

Choose a reason for hiding this comment

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

This should be changed to use a deno_graph::source::MemoryLoader actually. I will do that tomorrow along with adding unit tests for the cache.

@@ -42,11 +35,8 @@ pub fn contains_specifier(
pub enum ModuleEntry {
Module {
code: Arc<str>,
maybe_parsed_source: Option<ParsedSource>,
Copy link
Member Author

Choose a reason for hiding this comment

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

I think I placed this here a few releases back which caused the memory usage regression. I opened #15531

@@ -206,17 +208,15 @@ fn get_tsc_roots(
.into_iter()
Copy link
Member Author

@dsherret dsherret Aug 21, 2022

Choose a reason for hiding this comment

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

This get_tsc_roots (a few lines up) being in "emit" doesn't make much sense anymore because tsc never emits anymore (would maybe be good to move it in the future). This function is using has_ts_check in order to completely skip parsing with swc when type checking if module analysis has already occurred once before.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me. Open a follow up issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #15535

Copy link
Contributor

@kitsonk kitsonk left a comment

Choose a reason for hiding this comment

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

LGTM, no comments outside of what is already marked. Awesome stuff!

@@ -206,17 +208,15 @@ fn get_tsc_roots(
.into_iter()
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me. Open a follow up issue?

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@dsherret dsherret merged commit 7a1a082 into denoland:main Aug 22, 2022
@dsherret dsherret deleted the deno_graph_reduce_parsing branch August 22, 2022 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants