-
Notifications
You must be signed in to change notification settings - Fork 5.6k
perf: cache swc dependency analysis and don't hold onto ParsedSource
s in memory
#15502
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
Conversation
cli/tools/doc.rs
Outdated
|
||
let mut doc_nodes = if source_file == "--builtin" { | ||
struct LibDenoDtsLoader { |
There was a problem hiding this comment.
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>, |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #15535
There was a problem hiding this 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() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR caches swc dependency analysis and changes it so that we don't hold onto
ParsedSource
s 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:
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:
Before (after first run): ~1080ms
After (after first run): ~225ms