perf(pm): extract requested core from full manifest parse#3034
perf(pm): extract requested core from full manifest parse#3034elrrrrrrr wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the manifest parsing logic to allow optional extraction of a core version manifest during the initial parsing task. It introduces parse_full_manifest_with_core_off_runtime and a corresponding synchronous helper. Feedback suggests optimizing the path where a version specifier is provided to avoid redundant JSON parsing and memory allocations by using a DOM-based approach to extract both the full manifest and the specific version manifest in a single pass.
| fn parse_full_manifest_with_core_sync( | ||
| raw_bytes: Bytes, | ||
| spec: Option<String>, | ||
| ) -> Result<FullManifestParseResult, anyhow::Error> { | ||
| // simd_json mutates the parse buffer; copy inside the worker so | ||
| // response-body bytes can stay immutable until parsing starts. | ||
| let mut parse_buf = raw_bytes.to_vec(); | ||
| let mut manifest: FullManifest = simd_json::serde::from_slice::<FullManifest>(&mut parse_buf) | ||
| .map_err(|e| anyhow!("JSON parse error: {e}"))?; | ||
| manifest.raw = raw_bytes; | ||
|
|
||
| let speculative = spec.and_then(|spec| { | ||
| resolve_target_version((&manifest).into(), &spec) | ||
| .ok() | ||
| .and_then(|version| manifest.get_core_version(&version).map(|core| (spec, core))) | ||
| }); | ||
|
|
||
| Ok((manifest, speculative)) | ||
| } |
There was a problem hiding this comment.
This implementation performs two full JSON parses and an extra memory allocation when a spec is provided. manifest.get_core_version calls extract_version, which clones manifest.raw into a new Vec and re-parses it using simd_json::to_borrowed_value.
Since this PR is focused on performance, you can optimize the Some(spec) path by parsing the JSON once into a simd_json::BorrowedValue DOM. From this DOM, you can both deserialize the FullManifest and extract the specific CoreVersionManifest without re-parsing or re-allocating the raw bytes.
For the None case, keeping the direct simd_json::serde::from_slice is preferred as it is generally faster than building the intermediate DOM.
fn parse_full_manifest_with_core_sync(
raw_bytes: Bytes,
spec: Option<String>,
) -> Result<FullManifestParseResult, anyhow::Error> {
use serde::Deserialize;
// simd_json mutates the parse buffer; copy inside the worker so
// response-body bytes can stay immutable until parsing starts.
let mut parse_buf = raw_bytes.to_vec();
let (mut manifest, speculative) = if let Some(spec) = spec {
use simd_json::prelude::ValueObjectAccess;
let value = simd_json::to_borrowed_value(&mut parse_buf)
.map_err(|e| anyhow!("JSON parse error: {e}"))?;
let manifest = FullManifest::deserialize(&value)
.map_err(|e| anyhow!("JSON mapping error: {e}"))?;
let speculative = resolve_target_version((&manifest).into(), &spec)
.ok()
.and_then(|version| {
let version_obj = value.get("versions")?.get(version.as_str())?;
let core = CoreVersionManifest::deserialize(version_obj).ok()?;
Some((spec, core))
});
(manifest, speculative)
} else {
let manifest: FullManifest = simd_json::serde::from_slice::<FullManifest>(&mut parse_buf)
.map_err(|e| anyhow!("JSON parse error: {e}"))?;
(manifest, None)
};
manifest.raw = raw_bytes;
Ok((manifest, speculative))
}8096e2c to
bd39706
Compare
31aa711 to
133eaf3
Compare
bd39706 to
b96d81c
Compare
133eaf3 to
71f73b1
Compare
b96d81c to
604aa30
Compare
71f73b1 to
5af3c3b
Compare
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.11s | 0.03s | 10.30s | 10.02s | 719M | 337.8K |
| utoo-next | 7.83s | 0.29s | 10.29s | 12.00s | 1.01G | 123.0K |
| utoo-npm | 8.07s | 0.09s | 10.61s | 12.30s | 1022M | 123.2K |
| utoo | 8.28s | 0.44s | 10.32s | 11.96s | 1010M | 124.1K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 17.0K | 19.0K | 1.19G | 7M | 1.86G | 1.75G | 1M |
| utoo-next | 129.1K | 95.0K | 1.16G | 5M | 1.71G | 1.70G | 2M |
| utoo-npm | 143.7K | 103.9K | 1.16G | 5M | 1.71G | 1.70G | 2M |
| utoo | 130.5K | 80.4K | 1.16G | 6M | 1.71G | 1.70G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.06s | 0.02s | 3.90s | 1.12s | 495M | 167.0K |
| utoo-next | 2.89s | 0.02s | 5.00s | 1.84s | 615M | 89.2K |
| utoo-npm | 3.06s | 0.05s | 5.20s | 2.21s | 612M | 82.3K |
| utoo | 2.92s | 0.09s | 5.03s | 1.86s | 609M | 77.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 10.3K | 4.2K | 202M | 3M | 107M | - | 1M |
| utoo-next | 51.0K | 69.3K | 200M | 2M | 7M | 3M | 2M |
| utoo-npm | 77.0K | 91.4K | 200M | 3M | 7M | 3M | 2M |
| utoo | 51.8K | 71.4K | 200M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.73s | 0.45s | 6.29s | 9.55s | 610M | 206.6K |
| utoo-next | 7.48s | 2.62s | 5.00s | 10.57s | 475M | 61.8K |
| utoo-npm | 6.43s | 0.68s | 5.08s | 10.43s | 472M | 60.8K |
| utoo | 6.58s | 0.82s | 5.08s | 10.27s | 517M | 64.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 6.8K | 7.2K | 1019M | 4M | 1.76G | 1.76G | 1M |
| utoo-next | 116.2K | 52.5K | 988M | 3M | 1.70G | 1.70G | 2M |
| utoo-npm | 107.8K | 50.4K | 988M | 3M | 1.70G | 1.70G | 2M |
| utoo | 96.4K | 53.3K | 988M | 3M | 1.70G | 1.70G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.35s | 0.03s | 0.20s | 2.40s | 133M | 32.2K |
| utoo-next | 2.15s | 0.08s | 0.47s | 3.69s | 80M | 18.4K |
| utoo-npm | 2.23s | 0.07s | 0.46s | 3.71s | 79M | 18.6K |
| utoo | 2.15s | 0.04s | 0.46s | 3.70s | 79M | 18.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 216 | 25 | 5M | 18K | 1.91G | 1.75G | 1M |
| utoo-next | 41.5K | 19.0K | 324K | 24K | 1.70G | 1.70G | 2M |
| utoo-npm | 39.5K | 18.3K | 326K | 9K | 1.70G | 1.70G | 2M |
| utoo | 40.2K | 18.3K | 325K | 8K | 1.71G | 1.70G | 2M |
npmmirror.com: no output captured.
Summary
Review-sized split from #3028 / #2948, stacked after #3031.
Adds the full-manifest parse helper used by the upcoming provider implementation:
parse_full_manifest_off_runtimeas the compatibility wrapper;parse_full_manifest_with_core_off_runtimeto parse the full manifest and optionally extract the current edge's core version in the same worker;FullManifest::rawfor later on-demand extraction;latestextraction.This PR does not change resolver scheduling yet; it only exposes the CPU helper consumed by the provider/job PR.
Size
1 file changed, 65 insertions(+), 15 deletions(-)Validation
cargo fmtcargo check -p utoo-ruboristcargo test -p utoo-ruborist service::manifest::tests::parse_full_manifest_with_core_extracts_requested_speccargo clippy --all-targets -- -D warnings --no-depspack-napistill warns locally becausenext.jsis a symlink in this worktree; clippy exits successfully.