feat: emit files on demand and fix racy emit #15220
Conversation
…o verify they're getting the right source
| .filter(|p| p.is_file()); | ||
| Some(CacheInfo { | ||
| local: Some(local), | ||
| emit, | ||
| map, | ||
| map: None, |
There was a problem hiding this comment.
We haven't written out a map file for a while.
| #[derive(Debug, Clone, PartialEq)] | ||
| pub struct SpecifierEmitCacheData { | ||
| #[derive(Debug, Deserialize, Serialize)] | ||
| struct EmitMetadata { | ||
| pub source_hash: String, |
There was a problem hiding this comment.
This changed from version_hash (diff is not clear because I moved this struct here). It's using a faster xxHash now so worth it to rename I think.
Also, if someone upgrades deno, uses it a bit, then downgrades, older versions of deno reading this file will re-emit.
There was a problem hiding this comment.
Add as a docstring that Deno version mismatch will cause to skip the cache?
| @@ -40,6 +56,65 @@ impl CliModuleLoader { | |||
| ps, | |||
| }) | |||
| } | |||
|
|
|||
| fn load_prepared_module( | |||
There was a problem hiding this comment.
I had initially moved this code from proc_state here in the previous PR because I couldn't store an sqlite connection in an Arc, but I kind of like it here better anyway, so I left this here for this PR.
There was a problem hiding this comment.
I agree it makes more sense to have this method on CliModuleLoader 👍
| #[derive(Debug, Clone, PartialEq)] | ||
| pub struct SpecifierEmitCacheData { | ||
| #[derive(Debug, Deserialize, Serialize)] | ||
| struct EmitMetadata { | ||
| pub source_hash: String, |
There was a problem hiding this comment.
Add as a docstring that Deno version mismatch will cause to skip the cache?
| @@ -40,6 +56,65 @@ impl CliModuleLoader { | |||
| ps, | |||
| }) | |||
| } | |||
|
|
|||
| fn load_prepared_module( | |||
There was a problem hiding this comment.
I agree it makes more sense to have this method on CliModuleLoader 👍
| // we need the code with the source map in order for | ||
| // it to work with --inspect or --inspect-brk | ||
| code_source.code | ||
| } else { | ||
| // reduce memory and throw away the source map | ||
| // because we don't need it | ||
| code_without_source_map(code_source.code) | ||
| }; |
| Ok(ProcState(Arc::new(Inner { | ||
| dir, | ||
| coverage_dir, | ||
| options: cli_options, | ||
| emit_cache, | ||
| emit_options_hash: FastInsecureHasher::new() | ||
| // todo(dsherret): use hash of emit options instead as it's more specific |
There was a problem hiding this comment.
Can you address this TODO before landing?
There was a problem hiding this comment.
It needs to be addressed in deno_ast I think. I opened denoland/deno_ast#105
|
Ehh... I just realized I categorized this as a |
This is an alternative to #15198 (branched from it). Instead of using an sqlite database it keeps things pretty much the same, but adds an additional source hash to the "versions" file. So it's somewhat more non-breaking.
I feel a little bit better about this PR because it's non-breaking and we don't depend on sqlite as much.
Closes #13302
Closes #15124