Deno: Duplicate recompilation on circular imports

[kevinkassimo]

Basically, assume we have a file 1.ts:

import "2.ts";
console.log("1.ts");

and another file 2.ts:

import "1.ts";
console.log("2.ts");

With latest master, running deno 1.ts --recompile would generate the following:

Compiling /Users/kevinqian/Desktop/Programming/Deno/test/deps/1.ts
Compiling /Users/kevinqian/Desktop/Programming/Deno/test/deps/2.ts
Compiling /Users/kevinqian/Desktop/Programming/Deno/test/deps/1.ts  <== duplicate compilation
2.ts
1.ts

[kitsonk]

I am taking a look at this as part of the post-ESM cleanup.

[ry]

Nice catch.

Tangental comment to both of you: "Compiling" is currently printed from the TS isolate
https://github.com/denoland/deno/blob/cca3a9562bc41744a8fdb4fd8b185505ca9af1c3/js/compiler.ts#L238
But it needs to be moved into Rust in order to support progress bars (#1320). Maybe here:
https://github.com/denoland/deno/blob/cca3a9562bc41744a8fdb4fd8b185505ca9af1c3/src/isolate.rs#L380-L382

[ry]

I'm looking into this. My plan is to refactor the libdeno module API again to better expose the dependency graph in Rust. This will allow parallel downloads. Tentative new API:

// An "EcmaScript" module id. Good values are postiive.
typedef int deno_mod;

// Returns positive deno_mod module id on success. non-positive on failure.
// On failure, get error text with deno_last_exception().
// Not reentrant.
deno_mod deno_mod_new(Deno* d, char* filename, char* source);

// Returns the number of static imports made by the specified module.
// Not reentrant.
uint32_t deno_mod_imports(Deno* d, deno_mod id);

// Retreives the nth static import specifier. The returned string will be valid
// for the lifetime of the Deno* isolate.
// Use this to pre-download the source code of each dependency so that later,
// and when it is complete call deno_mod_resolve() with a newly created module.
// Not reentrant.
const char* deno_mod_get_import(Deno* d, deno_mod id, uint32_t index);

// Call this for each of the import requests.
// Not reentrant.
void deno_mod_resolve(Deno* d, deno_mod parent_id, uint32_t index,
                      deno_mod child_id);

// Call this after deno_mod_resolve() as been called for each of the module's
// imports.
// Return value: 0 = fail, 1 = success
// Get error text with deno_last_exception().
// Not reentrant.
int deno_mod_instantiate(Deno* d, deno_mod id);

// The module must have first been instanciated before being evaluted.
// Only the main module is evaluated.
// Child modules only should have only deno_mod_instantiate() called on them.
// Return value: 0 = fail, 1 = success
// Get error text with deno_last_exception().
// Potentially reentrant.
int deno_mod_evaluate(Deno* d, deno_mod id, void* user_data);

[kitsonk]

@ry the API looks logical to me... The only thing I think worth discussing is the macro logic that is going around in my head, and I haven't had the time yet to get my head around the flows now that it is ESM.

• If a module source code is JavaScript, I believe we are bypassing the TypeScript compiler wholly. I understand the performance improvement of this, though we may want to consider a mode when we support tsconfig.json where if they assert checkJs in the compiler options, that all JavaScript is sent through the compiler. If I am right about the bypass, I would remove checkJs from the compiler options in the compiler, as it is really doing nothing. One advantage of the checkJs is that CommonJS and AMD modules would be transformed to ESM by the compiler.
• TypeScript can and does elide modules, so not every module it requests to do the type checking is actually required to be run. I suspect this just works at the moment, because TypeScript's dependency graph is independent of the ESM dependency graph, but something to consider if somehow there was an attempt to merge. I do believe they are different, as sometimes there maybe erasable types in another module, which doesn't actually get loaded into the runtime.
• TypeScript requests modules in tranches, but up to this point we have been resolving them serially. An improvement might be possible in having the code fetch op out of the compiler take an array/vector so the Rust can start to better parallelise fetching dependencies there as well.

[ry]

if they assert checkJs in the compiler options, that all JavaScript is sent through the compiler

I'm open to it. Let's discuss after we've got this done. Until then I'd like to assume to that JS does not go thru TS.

I suspect this just works at the moment, because ...

Hopefully we can get some tests that demo such problems.

An improvement might be possible in having the code fetch op out of the compiler take an array/vector so the Rust can start to better parallelise fetching dependencies there as well.

Or make codeFetch async?

[kitsonk]

An improvement might be possible in having the code fetch op out of the compiler take an array/vector so the Rust can start to better parallelise fetching dependencies there as well.

Or make codeFetch async?

Well, not directly. Having a function like that would bubble up and try to turn the whole TypeScript compiler into async, which it can't be. When TypeScript runs in a browser, it loads itself into a web worker so that it doesn't block the main thread, because it is inherently sync.

The only trick is that resolving the module name is seperate from fetching the contents of the module. Again, the resolution of the modules come in batches (per module, all the deps get requested), but contents are requested by module serially. So if we split the ops of resolving the module names and the actual fetching of the content, Rust could go off, start fetching stuff and then the individual contents ops would not be able to be responded to until the request was fulfilled.

[ry]

Version 2

// Returns positive deno_mod module id on success. non-positive on failure.
// Re-entrant: deno_resolve_cb will be called during this invocation.
// On failure, get error text with deno_last_exception().
deno_mod deno_mod_new(Deno* d, char* filename, char* source);

typedef enum {
  DENO_MOD_ERROR = 0,
  DENO_MOD_UNINSTANCIATED = 1,
  DENO_MOD_INSTANCIATED = 2,
  DENO_MOD_EVALUATED = 3,
} deno_mod_state;
deno_mod_state deno_mod_get_state(Deno* d, deno_mod id);

// The module must have state DENO_MOD_INSTANCIATED.
// Only call this on the main module.
// Child modules only should have only deno_mod_instantiate() called on them.
// The state of the module  will be DENO_MOD_ERROR or DENO_MOD_EVALUATED.
// Get error text with deno_last_exception().
// Re-entrant.
void deno_mod_evaluate(Deno* d, deno_mod id, void* user_data);

// Called during deno_mod_new and deno_mod_evaluate.
// The receiver must call deno_resolve() for each resolve_id received in this
// way.
// is_dynamic: 0 = static import, 1 = dynamic import.
typedef void (*deno_resolve_cb)(Deno* d, uint32_t resolve_id, int is_dynamic,
                        const char* specifier, const char* referrer_name,
                        deno_mod referrer);

// Call this for every invocation of deno_resolve_cb with the given index.
// Must be called on the V8 thread. Not thread safe.
// Set child to 0 to signal error.
// Not Re-entrant.
// The state of referrer should be checked after invocation.
void deno_resolve(Deno* d, uint32_t resolve_id, deno_mod child);

[kevinkassimo]

Under the assumption that JS does not go through TS compiler, code fetch and resolution could happen asynchronously. Not so sure if this has been alluded to, but once a module is compiled (and before it is instantiated), we can get a list of its dependency requests from v8::Module APIs (GetModuleRequest()) and thus could fetch in parallel (and recursively). (This is what Node and browser is doing)

Actually I'm a little bit curious how many times is the TS compiler dependency map built? Is it built separately for each node in the dep tree and spans to the innermost deps, every single time the DenoCompiler.compile is called (seems to be like this from my experiment)? If this is true, I assume it also causes some performance penalties since the compiler has to read the same type information of each files multiple times?