denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
95.58k stars 5.3k forks source link

Per-`JsRuntime` maps in `ProcState` #12458

Open andreubotella opened 2 years ago

andreubotella commented 2 years ago

Module maps are conceptually specific to an instance of JsRuntime, yet part of the module map mechanics in our current implementation is maintained in ProcState, which is shared across multiple JsRuntimes. An implementation of #12248 would need ProcState to keep a per-JsRuntime url → blob map, and according to @bartlomieju, something similar would be needed to cache resolved modules.

One issue is how to represent JsRuntime identity in those maps. JsRuntime is not cloneable or a reference type, and it is not typically used with a reference-counted pointer type. What's more, if a JsRuntime is dropped ­–if a worker is terminated, for example– the blob references and cached modules associated with that JsRuntime should be dropped as well, but there doesn't seem to be any obvious way to do this, since JsRuntime has no concept of ProcState.

bartlomieju commented 2 years ago

One issue is how to represent JsRuntime identity in those maps.

Does it need to be represented? We can solve this easily by storing GraphData on instance of CliModuleLoader which is passed by MainWorker/WebWorker down to JsRuntime. If you need to keep blobs accounted for I believe storing them on the CliModuleLoader would be the way to go, although I don't know all the nitty gritty details. I'll be tackling this refactor, so if you have any particular requirements, please let me know.

andreubotella commented 2 years ago

I suspect the requirements for blobs would be the same as for GraphData: that the graph/blob data must be accessible when resolving and loading every module in a module graph, not just the initial module which is directly handled by CliModuleLoader.

bartlomieju commented 2 years ago

I suspect the requirements for blobs would be the same as for GraphData: that the graph/blob data must be accessible when resolving and loading every module in a module graph, not just the initial module which is directly handled by CliModuleLoader.

All modules that are loaded are handled by CliModuleLoader via the load method - the whole "module graph" is just a requirement for type-checking, if we did not have it the same analysis would be performed (and actually is) by V8 when requesting all modules to be loaded. So this shouldn't be a problem, you'll be able to access state of module loader from within load method.