denoland / deno_core

The core engine at the heart of Deno
MIT License
233 stars 76 forks source link

perf: Batch calls to `ModuleLoader::prepare_load` for dynamic imports #761

Open nathanwhit opened 1 month ago

nathanwhit commented 1 month ago

Collects dynamic imports between event loop ticks and then calls prepare_load with the collected imports. This reduces the number of calls to prepare_load. Performance impact on deno is noticeable but relatively modest, around 6-7%:

❯ hyperfine --warmup 10 "deno run -A repro.ts" "devdeno run -A repro.ts"
Benchmark 1: deno run -A repro.ts
  Time (mean ± σ):     211.8 ms ±   2.0 ms    [User: 212.7 ms, System: 85.8 ms]
  Range (min … max):   209.3 ms … 217.4 ms    14 runs

Benchmark 2: devdeno run -A repro.ts
  Time (mean ± σ):     197.3 ms ±   1.2 ms    [User: 189.9 ms, System: 84.5 ms]
  Range (min … max):   195.9 ms … 200.7 ms    14 runs

Summary
  devdeno run -A repro.ts ran
    1.07 ± 0.01 times faster than deno run -A repro.ts

❯ hyperfine --warmup 10 "deno run -A repro2.ts" "devdeno run -A repro2.ts"
Benchmark 1: deno run -A repro2.ts
  Time (mean ± σ):     146.3 ms ±   1.0 ms    [User: 180.9 ms, System: 51.6 ms]
  Range (min … max):   144.5 ms … 148.0 ms    20 runs

Benchmark 2: devdeno run -A repro2.ts
  Time (mean ± σ):     137.8 ms ±   0.8 ms    [User: 160.7 ms, System: 51.7 ms]
  Range (min … max):   136.5 ms … 139.9 ms    21 runs

Summary
  devdeno run -A repro2.ts ran
    1.06 ± 0.01 times faster than deno run -A repro2.ts

My main concern here is the loss of granularity in errors that occur during prepare_load. Since we now are calling prepare_load with multiple specifiers, if the loader returns an error we can't really tell which specifier the error applies to. That results in a visible behavior change for users, for example

await Promise.all([
  import("foo.ts"),
  import("will-cause-prepare-load-error.ts").catch((_e) => {}),
]);

console.log("Done");

Before this PR, that would successfully print "Done", currently with this PR it would throw for the import of "foo.ts" (even though it didn't actually have an error).