Uh oh!
There was an error while loading. Please reload this page.
[pull] main from jsr-io:main - #131
Merged
Merged
Conversation
`GET
/api/scopes/:scope/packages/:package/versions/:version/dependencies/graph`
downloaded and parsed **every module of every transitive dependency** on
each request. Cold, that is seconds to tens of seconds, and the frontend
blocks its SSR render on it.
Everything the endpoint returns — edges, sizes, media types — is already
written into `<version>_meta.json` at publish time (`moduleGraph2` +
`manifest`), and the endpoint was fetching those documents anyway to
resolve `jsr:` specifiers. It was re-deriving all of it per request.
## Why the metadata wasn't used
deno_graph already implements this: when it resolves a `jsr:` specifier
it probes the loader with `CacheSetting::Only`, and on `Ok(None)` builds
the module from the package's published `moduleGraph2` without touching
the source. Three things defeated it:
1. **`DepTreeLoader` ignored `LoadOptions`** — it answered that probe
with a real bucket fetch, forcing the download-and-parse path for every
dependency module.
2. **Sources were fetched a second time** (`pending_content_loads`)
purely to fill in each node's `size`, which the manifest beside
`moduleGraph2` already records.
3. **The viewed package's own modules are `file:` specifiers**, outside
deno_graph's `jsr:` handling entirely, so they were always downloaded
and parsed — even though the handler had already read their
`_meta.json`.
The consequence was that the walk could only advance one module at a
time: a module's imports were unknown until its source came back and
went through swc.
## Result
Measured over **419 packages**, replaying the same deno_graph build:
| | before | after |
|---|---|---|
| bucket GETs per graph | 57.8 | **4.2** |
| **critical-path round trips** | 10–20 | **2–6** |
| modules parsed | 53.7 | **0** |
| bytes read per graph | 352 KB | **52 KB** |
Per package, on a representative set:
| package | GETs | parses |
|---|---|---|
| `@hono/hono` | 181 → **0** | 181 → **0** |
| `@zod/zod` | 106 → **1** | 105 → **0** |
| `@std/testing` | 80 → **12** | 68 → **0** |
| `@oak/oak` | 157 → **18** | 139 → **0** |
| `@david/dax` | 156 → **18** | 138 → **0** |
| `@fresh/core` | 240 → **35** | 205 → **0** |
| `@gfx/canvas` | 235 → **17** | 218 → **0** |
## Metadata cache
What remains is two round trips per level of the dependency tree,
against a bucket in a different cloud from the API — and those read the
same few documents over and over. Over that sweep **56% of the reads
were repeats**, with `@std/path/meta.json` alone read by one build in
seven; every dependency tree bottoms out in the same handful of
packages.
`RegistryMetadataCache` (following the existing `GenerateCtxCache`
pattern, registered as router data so tests stay isolated) holds them.
The handler's own `<version>_meta.json` read goes through it too, so **a
warm build touches the bucket zero times** and is pure CPU.
## Caching fix
The route used `util::cache`, not `cache_shared`. The lb refuses to read
*or* write its cache for authenticated requests unless the response is
marked identity-independent, and the frontend's SSR fetch carries the
user's token — so **every signed-in view paid the full cold build**, and
the response came back `private, no-store`. The handler reads only the
buckets keyed by scope/package/version (no db, no iam), same as `docs`,
so it is `cache_versioned_shared` now. That also stops `latest` being
pinned for a day.
## Fallback registry
The fallback probe no longer borrows publish's
`FALLBACK_REQUEST_TIMEOUT` (30s). It fires once per bucket miss, so on a
fallback-hosted package that patience was paid per file; it has its own
5s bound now.
## Correctness
Every module's specifier, size and media type compared between the old
and new paths across **419 packages / 23,764 modules**: **418
identical**.
The exception is `@algo-lang/algo-compiler@1.0.9`, whose stored metadata
records `{}` for a CommonJS file whose `require()` calls a current
deno_graph parse does resolve — its metadata predates that analyser
improvement. Worth noting that the stored metadata is exactly what Deno
itself resolves against (the CLI reads `moduleGraph2` and does not
re-parse), so the graph now matches what the CLI sees rather than
something only this endpoint computed.
Checked along the way:
- **Sizes** identical on all 23,764. `Module::size()` is decoded text
length while the manifest is raw bytes, so they would diverge on a BOM'd
or non-UTF-8 file; no instance in the sample.
- **Pre-`moduleGraph2` packages** are converted and sized correctly.
This guard is load-bearing — without it `@gfx/canvas` silently loses
sizes on 203 modules.
- **Wasm** never reaches the response (`build_module_info` returns
`None` for `Module::Wasm`), confirmed against production: `@deno/loader`
has 7 graph modules and the API returns 6. The stub is the minimum valid
wasm module so `graph.valid()` still passes.
- **Fallback registry** still tags a package as fallback-hosted via its
`_meta.json` read, before any file is stubbed.
## Tradeoffs
Two new staleness windows, both dwarfed by the lb's 1-day `s-maxage` on
the response:
- A package's version list is held **60s** — the delay before a newly
published version starts satisfying a `jsr:` range in someone else's
graph.
- A version's metadata is held **5 minutes**. The document is immutable,
so this is not about staleness: a staff version delete purges the CDN,
and a per-instance cache cannot be purged. 5 minutes bounds how long a
deleted version can still appear in a graph.
## Tests
The saving is invisible in the output — the graph is identical either
way — so `test_package_dependencies_graph` now asserts that no module in
the graph reached the parser. Verified it fails when either half of the
change is reverted. 257 tests pass; clippy and fmt clean.
## Not addressed
`analyze_deps_tree(...).await.unwrap().unwrap()` panics per request on
any unresolvable dependency — `graph.valid()?` is a normal outcome, not
a programmer error. Left alone as a separate concern.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )