diff --git a/.devcontainer/rust/devcontainer-feature.json b/.devcontainer/rust/devcontainer-feature.json index 368c410bf5c3..f7098e7c26ba 100644 --- a/.devcontainer/rust/devcontainer-feature.json +++ b/.devcontainer/rust/devcontainer-feature.json @@ -7,7 +7,7 @@ // this should match the `rust-toolchain.toml` "version": "nightly-2026-08-20", "profile": "minimal", - "components": "rustfmt,clippy,rust-analyzer" + "components": "rustfmt,clippy,rust-analyzer,miri" } } } diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 14630bd49ba1..7fe39a4e2c95 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -300,6 +300,7 @@ jobs: with: needsRust: 'yes' needsNextest: 'yes' + needsRustJsInstall: 'yes' skipNativeBuild: 'yes' skipInstallBuild: 'yes' afterBuild: pnpm dlx turbo@${TURBO_VERSION} run test-cargo-unit ${TURBO_ARGS} @@ -313,6 +314,23 @@ jobs: runs_on_labels: '["ubuntu-latest-16-core-oss"]' secrets: inherit + # Turbo Tasks' link-section registry is unavailable under Miri, so this runs the explicit + # low-level package allowlist in packages/next-swc/package.json instead of the whole workspace. + test-cargo-unit-miri: + name: test cargo unit (miri) + needs: ['changes', 'build-next'] + if: ${{ needs.changes.outputs.docs-only == 'false' }} + + uses: ./.github/workflows/build_reusable.yml + with: + needsRust: 'yes' + needsNextest: 'yes' + skipNativeBuild: 'yes' + skipInstallBuild: 'yes' + afterBuild: pnpm dlx turbo@${TURBO_VERSION} run test-cargo-unit-miri ${TURBO_ARGS} + stepName: 'test-cargo-unit-miri' + secrets: inherit + test-bench: name: test cargo benches needs: ['optimize-ci', 'changes', 'build-next'] diff --git a/.github/workflows/build_reusable.yml b/.github/workflows/build_reusable.yml index 0d30cdd8c897..20aefba8cc92 100644 --- a/.github/workflows/build_reusable.yml +++ b/.github/workflows/build_reusable.yml @@ -36,6 +36,10 @@ on: required: false description: 'if nextest rust dep is needed' type: string + needsRustJsInstall: + required: false + description: 'if Rust JS dependencies should be installed' + type: string rustBuildProfile: required: false description: 'The profile to use for the build, default is `release-with-assertions`, also supports `` for debug and `release` for normal release' @@ -360,7 +364,7 @@ jobs: # risk caching an incomplete store # If keep conditions in sync breaks, we can split into restore and save # steps where saving runs based on the outcome of the install step - if: ${{ inputs.skipInstallBuild != 'yes' || inputs.needsNextest == 'yes' }} + if: ${{ inputs.skipInstallBuild != 'yes' || inputs.needsRustJsInstall == 'yes' }} uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 timeout-minutes: 5 id: cache-pnpm-store @@ -371,10 +375,10 @@ jobs: - run: pnpm install # condititions must be superset of cache step, otherwise we risk running install without a cache and then caching the incomplete store. - if: ${{ inputs.skipInstallBuild != 'yes' || inputs.needsNextest == 'yes' }} + if: ${{ inputs.skipInstallBuild != 'yes' || inputs.needsRustJsInstall == 'yes' }} - name: Install node-file-trace test dependencies - if: ${{ inputs.needsNextest == 'yes' }} + if: ${{ inputs.needsRustJsInstall == 'yes' }} working-directory: turbopack/crates/turbopack-tracing/tests/node-file-trace run: pnpm install --recursive diff --git a/examples/cache-handler-redis/README.md b/examples/cache-handler-redis/README.md index e29fd1f81c73..301f7cba5815 100644 --- a/examples/cache-handler-redis/README.md +++ b/examples/cache-handler-redis/README.md @@ -46,9 +46,9 @@ The `/[timezone]` page renders a mostly static shell and, inside it, a `'use cac - **ISR cache (`cache-handler.js`):** stores the prerendered page entries as JSON under a `nextjs:cache:` prefix, and tracks which keys belong to each tag in a Redis set (`nextjs:tag:`). `revalidateTag` deletes every key associated with a tag. -- **Remote cache (`remote-cache-handler.js`):** stores each `'use cache: remote'` entry under a `nextjs:use-cache:` prefix (the streamed value is base64-encoded). Tag revalidation is timestamp-based: `updateTags` records `nextjs:use-cache-tag:` = now, and `getExpiration` reports the latest time so Next treats older entries as stale. Clicking **Revalidate** calls [`updateTag('time-data')`](https://nextjs.org/docs/app/api-reference/functions/updateTag), which regenerates the remote entry. +- **Remote cache (`remote-cache-handler.js`):** stores each `'use cache: remote'` entry under a `nextjs:use-cache:` prefix (the streamed value is base64-encoded). Tag revalidation is timestamp-based: `updateTags` records `nextjs:use-cache-tag:` = now. On a hit, `get` compares the entry's own tags (from `cacheTag`) against those timestamps and reports a miss if any is newer, and `getExpiration` reports the latest time for the route's soft tags so Next discards older entries. Clicking **Revalidate** calls [`updateTag('time-data')`](https://nextjs.org/docs/app/api-reference/functions/updateTag), which regenerates the remote entry. -- **Building without Redis:** both handlers skip connecting during `next build` (they check `NEXT_PHASE`) and degrade gracefully when Redis is unavailable, so the app still builds and runs, just without a shared cache. +- **Building without Redis:** both handlers skip connecting during `next build` (they check `NEXT_PHASE`) and degrade gracefully when Redis is unavailable, so the app still builds and runs, just without a shared cache. Revalidation is the exception: `revalidateTag` and `updateTags` throw while Redis is unavailable, because an invalidation that never reached Redis would be lost, and the old entries would be served again once Redis is back. - **Redis server setup:** ensure your Redis server is running before starting the app. Configure the connection with `REDIS_URL` (defaults to `redis://localhost:6379`). diff --git a/examples/cache-handler-redis/cache-handler.js b/examples/cache-handler-redis/cache-handler.js index 848155b2a09d..7faba1aa1f03 100644 --- a/examples/cache-handler-redis/cache-handler.js +++ b/examples/cache-handler-redis/cache-handler.js @@ -43,49 +43,77 @@ function deserialize(text) { }); } +// Next.js constructs the `cacheHandler` class once per request, so the Redis +// client must live at module scope: creating it in the constructor would open +// a new connection on every request and never close it. +const client = createClient({ + url: process.env.REDIS_URL ?? "redis://localhost:6379", + // Fail commands immediately while the connection is down instead of queueing + // them until Redis is back. + disableOfflineQueue: true, +}); + +// Redis won't work without error handling. Do not throw here, otherwise the +// client won't reconnect after a connection drop. +client.on("error", (error) => { + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis client error:", error); + } +}); + +// Connecting to Redis during `next build` can cause issues, so we only connect +// at runtime. +const connection = + process.env.NEXT_PHASE === PHASE_PRODUCTION_BUILD + ? Promise.resolve() + : client.connect().catch((error) => { + console.warn("Failed to connect to Redis:", error); + }); + +// `connect()` stays pending for as long as Redis is unreachable, so cap the +// wait: requests arriving during startup wait for the connection at most +// once, and while Redis is down every request is served uncached instead of +// blocking. The client keeps retrying in the background, so `isReady` flips +// back on its own once Redis is reachable again. +const CONNECT_TIMEOUT_MS = 1000; +const ready = Promise.race([ + connection, + // `unref()` so this timer never keeps the process alive. + new Promise((resolve) => setTimeout(resolve, CONNECT_TIMEOUT_MS).unref()), +]); + +// Resolve a connected client, or `null` when Redis is unavailable so the app +// keeps working (without a shared cache) instead of hanging or crashing. +async function getClient() { + await ready; + return client.isReady ? client : null; +} + module.exports = class CacheHandler { constructor(options) { this.options = options; - - this.client = createClient({ - url: process.env.REDIS_URL ?? "redis://localhost:6379", - }); - - // Redis won't work without error handling. Do not throw here, otherwise - // the client won't reconnect after a connection drop. - this.client.on("error", (error) => { - if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { - console.warn("Redis client error:", error); - } - }); - - // Connecting to Redis during `next build` can cause issues, so we only - // connect at runtime. `this.connection` resolves once the client is ready. - this.connection = - process.env.NEXT_PHASE === PHASE_PRODUCTION_BUILD - ? Promise.resolve() - : this.client.connect().catch((error) => { - console.warn("Failed to connect to Redis:", error); - }); - } - - // Resolve a connected client, or `null` when Redis is unavailable so the - // app keeps working (without a shared cache) instead of crashing. - async getClient() { - await this.connection; - return this.client.isReady ? this.client : null; } async get(key) { - const client = await this.getClient(); + const client = await getClient(); if (!client) return null; - const entry = await client.get(CACHE_PREFIX + key); + let entry; + try { + entry = await client.get(CACHE_PREFIX + key); + } catch (error) { + // A connection dropping mid-request degrades to a cache miss. + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis get failed:", error); + } + return null; + } + return entry ? deserialize(entry) : null; } async set(key, data, ctx) { - const client = await this.getClient(); + const client = await getClient(); if (!client || !data) return; // Collect tags from both sources: `ctx.tags` (fetch entries) and the @@ -105,19 +133,29 @@ module.exports = class CacheHandler { ? { expiration: { type: "EX", value: Math.max(1, Math.ceil(expire)) } } : {}; - await client.set( - CACHE_PREFIX + key, - serialize({ value: data, lastModified: Date.now(), tags }), - options, - ); + const value = serialize({ value: data, lastModified: Date.now(), tags }); + + try { + await client.set(CACHE_PREFIX + key, value, options); - // Index this key under each of its tags so `revalidateTag` can find it. - await Promise.all(tags.map((tag) => client.sAdd(TAG_PREFIX + tag, key))); + // Index this key under each of its tags so `revalidateTag` can find it. + await Promise.all(tags.map((tag) => client.sAdd(TAG_PREFIX + tag, key))); + } catch (error) { + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis set failed:", error); + } + } } async revalidateTag(tags) { - const client = await this.getClient(); - if (!client) return; + const client = await getClient(); + // Don't report success for a revalidation that never reached Redis: once + // Redis is back, every instance would serve the old entries again. + if (!client) { + throw new Error( + "Redis is unavailable, so the tag revalidation was not recorded", + ); + } // `tags` is either a single tag or an array of tags. for (const tag of [tags].flat()) { diff --git a/examples/cache-handler-redis/remote-cache-handler.js b/examples/cache-handler-redis/remote-cache-handler.js index 8bfe82fadbad..cdb9864b4558 100644 --- a/examples/cache-handler-redis/remote-cache-handler.js +++ b/examples/cache-handler-redis/remote-cache-handler.js @@ -13,6 +13,9 @@ const TAG_PREFIX = "nextjs:use-cache-tag:"; const client = createClient({ url: process.env.REDIS_URL ?? "redis://localhost:6379", + // Fail commands immediately while the connection is down instead of queueing + // them until Redis is back. + disableOfflineQueue: true, }); client.on("error", (error) => { @@ -28,8 +31,20 @@ const connection = console.warn("Failed to connect to Redis (remote cache):", error); }); +// `connect()` stays pending for as long as Redis is unreachable, so cap the +// wait: requests arriving during startup wait for the connection at most +// once, and while Redis is down every request is served uncached instead of +// blocking. The client keeps retrying in the background, so `isReady` flips +// back on its own once Redis is reachable again. +const CONNECT_TIMEOUT_MS = 1000; +const ready = Promise.race([ + connection, + // `unref()` so this timer never keeps the process alive. + new Promise((resolve) => setTimeout(resolve, CONNECT_TIMEOUT_MS).unref()), +]); + async function getClient() { - await connection; + await ready; return client.isReady ? client : null; } @@ -38,7 +53,16 @@ module.exports = { const redis = await getClient(); if (!redis) return undefined; - const stored = await redis.get(ENTRY_PREFIX + cacheKey); + let stored; + try { + stored = await redis.get(ENTRY_PREFIX + cacheKey); + } catch (error) { + // A connection dropping mid-request degrades to a cache miss. + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis get failed (remote cache):", error); + } + return undefined; + } if (!stored) return undefined; const data = JSON.parse(stored); @@ -50,6 +74,33 @@ module.exports = { return undefined; } + // Next.js only asks `getExpiration` about the route's soft tags, so the + // entry's own tags (from `cacheTag`) are checked here. If any of them was + // revalidated after this entry was created, on this instance or another, + // the entry is out of date: report a miss so Next.js regenerates it. + if (data.tags.length) { + let revalidatedAt; + try { + revalidatedAt = await redis.mGet( + data.tags.map((tag) => TAG_PREFIX + tag), + ); + } catch (error) { + // Without the tag timestamps we can't tell, so don't serve it. + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis tag lookup failed (remote cache):", error); + } + return undefined; + } + + if ( + revalidatedAt.some( + (time) => time !== null && Number(time) > data.timestamp, + ) + ) { + return undefined; + } + } + return { // `value` must be a stream; rebuild it from the stored bytes. value: new ReadableStream({ @@ -99,38 +150,60 @@ module.exports = { } : {}; - await redis.set( - ENTRY_PREFIX + cacheKey, - JSON.stringify({ - value: bytes.toString("base64"), - tags: entry.tags, - stale: entry.stale, - timestamp: entry.timestamp, - expire: entry.expire, - revalidate: entry.revalidate, - }), - options, - ); + const value = JSON.stringify({ + value: bytes.toString("base64"), + tags: entry.tags, + stale: entry.stale, + timestamp: entry.timestamp, + expire: entry.expire, + revalidate: entry.revalidate, + }); + + try { + await redis.set(ENTRY_PREFIX + cacheKey, value, options); + } catch (error) { + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis set failed (remote cache):", error); + } + } }, // Redis is the single source of truth and every read hits it, so there's no // local tag state to sync between requests. async refreshTags() {}, - // Return the most recent revalidation time across `tags`. Next treats an - // entry as stale when this is newer than the entry's `timestamp`. + // Return the most recent revalidation time across `tags`. Next.js calls + // this after a hit with the route's soft tags (the implicit `_N_T_` tags + // that `revalidatePath` uses) and discards the entry when the result is at + // or after the entry's `timestamp`. async getExpiration(tags) { + if (!tags.length) return 0; + + // If Redis can't answer, report the tags as revalidated just now: Next.js + // then discards the entry and regenerates it, the same miss `get` falls + // back to. Returning `0` would instead serve an entry that a + // `revalidatePath` may already have invalidated. const redis = await getClient(); - if (!redis || !tags.length) return 0; + if (!redis) return Date.now(); + + let values; + try { + values = await redis.mGet(tags.map((tag) => TAG_PREFIX + tag)); + } catch (error) { + if (process.env.NEXT_PRIVATE_DEBUG_CACHE) { + console.warn("Redis tag lookup failed (remote cache):", error); + } + return Date.now(); + } - const values = await redis.mGet(tags.map((tag) => TAG_PREFIX + tag)); const timestamps = values.filter(Boolean).map(Number); return timestamps.length ? Math.max(...timestamps) : 0; }, - // Record when each tag was last revalidated so `getExpiration` can report - // it. There's one key per distinct tag, overwritten in place, so a small - // fixed tag set (like this example's single `time-data` tag) never grows. + // Record when each tag was last revalidated, for `get` (the entry's own + // tags) and `getExpiration` (soft tags) to compare against. There's one key + // per distinct tag, overwritten in place, so a small fixed tag set (like + // this example's single `time-data` tag) never grows. // // An app that mints many distinct, short-lived tags (e.g. `user-`) would // instead keep a key per tag forever. To bound that, give each key a TTL @@ -140,7 +213,13 @@ module.exports = { // still cached and serve it as fresh when it should be stale. async updateTags(tags) { const redis = await getClient(); - if (!redis) return; + // Don't report success for a revalidation Redis never recorded: once Redis + // is back, every instance would serve the old entries again. + if (!redis) { + throw new Error( + "Redis is unavailable, so the tag revalidation was not recorded", + ); + } const now = String(Date.now()); await Promise.all(tags.map((tag) => redis.set(TAG_PREFIX + tag, now))); diff --git a/packages/next-swc/package.json b/packages/next-swc/package.json index a42afe4345bd..4ae347fd2afa 100644 --- a/packages/next-swc/package.json +++ b/packages/next-swc/package.json @@ -19,7 +19,8 @@ "rust-check-doc": "RUSTDOCFLAGS='-Zunstable-options --output-format=json' cargo doc --no-deps --workspace", "rust-check-fmt": "cd ../..; cargo fmt -- --check", "rust-check-napi": "cargo check -p next-napi-bindings", - "test-cargo-unit": "cargo nextest run --workspace --exclude next-napi-bindings --exclude turbo-tasks-macros --cargo-profile release-with-assertions --no-fail-fast && cargo test --workspace --doc --profile=release-with-assertions --no-fail-fast" + "test-cargo-unit": "cargo nextest run --workspace --exclude next-napi-bindings --exclude turbo-tasks-macros --cargo-profile release-with-assertions --no-fail-fast && cargo test --workspace --doc --profile=release-with-assertions --no-fail-fast", + "test-cargo-unit-miri": "MIRIFLAGS='-Zmiri-disable-isolation -Zmiri-permissive-provenance' cargo miri nextest run -p turbo-persistence -p turbo-rcstr -p turbo-tasks-malloc --no-default-features --features turbo-tasks-malloc/custom_allocator --no-fail-fast" }, "napi": { "binaryName": "next-swc", diff --git a/packages/next-swc/turbo.jsonc b/packages/next-swc/turbo.jsonc index b20c048642ba..e17a285b3664 100644 --- a/packages/next-swc/turbo.jsonc +++ b/packages/next-swc/turbo.jsonc @@ -91,5 +91,14 @@ "!../../turbopack/crates/turbopack-tests/tests/execution/**/output/*", ], }, + "test-cargo-unit-miri": { + "dependsOn": ["rust-fingerprint"], + "inputs": [ + "../../target/.rust-fingerprint", + "../../crates/*/tests/**", + "../../turbopack/crates/*/tests/**", + "!../../turbopack/crates/turbopack-tests/tests/execution/**/output/*", + ], + }, }, } diff --git a/packages/next/src/lib/upgrade/prepare-upgrade.ts b/packages/next/src/lib/upgrade/prepare-upgrade.ts index 69d5e6a9fcf6..22d0e9a342c3 100644 --- a/packages/next/src/lib/upgrade/prepare-upgrade.ts +++ b/packages/next/src/lib/upgrade/prepare-upgrade.ts @@ -37,7 +37,6 @@ export async function prepareUpgrade( await readFile(requireFromApp.resolve('next/package.json'), 'utf8') ) as { version: string - engines: { node: string | undefined } | undefined } const installedVersion = installedNext.version @@ -55,10 +54,7 @@ export async function prepareUpgrade( if (targetRequest === 'latest' || targetRequest === 'future') { const url = `${NPM_REGISTRY}next/latest` const { value } = await fetchJSON(url) - const release = value as { - version: string - engines: { node: string | undefined } | undefined - } | null + const release = value as { version: string } | null if ( !release || @@ -72,10 +68,6 @@ export async function prepareUpgrade( targetRequest === 'future' && semver.gt(installedVersion, release.version) ? installedVersion : release.version - const nodeRange = - targetVersion === installedVersion - ? (installedNext.engines?.node ?? null) - : (release.engines?.node ?? null) if ( targetRequest === 'latest' && @@ -97,12 +89,6 @@ export async function prepareUpgrade( } } - if (!nodeRange || !semver.satisfies(process.versions.node, nodeRange)) { - throw new Error( - `Next.js ${targetVersion} requires Node.js ${nodeRange ?? '(version unavailable)'}. Update Node.js before continuing.` - ) - } - let pendingFutureDefaults: FutureDefaultEntry[] = [] if (targetRequest === 'future') { diff --git a/rust-toolchain.toml b/rust-toolchain.toml index decce1d880d2..72873e38c702 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -2,5 +2,5 @@ # if you move the file, also update any turbo.json inputs that references it. [toolchain] channel = "nightly-2026-08-20" -components = ["rustfmt", "clippy", "rust-analyzer"] +components = ["rustfmt", "clippy", "rust-analyzer", "miri"] profile = "minimal" diff --git a/turbopack/crates/turbo-persistence/Cargo.toml b/turbopack/crates/turbo-persistence/Cargo.toml index 872d4f5eaf33..5f020ce82234 100644 --- a/turbopack/crates/turbo-persistence/Cargo.toml +++ b/turbopack/crates/turbo-persistence/Cargo.toml @@ -5,7 +5,8 @@ edition = "2024" license = "MIT" [features] -default = [] +default = ["mmap"] +mmap = ["dep:memmap2"] verify_sst_content = [] strict_checks = [] stats = ["quick_cache/stats"] @@ -23,7 +24,7 @@ either = { workspace = true } fs-err = { workspace = true } jiff = { version = "0.2.10", features = ["serde"] } lz4_flex = { workspace = true } -memmap2 = "0.9.5" +memmap2 = { version = "0.9.5", optional = true } nohash-hasher = { workspace = true } parking_lot = { workspace = true } # See https://github.com/vercel/next.js/issues/91708 for why we need the legacy_x86_64_support feature @@ -51,6 +52,7 @@ turbo-tasks-malloc = { workspace = true, features = ["custom_allocator"] } [[bin]] name = "sst_inspect" path = "src/bin/sst_inspect.rs" +required-features = ["mmap"] [lints] workspace = true @@ -58,3 +60,4 @@ workspace = true [[bench]] name = "mod" harness = false +required-features = ["mmap"] diff --git a/turbopack/crates/turbo-persistence/benches/mod.rs b/turbopack/crates/turbo-persistence/benches/mod.rs index bc4a5e03efaf..28b84f167775 100644 --- a/turbopack/crates/turbo-persistence/benches/mod.rs +++ b/turbopack/crates/turbo-persistence/benches/mod.rs @@ -1,3 +1,5 @@ +#![cfg(feature = "mmap")] + use std::{cell::UnsafeCell, path::Path, sync::LazyLock, time::Duration}; use anyhow::Result; diff --git a/turbopack/crates/turbo-persistence/src/arc_bytes.rs b/turbopack/crates/turbo-persistence/src/arc_bytes.rs index 8b32bf939366..960bedd409c0 100644 --- a/turbopack/crates/turbo-persistence/src/arc_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/arc_bytes.rs @@ -6,6 +6,7 @@ use std::{ sync::Arc, }; +#[cfg(feature = "mmap")] use memmap2::Mmap; use crate::{ @@ -24,6 +25,7 @@ enum Repr { data: *const [u8], _backing: Arc<[u8]>, }, + #[cfg(feature = "mmap")] Mmap { data: *const [u8], _backing: Arc, @@ -45,6 +47,7 @@ impl ArcBytes { fn backing_bytes(&self) -> Option<&[u8]> { match &self.repr { Repr::Arc { _backing, .. } => Some(_backing), + #[cfg(feature = "mmap")] Repr::Mmap { _backing, .. } => Some(_backing), Repr::Inline { .. } => None, } @@ -78,7 +81,9 @@ impl Deref for ArcBytes { match &self.repr { // SAFETY: `data` points into the backing held by the same variant, which keeps it // alive for as long as `self`. - Repr::Arc { data, .. } | Repr::Mmap { data, .. } => unsafe { &**data }, + Repr::Arc { data, .. } => unsafe { &**data }, + #[cfg(feature = "mmap")] + Repr::Mmap { data, .. } => unsafe { &**data }, // Borrowed from `self`, so this is recomputed after a move rather than stored. Repr::Inline { buf, len } => &buf[..*len as usize], } @@ -114,7 +119,11 @@ impl Eq for ArcBytes {} impl ArcBytes { /// Returns `true` if this `ArcBytes` is backed by a memory-mapped file. pub fn is_mmap_backed(&self) -> bool { - matches!(self.repr, Repr::Mmap { .. }) + #[cfg(feature = "mmap")] + return matches!(self.repr, Repr::Mmap { .. }); + + #[cfg(not(feature = "mmap"))] + false } /// Returns `true` if the backing `Arc` allocation is shared (i.e., there @@ -124,12 +133,15 @@ impl ArcBytes { pub fn is_shared_arc(&self) -> bool { match &self.repr { Repr::Arc { _backing, .. } => Arc::strong_count(_backing) > 1, - Repr::Mmap { .. } | Repr::Inline { .. } => false, + #[cfg(feature = "mmap")] + Repr::Mmap { .. } => false, + Repr::Inline { .. } => false, } } } impl SharedBytes for ArcBytes { + #[cfg(feature = "mmap")] type MmapHandle = Arc; fn slice(self, range: Range) -> Self { @@ -142,6 +154,7 @@ impl SharedBytes for ArcBytes { Self { repr: match self.repr { Repr::Arc { _backing, .. } => Repr::Arc { data, _backing }, + #[cfg(feature = "mmap")] Repr::Mmap { _backing, .. } => Repr::Mmap { data, _backing }, Repr::Inline { .. } => unreachable!("handled above"), }, @@ -168,6 +181,7 @@ impl SharedBytes for ArcBytes { data, _backing: _backing.clone(), }, + #[cfg(feature = "mmap")] Repr::Mmap { _backing, .. } => Repr::Mmap { data, _backing: _backing.clone(), @@ -179,6 +193,7 @@ impl SharedBytes for ArcBytes { } } + #[cfg(feature = "mmap")] unsafe fn from_mmap(mmap: &Arc, subslice: &[u8]) -> Self { debug_assert!( is_subslice_of(subslice, mmap), diff --git a/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs b/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs index 153edc2e9f0f..059f5911454d 100644 --- a/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs +++ b/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs @@ -264,10 +264,14 @@ fn collect_sst_info(db_path: &Path) -> Result>> { meta_seqs.sort_unstable(); + #[cfg(feature = "mmap")] + let access_mode = turbo_persistence::AccessMode::Mmap; + #[cfg(not(feature = "mmap"))] + let access_mode = turbo_persistence::AccessMode::File; let mut meta_files: Vec = meta_seqs .iter() .map(|&seq| { - MetaFile::open(db_path, seq, None, turbo_persistence::AccessMode::Mmap) + MetaFile::open(db_path, seq, None, access_mode) .with_context(|| format!("Failed to open {seq:08}.meta")) }) .collect::>()?; diff --git a/turbopack/crates/turbo-persistence/src/compaction/interval_map.rs b/turbopack/crates/turbo-persistence/src/compaction/interval_map.rs index 97101bedd90d..e404aa4e88cb 100644 --- a/turbopack/crates/turbo-persistence/src/compaction/interval_map.rs +++ b/turbopack/crates/turbo-persistence/src/compaction/interval_map.rs @@ -380,6 +380,7 @@ mod tests { } } + #[cfg(not(miri))] #[test] fn test_exhaustive_replace_versus_naive() { for_all_tiny_int_ranges([0, 1], |a| { @@ -400,6 +401,7 @@ mod tests { }); } + #[cfg(not(miri))] #[test] fn test_exhaustive_update_versus_naive() { for_all_tiny_int_ranges([1, 2], |a| { diff --git a/turbopack/crates/turbo-persistence/src/compaction/selector.rs b/turbopack/crates/turbo-persistence/src/compaction/selector.rs index 2506ab6c15ac..fa28fb058f92 100644 --- a/turbopack/crates/turbo-persistence/src/compaction/selector.rs +++ b/turbopack/crates/turbo-persistence/src/compaction/selector.rs @@ -588,6 +588,7 @@ mod tests { } } + #[cfg(not(miri))] #[test] fn simulate_compactions() { const KEY_RANGE: u64 = 10000; diff --git a/turbopack/crates/turbo-persistence/src/compression.rs b/turbopack/crates/turbo-persistence/src/compression.rs index e47ca5f9b739..a53f58bbfcf2 100644 --- a/turbopack/crates/turbo-persistence/src/compression.rs +++ b/turbopack/crates/turbo-persistence/src/compression.rs @@ -1,6 +1,11 @@ -use std::{cell::RefCell, mem::MaybeUninit, rc::Rc, sync::Arc}; +#[cfg(not(miri))] +use std::cell::RefCell; +use std::{mem::MaybeUninit, rc::Rc, sync::Arc}; -use anyhow::{Context, Result, ensure}; +#[cfg(not(miri))] +use anyhow::Context; +use anyhow::{Result, ensure}; +#[cfg(not(miri))] use lz4_flex::block::{ CompressTable, compress_into_with_table, decompress_into, get_maximum_output_size, }; @@ -16,6 +21,7 @@ pub enum Compression { Zstd3 = 1, } +#[cfg(not(miri))] thread_local! { /// Reuse lz4_flex's large hash table across independent blocks. Starting large improves /// compression speed and produces faster-to-decode streams for typical persistence blocks. @@ -37,30 +43,43 @@ fn decompress_block( ) -> Result<()> { debug_assert!( expected_len > 0, - "decompress_block called with uncompressed_length=0; uncompressed blocks should use \ - zero-copy mmap path" + "decompress_block called with uncompressed_length=0; uncompressed blocks are served \ + directly from their backing" ); - let bytes_written = match compression { - Compression::Lz4 => decompress_into(block, dest).map_err(anyhow::Error::from), - Compression::Zstd3 => ZSTD_DECOMPRESSOR.with_borrow_mut(|decompressor| { - decompressor - .decompress_to_buffer(block, dest) - .map_err(anyhow::Error::from) - }), + #[cfg(not(miri))] + { + let bytes_written = match compression { + Compression::Lz4 => decompress_into(block, dest).map_err(anyhow::Error::from), + Compression::Zstd3 => ZSTD_DECOMPRESSOR.with_borrow_mut(|decompressor| { + decompressor + .decompress_to_buffer(block, dest) + .map_err(anyhow::Error::from) + }), + } + .with_context(|| { + format!( + "Failed to decompress {compression:?} block ({} bytes compressed, {} bytes \ + uncompressed)", + block.len(), + expected_len + ) + })?; + ensure!( + bytes_written == expected_len as usize, + "Decompressed length does not match expected length: decompressed {bytes_written} \ + bytes, expected {expected_len}" + ); + } + #[cfg(miri)] + { + // Compression is skipped under Miri, so Miri-created blob payloads are verbatim. + let _ = compression; + ensure!( + block.len() == expected_len as usize, + "Miri builds skip compression, so a compressed block cannot be read under Miri" + ); + dest.copy_from_slice(block); } - .with_context(|| { - format!( - "Failed to decompress {compression:?} block ({} bytes compressed, {} bytes \ - uncompressed)", - block.len(), - expected_len - ) - })?; - ensure!( - bytes_written == expected_len as usize, - "Decompressed length does not match expected length: decompressed {bytes_written} bytes, \ - expected {expected_len}" - ); Ok(()) } @@ -108,18 +127,24 @@ pub fn checksum_block(data: &[u8]) -> u32 { /// Reusable compressor for a stream of blocks using the same family configuration. pub(crate) struct Compressor { compression: Compression, + #[cfg(not(miri))] zstd: Option>, } impl Compressor { pub(crate) fn new(compression: Compression) -> Result { + #[cfg(not(miri))] let zstd = match compression { Compression::Zstd3 => { Some(zstd::bulk::Compressor::new(3).context("Failed to create zstd compressor")?) } Compression::Lz4 => None, }; - Ok(Self { compression, zstd }) + Ok(Self { + compression, + #[cfg(not(miri))] + zstd, + }) } /// Compresses `block` into reusable storage, replacing its contents. @@ -130,6 +155,7 @@ impl Compressor { buffer: &mut Vec, ) -> Result<()> { buffer.clear(); + #[cfg(not(miri))] match self.compression { Compression::Lz4 => { let max_output_size = get_maximum_output_size(block.len()); @@ -156,6 +182,14 @@ impl Compressor { .context("zstd compression failed")?; } } + #[cfg(miri)] + { + // Compression is deliberately skipped under Miri. This avoids native Zstd, and using + // the same raw representation for both algorithms keeps blob reads on the matching + // copy path above. The caller's savings check stores SST blocks as uncompressed. + let _ = self.compression; + buffer.extend_from_slice(block); + } Ok(()) } } diff --git a/turbopack/crates/turbo-persistence/src/db.rs b/turbopack/crates/turbo-persistence/src/db.rs index 531224fa0769..fade89645792 100644 --- a/turbopack/crates/turbo-persistence/src/db.rs +++ b/turbopack/crates/turbo-persistence/src/db.rs @@ -17,9 +17,11 @@ use anyhow::{Context, Result, bail}; use auto_hash_map::AutoSet; use byteorder::{BE, ReadBytesExt, WriteBytesExt}; use dashmap::DashSet; +#[cfg(feature = "mmap")] use either::Either; use fs_err::{self as fs, File, OpenOptions, ReadDir}; use jiff::Timestamp; +#[cfg(feature = "mmap")] use memmap2::Mmap; use nohash_hasher::BuildNoHashHasher; use parking_lot::{Mutex, RwLock}; @@ -29,8 +31,10 @@ use smallvec::SmallVec; use tracing::span::EnteredSpan; pub use crate::compaction::selector::CompactConfig; +#[cfg(feature = "mmap")] +use crate::{AccessMode, mmap_helper::advise_mmap_for_persistence}; use crate::{ - AccessMode, DbConfig, FamilyKind, QueryKey, + DbConfig, FamilyKind, QueryKey, arc_bytes::ArcBytes, compaction::selector::{Compactable, get_merge_segments}, compression::{Compression, checksum_block, decompress_into_arc}, @@ -43,7 +47,6 @@ use crate::{ merge_iter::MergeIter, meta_file::{MetaEntryFlags, MetaFile, MetaLookupResult, StaticSortedFileRange}, meta_file_builder::MetaFileBuilder, - mmap_helper::advise_mmap_for_persistence, parallel_scheduler::ParallelScheduler, rc_bytes::RcBytes, sst_filter::SstFilter, @@ -703,7 +706,9 @@ impl TurboPersistence #[tracing::instrument(level = "info", name = "reading database blob", skip_all)] fn read_blob(&self, seq: u32, compression: Compression) -> Result { let path = self.path.join(format!("{seq:08}.blob")); + #[cfg(feature = "mmap")] let file = File::open(&path)?; + #[cfg(feature = "mmap")] let data: Either> = match self.config.access_mode { AccessMode::Mmap => { let mmap = unsafe { Mmap::map(file.file()) }.with_context(|| { @@ -722,10 +727,16 @@ impl TurboPersistence } AccessMode::File => Either::Right(fs::read(&path)?), }; + #[cfg(feature = "mmap")] let mut reader: &[u8] = match &data { Either::Left(mmap) => mmap, Either::Right(bytes) => bytes, }; + // Without mmap support, read the whole blob into memory. + #[cfg(not(feature = "mmap"))] + let data = fs::read(&path)?; + #[cfg(not(feature = "mmap"))] + let mut reader: &[u8] = &data; let uncompressed_length = reader .read_u32::() .context("Failed to read uncompressed length from blob file")?; diff --git a/turbopack/crates/turbo-persistence/src/lib.rs b/turbopack/crates/turbo-persistence/src/lib.rs index 67fe6ad4ad09..b9e34d6bd5ea 100644 --- a/turbopack/crates/turbo-persistence/src/lib.rs +++ b/turbopack/crates/turbo-persistence/src/lib.rs @@ -15,6 +15,7 @@ mod lookup_entry; mod merge_iter; pub mod meta_file; mod meta_file_builder; +#[cfg(feature = "mmap")] pub mod mmap_helper; mod parallel_scheduler; mod rc_bytes; @@ -40,6 +41,7 @@ pub use db::{ #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AccessMode { /// Memory-map the file and access blocks via the mapped region. + #[cfg(feature = "mmap")] Mmap, /// Read blocks directly from the file via pread (no mmap). File, @@ -78,8 +80,32 @@ pub struct DbConfig { pub access_mode: AccessMode, } -/// Reads the `TURBO_PERSISTENCE_MMAP` env var (cached). Returns `AccessMode::File` when the var -/// is set to `"0"`, `AccessMode::Mmap` otherwise. +/// Returns the default access mode for this execution environment. +/// +/// Builds without mmap support always use file I/O. Builds with mmap support honor +/// `TURBO_PERSISTENCE_MMAP=0`; mmap remains the default otherwise. +fn default_access_mode() -> AccessMode { + #[cfg(not(feature = "mmap"))] + return AccessMode::File; + + #[cfg(feature = "mmap")] + access_mode_env_var() +} + +/// Returns mmap mode when the feature is enabled, and file mode otherwise. +/// +/// Call sites that specifically want mmap use this helper because `AccessMode::Mmap` does not +/// exist without the feature; they fall back to file I/O and still exercise surrounding logic. +#[cfg(any(test, feature = "verify_sst_content"))] +pub(crate) fn mmap_access_mode() -> AccessMode { + #[cfg(not(feature = "mmap"))] + return AccessMode::File; + + #[cfg(feature = "mmap")] + AccessMode::Mmap +} + +#[cfg(feature = "mmap")] fn access_mode_env_var() -> AccessMode { static ACCESS_MODE_ENV: std::sync::LazyLock = std::sync::LazyLock::new(|| { if std::env::var("TURBO_PERSISTENCE_MMAP") @@ -95,8 +121,7 @@ fn access_mode_env_var() -> AccessMode { } impl DbConfig { - /// Returns a config with all defaults, reading the `TURBO_PERSISTENCE_MMAP` env var - /// to determine the access mode. + /// Returns a config with all defaults, using the execution environment's default access mode. pub fn new() -> Self { Self { family_configs: [FamilyConfig { @@ -104,7 +129,7 @@ impl DbConfig { kind: FamilyKind::SingleValue, compression: Compression::Lz4, }; FAMILIES], - access_mode: access_mode_env_var(), + access_mode: default_access_mode(), } } } diff --git a/turbopack/crates/turbo-persistence/src/meta_file.rs b/turbopack/crates/turbo-persistence/src/meta_file.rs index e2948c70cf71..580c419a67b3 100644 --- a/turbopack/crates/turbo-persistence/src/meta_file.rs +++ b/turbopack/crates/turbo-persistence/src/meta_file.rs @@ -4,21 +4,24 @@ use std::{ mem::take, ops::Deref, path::{Path, PathBuf}, - sync::OnceLock, + sync::{Arc, OnceLock}, }; use anyhow::{Context, Result, bail, ensure}; use bitfield::bitfield; use byteorder::{BE, ReadBytesExt}; +#[cfg(feature = "mmap")] use fs_err::File; +#[cfg(feature = "mmap")] use memmap2::{Mmap, MmapOptions}; use smallvec::SmallVec; use zerocopy::{FromBytes, Immutable, IntoBytes, KnownLayout, Ref, big_endian as be}; +#[cfg(feature = "mmap")] +use crate::mmap_helper::advise_mmap_for_persistence; use crate::{ AccessMode, Compression, FamilyConfig, QueryKey, lookup_entry::LookupValue, - mmap_helper::advise_mmap_for_persistence, static_sorted_file::{BlockCache, SstLookupResult, StaticSortedFile, StaticSortedFileMetaData}, }; @@ -229,8 +232,15 @@ impl StaticSortedFileRange { } enum MetaFileBacking { + #[cfg(feature = "mmap")] Mmap(Mmap), - Bytes(Box<[u8]>), + /// Heap bytes for [`AccessMode::File`]. + /// + /// This is an `Arc<[u8]>` rather than a `Box<[u8]>` so that moving the backing into + /// [`MetaFile`] does not reborrow the bytes: a `Box` is a unique pointer, so the move + /// invalidates the `FilterRef`s that already borrow from it, which Miri reports as undefined + /// behavior under Stacked Borrows. An `Arc` moves its handle without retagging the allocation. + Bytes(Arc<[u8]>), } impl Deref for MetaFileBacking { @@ -238,6 +248,7 @@ impl Deref for MetaFileBacking { fn deref(&self) -> &Self::Target { match self { + #[cfg(feature = "mmap")] MetaFileBacking::Mmap(mmap) => mmap, MetaFileBacking::Bytes(bytes) => bytes, } @@ -307,6 +318,7 @@ impl MetaFile { access_mode: AccessMode, ) -> Result { let backing = match access_mode { + #[cfg(feature = "mmap")] AccessMode::Mmap => { let file = File::open(path)?; let mmap = unsafe { MmapOptions::new().map(file.file()) } @@ -317,7 +329,7 @@ impl MetaFile { advise_mmap_for_persistence(&mmap)?; MetaFileBacking::Mmap(mmap) } - AccessMode::File => MetaFileBacking::Bytes(fs_err::read(path)?.into_boxed_slice()), + AccessMode::File => MetaFileBacking::Bytes(fs_err::read(path)?.into()), }; // Parse the header from stable backing bytes via ReadBytesExt on &[u8]. let mut reader: &[u8] = &backing; diff --git a/turbopack/crates/turbo-persistence/src/rc_bytes.rs b/turbopack/crates/turbo-persistence/src/rc_bytes.rs index be380887b112..adb327728a08 100644 --- a/turbopack/crates/turbo-persistence/src/rc_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/rc_bytes.rs @@ -6,6 +6,7 @@ use std::{ rc::Rc, }; +#[cfg(feature = "mmap")] use memmap2::Mmap; use crate::{ @@ -24,6 +25,7 @@ enum Repr { data: *const [u8], _backing: Rc<[u8]>, }, + #[cfg(feature = "mmap")] Mmap { data: *const [u8], _backing: Rc, @@ -48,6 +50,7 @@ impl RcBytes { fn backing_bytes(&self) -> Option<&[u8]> { match &self.repr { Repr::Rc { _backing, .. } => Some(_backing), + #[cfg(feature = "mmap")] Repr::Mmap { _backing, .. } => Some(_backing), Repr::Inline { .. } => None, } @@ -78,7 +81,9 @@ impl Deref for RcBytes { match &self.repr { // SAFETY: `data` points into the backing held by the same variant, which keeps it // alive for as long as `self`. - Repr::Rc { data, .. } | Repr::Mmap { data, .. } => unsafe { &**data }, + Repr::Rc { data, .. } => unsafe { &**data }, + #[cfg(feature = "mmap")] + Repr::Mmap { data, .. } => unsafe { &**data }, // Borrowed from `self`, so this is recomputed after a move rather than stored. Repr::Inline { buf, len } => &buf[..*len as usize], } @@ -112,6 +117,7 @@ impl Hash for RcBytes { } impl SharedBytes for RcBytes { + #[cfg(feature = "mmap")] type MmapHandle = Rc; fn slice(self, range: Range) -> Self { @@ -124,6 +130,7 @@ impl SharedBytes for RcBytes { Self { repr: match self.repr { Repr::Rc { _backing, .. } => Repr::Rc { data, _backing }, + #[cfg(feature = "mmap")] Repr::Mmap { _backing, .. } => Repr::Mmap { data, _backing }, Repr::Inline { .. } => unreachable!("handled above"), }, @@ -149,6 +156,7 @@ impl SharedBytes for RcBytes { data, _backing: _backing.clone(), }, + #[cfg(feature = "mmap")] Repr::Mmap { _backing, .. } => Repr::Mmap { data, _backing: _backing.clone(), @@ -160,6 +168,7 @@ impl SharedBytes for RcBytes { } } + #[cfg(feature = "mmap")] unsafe fn from_mmap(mmap: &Rc, subslice: &[u8]) -> Self { debug_assert!( is_subslice_of(subslice, mmap), diff --git a/turbopack/crates/turbo-persistence/src/shared_bytes.rs b/turbopack/crates/turbo-persistence/src/shared_bytes.rs index 92e887ff16cc..ac94095f8832 100644 --- a/turbopack/crates/turbo-persistence/src/shared_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/shared_bytes.rs @@ -1,5 +1,6 @@ use std::ops::{Deref, Range}; +#[cfg(feature = "mmap")] use memmap2::Mmap; use crate::Compression; @@ -30,6 +31,7 @@ const _: () = assert!( /// once and used for both lookup (ArcBytes) and iteration (RcBytes) paths. pub trait SharedBytes: Clone + Deref + Sized { /// The ref-counted handle to the memory-mapped file. + #[cfg(feature = "mmap")] type MmapHandle: Deref; /// Returns a new instance that points to a sub-range of the current slice. @@ -50,6 +52,7 @@ pub trait SharedBytes: Clone + Deref + Sized { /// /// The caller must ensure that `subslice` points to memory within the /// given `mmap`. + #[cfg(feature = "mmap")] unsafe fn from_mmap(mmap: &Self::MmapHandle, subslice: &[u8]) -> Self; /// Creates an instance from a decompressed block. diff --git a/turbopack/crates/turbo-persistence/src/static_sorted_file.rs b/turbopack/crates/turbo-persistence/src/static_sorted_file.rs index 5107ab11922c..7c3f16168aed 100644 --- a/turbopack/crates/turbo-persistence/src/static_sorted_file.rs +++ b/turbopack/crates/turbo-persistence/src/static_sorted_file.rs @@ -1,9 +1,11 @@ +#[cfg(feature = "mmap")] +use std::ops::Range; use std::{ borrow::Cow, cmp::Ordering, hash::BuildHasherDefault, io, - ops::Range, + marker::PhantomData, path::Path, rc::Rc, sync::{ @@ -14,11 +16,14 @@ use std::{ use anyhow::{Context, Result, bail, ensure}; use fs_err::File; +#[cfg(feature = "mmap")] use memmap2::Mmap; use quick_cache::{Lifecycle, sync::GuardResult}; use rustc_hash::FxHasher; use smallvec::SmallVec; +#[cfg(feature = "mmap")] +use crate::mmap_helper::advise_mmap_for_persistence; use crate::{ AccessMode, Compression, QueryKey, arc_bytes::ArcBytes, @@ -26,7 +31,6 @@ use crate::{ compression::checksum_block, constants::MAX_INLINE_VALUE_SIZE, lookup_entry::{IterValue, LookupEntry, LookupValue}, - mmap_helper::advise_mmap_for_persistence, rc_bytes::RcBytes, shared_bytes::SharedBytes, static_sorted_file_builder::{ @@ -315,6 +319,7 @@ impl StaticSortedFileMetaData { } enum StaticSortedFileBacking { + #[cfg(feature = "mmap")] Mmap(Arc), File { file: Arc, @@ -360,6 +365,7 @@ enum IndexEntries { /// borrow from its own `backing` field, and an `ArcBytes` would bump and drop the `mmap` /// refcount on every lookup. All readers of a file share that one counter, so the contention /// scales with reader threads — measured ~3 ns single-threaded but ~70 ns at 8 threads. + #[cfg(feature = "mmap")] Mmap(Range), /// Read into memory at open time, for the non-mmap backing, which has nothing to borrow from. Owned(Box<[u8]>), @@ -419,6 +425,7 @@ impl IndexBlock { let entries = match backing { // Store a range, not the slice: `StaticSortedFile` owns the mmap these bytes live in. + #[cfg(feature = "mmap")] StaticSortedFileBacking::Mmap(mmap) => { let start = entry_bytes.as_ptr() as usize - mmap.as_ptr() as usize; IndexEntries::Mmap(start..start + entry_bytes.len()) @@ -447,6 +454,7 @@ impl StaticSortedFile { let path = db_path.join(&filename); let file = File::open(&path)?; let backing = match access_mode { + #[cfg(feature = "mmap")] AccessMode::Mmap => { let mmap = unsafe { Mmap::map(file.file()) }.with_context(|| { format!( @@ -504,12 +512,14 @@ impl StaticSortedFile { #[inline] fn index_entries(&self) -> &[[u8; INDEX_BLOCK_ENTRY_SIZE]] { let bytes = match (&self.index.entries, &self.backing) { + #[cfg(feature = "mmap")] (IndexEntries::Mmap(range), StaticSortedFileBacking::Mmap(mmap)) => { &mmap[range.clone()] } (IndexEntries::Owned(bytes), _) => &bytes[..], // `IndexBlock::parse` only produces `Mmap` entries for an mmap backing, and the // backing never changes after open. + #[cfg(feature = "mmap")] (IndexEntries::Mmap(_), StaticSortedFileBacking::File { .. }) => unreachable!( "mmap-ranged index entries with a file backing in {:08}.sst", self.meta.sequence_number @@ -749,17 +759,20 @@ impl StaticSortedFile { /// its refcount belongs to one cache entry rather than the whole file. enum BlockRef<'l> { /// Borrowed from the memory-mapped file. + #[cfg(feature = "mmap")] Mmap(&'l [u8]), - /// Owned, and shared with the block cache. - Cached(ArcBytes), + /// Owned, and shared with the block cache. The zero-sized marker keeps the backing borrow + /// lifetime represented in builds where the mmap variant is disabled. + Cached(ArcBytes, PhantomData<&'l ()>), } impl BlockRef<'_> { #[inline] fn as_slice(&self) -> &[u8] { match self { + #[cfg(feature = "mmap")] BlockRef::Mmap(data) => data, - BlockRef::Cached(block) => block, + BlockRef::Cached(block, _) => block, } } @@ -767,8 +780,10 @@ impl BlockRef<'_> { /// /// Only needed by callers that hand the bytes to something outliving the lookup. #[inline] + #[cfg_attr(not(feature = "mmap"), allow(unused_variables))] fn into_owned(self, backing: &StaticSortedFileBacking) -> ArcBytes { match self { + #[cfg(feature = "mmap")] BlockRef::Mmap(data) => { let StaticSortedFileBacking::Mmap(mmap) = backing else { // `get_or_read_block` only borrows from an mmap backing. @@ -777,7 +792,7 @@ impl BlockRef<'_> { // SAFETY: the borrow came from this mmap, via `get_or_read_block`. unsafe { ArcBytes::from_mmap(mmap, data) } } - BlockRef::Cached(block) => block, + BlockRef::Cached(block, _) => block, } } } @@ -799,6 +814,11 @@ fn get_or_read_block<'l>( verified_blocks: &[AtomicU64], compression: Compression, ) -> Result> { + // `verified_blocks` tracks which mmap-backed blocks already passed their checksum. There is + // no mmap backing without the feature, so the bitmap is unused there. + #[cfg(not(feature = "mmap"))] + let _ = verified_blocks; + #[cfg(feature = "mmap")] let mmap_block = if let StaticSortedFileBacking::Mmap(mmap) = backing { let (uncompressed_length, checksum, block_data) = get_raw_block_slice(mmap, meta, block_index).with_context(|| { @@ -818,6 +838,10 @@ fn get_or_read_block<'l>( } else { None }; + // Without an mmap backing there is never a zero-copy block to borrow, so every block goes + // through the decompress/cache path below. + #[cfg(not(feature = "mmap"))] + let mmap_block: Option<(u32, u32, &[u8])> = None; // Compressed: check cache; decompress and insert on miss. // File-backed blocks use the same cache, including uncompressed ones. @@ -834,6 +858,7 @@ fn get_or_read_block<'l>( // A cached block may have been evicted, so re-reading still // benefits from the bitmap to skip redundant CRC verification. match backing { + #[cfg(feature = "mmap")] StaticSortedFileBacking::Mmap(_) => verify_checksum_once( meta, &block_data, @@ -862,11 +887,13 @@ fn get_or_read_block<'l>( } GuardResult::Timeout => unreachable!(), }, + PhantomData, )) } /// Gets the raw block slice directly from a memory-mapped file. /// Returns `(uncompressed_length, checksum, block_data)`. +#[cfg(feature = "mmap")] fn get_raw_block_slice<'a>( mmap: &'a Mmap, meta: &StaticSortedFileMetaData, @@ -932,6 +959,7 @@ fn get_raw_block<'a>( block_index: u16, ) -> Result<(u32, u32, Cow<'a, [u8]>)> { match backing { + #[cfg(feature = "mmap")] StaticSortedFileBacking::Mmap(mmap) => { let (uncompressed_length, checksum, block) = get_raw_block_slice(mmap, meta, block_index)?; @@ -1026,6 +1054,7 @@ fn verify_checksum( /// since the check is deterministic and idempotent. Verification failures are /// *not* recorded in the bitmap, so a corrupted block will be re-checked (and /// fail again) on every access. +#[cfg_attr(not(feature = "mmap"), allow(dead_code))] fn verify_checksum_once( meta: &StaticSortedFileMetaData, data: &[u8], @@ -1054,6 +1083,7 @@ fn read_block_lookup( verify_checksum(meta, &block, checksum, block_index)?; if uncompressed_length == 0 { return match (backing, block) { + #[cfg(feature = "mmap")] (StaticSortedFileBacking::Mmap(mmap), Cow::Borrowed(block)) => { // SAFETY: block points into mmap. Ok(unsafe { ArcBytes::from_mmap(mmap, block) }) @@ -1077,6 +1107,7 @@ fn get_raw_block_iter( block_index: u16, ) -> Result<(u32, u32, RcBytes)> { match backing { + #[cfg(feature = "mmap")] StaticSortedFileIterBacking::Mmap(mmap) => { let (uncompressed_length, checksum, block) = get_raw_block_slice(mmap, meta, block_index)?; @@ -1184,6 +1215,7 @@ fn handle_key_match_generic( } enum StaticSortedFileIterBacking { + #[cfg(feature = "mmap")] Mmap(Rc), File { file: Rc, @@ -1290,6 +1322,7 @@ impl StaticSortedFileIter { let path = db_path.join(&filename); let file = File::open(&path)?; let backing = match access_mode { + #[cfg(feature = "mmap")] AccessMode::Mmap => { let mmap = unsafe { Mmap::map(file.file()) }.with_context(|| { format!( diff --git a/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs b/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs index 2cefa8f15ec3..fb08faea71b1 100644 --- a/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs +++ b/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs @@ -1434,7 +1434,6 @@ impl IndexBlockBuilder { mod tests { use super::*; use crate::{ - AccessMode, key::hash_key, lookup_entry::LookupValue, static_sorted_file::{ @@ -1566,7 +1565,7 @@ mod tests { block_count: meta.block_count, }, Compression::Lz4, - AccessMode::Mmap, + crate::mmap_access_mode(), ) } @@ -1913,7 +1912,7 @@ mod tests { block_count: meta1.block_count, }, Compression::Lz4, - AccessMode::Mmap, + crate::mmap_access_mode(), )?; let sst2 = StaticSortedFile::open( dir.path(), @@ -1922,7 +1921,7 @@ mod tests { block_count: meta2.block_count, }, Compression::Lz4, - AccessMode::Mmap, + crate::mmap_access_mode(), )?; let kc = make_cache(); let vc = make_cache(); diff --git a/turbopack/crates/turbo-persistence/src/tests.rs b/turbopack/crates/turbo-persistence/src/tests.rs index 524116129d6a..a28d10be11d2 100644 --- a/turbopack/crates/turbo-persistence/src/tests.rs +++ b/turbopack/crates/turbo-persistence/src/tests.rs @@ -1,23 +1,36 @@ -use std::{fs, path::Path, time::Instant}; +#[cfg(not(miri))] +use std::time::Instant; +use std::{fs, path::Path}; use anyhow::Result; +#[cfg(not(miri))] use rayon::iter::{IntoParallelIterator, ParallelIterator}; use rstest::rstest; +// Miri rejects a process that exits while rayon's global worker threads are still parked, so +// run the same tests on the serial scheduler there. The alias keeps the test bodies identical. +#[cfg(miri)] +use crate::parallel_scheduler::SerialScheduler as RayonParallelScheduler; use crate::{ AccessMode, Compression, DbConfig, FamilyConfig, FamilyKind, - constants::{MAX_INLINE_VALUE_SIZE, MAX_MEDIUM_VALUE_SIZE, MAX_SMALL_VALUE_SIZE}, + constants::MAX_INLINE_VALUE_SIZE, db::{CompactConfig, TurboPersistence, read_current_version}, lookup_entry::IterValue, + static_sorted_file::{StaticSortedFileIter, StaticSortedFileMetaData}, +}; +#[cfg(not(miri))] +use crate::{ + constants::{MAX_MEDIUM_VALUE_SIZE, MAX_SMALL_VALUE_SIZE}, meta_file::MetaFile, parallel_scheduler::ParallelScheduler, - static_sorted_file::{StaticSortedFileIter, StaticSortedFileMetaData}, write_batch::WriteBatch, }; +#[cfg(not(miri))] #[derive(Clone, Copy)] struct RayonParallelScheduler; +#[cfg(not(miri))] impl ParallelScheduler for RayonParallelScheduler { fn block_in_place(&self, f: impl FnOnce() -> R + Send) -> R where @@ -108,6 +121,7 @@ impl ParallelScheduler for RayonParallelScheduler { } } +#[cfg(not(miri))] fn tuple_key(prefix: u8, suffix: [u8; 4]) -> Box<[u8]> { let mut key = Vec::with_capacity(1 + suffix.len()); key.push(prefix); @@ -118,7 +132,7 @@ fn tuple_key(prefix: u8, suffix: [u8; 4]) -> Box<[u8]> { fn config_with_mmap(mmap: bool) -> DbConfig { DbConfig { access_mode: if mmap { - AccessMode::Mmap + crate::mmap_access_mode() } else { AccessMode::File }, @@ -152,7 +166,7 @@ fn multi_value_config_with_mmap(mmap: bool) -> DbConfig<1> { compression: Compression::Lz4, }], access_mode: if mmap { - AccessMode::Mmap + crate::mmap_access_mode() } else { AccessMode::File }, @@ -166,6 +180,9 @@ fn open_multi_value_db( open_db_with_config(path, multi_value_config_with_mmap(mmap)) } +// This stress test writes more than ten million entries and uses Rayon directly, so it is too slow +// to run under Miri and depends on concurrency that Miri cannot provide efficiently. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -552,6 +569,9 @@ fn full_cycle(#[case] mmap: bool) -> Result<()> { Ok(()) } +// This test exceeded 20 minutes under Miri because it writes and repeatedly reads tens of +// thousands of entries across multiple reopen/compaction cycles. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -664,6 +684,8 @@ fn persist_changes(#[case] mmap: bool) -> Result<()> { Ok(()) } +// This 50-iteration compaction/restore stress test exceeded 20 minutes under Miri. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -751,6 +773,8 @@ fn partial_compaction(#[case] mmap: bool) -> Result<()> { Ok(()) } +// This repeated large-file merge/removal stress test exceeded 20 minutes under Miri. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -1047,6 +1071,8 @@ fn batch_get_large_batch(#[case] mmap: bool) -> Result<()> { Ok(()) } +// Crossing the real blob-size boundary makes this test exceed 20 minutes under Miri. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -1372,6 +1398,8 @@ fn batch_get_after_restore(#[case] mmap: bool) -> Result<()> { /// Test that compaction works with many small values without overflowing block indices. /// Reproduces a CI benchmark failure with key_4/value_512/entries_1.98Mi/compacted. +// Miri was killed while processing this production-scale, two-million-entry workload. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -1418,6 +1446,8 @@ fn many_small_values_compaction(#[case] mmap: bool) -> Result<()> { /// Test compaction with MAX_SMALL_VALUE_SIZE (4096-byte) values. /// Worst case for small value blocks: fewest entries per block. +// This production-scale 512K-entry workload exceeded 20 minutes under Miri. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -1463,6 +1493,8 @@ fn many_max_small_values_compaction(#[case] mmap: bool) -> Result<()> { /// Test compaction with 4097-byte values (minimum medium size). /// Each medium value gets its own dedicated block, so this is the worst case for block count. +// This production-scale 128K-entry workload exceeded 20 minutes under Miri. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -1974,6 +2006,7 @@ fn multi_value_tombstone_shadows_older_sst_only(#[case] mmap: bool) -> Result<() } /// Returns the number of `.blob` files in the given directory. +#[cfg(not(miri))] fn count_blob_files(dir: &Path) -> usize { fs::read_dir(dir) .unwrap() @@ -1984,6 +2017,9 @@ fn count_blob_files(dir: &Path) -> usize { /// Test that compaction deletes blob files when their entries are superseded /// by newer values (SingleValue family). +// The first access-mode variant exceeded 20 minutes under Miri while processing the production-size +// blob boundary. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -2043,6 +2079,9 @@ fn compaction_deletes_superseded_blob(#[case] mmap: bool) -> Result<()> { /// Test that compaction deletes blob files when a key is deleted via tombstone /// (SingleValue family). +// The first access-mode variant exceeded 20 minutes under Miri while processing the production-size +// blob boundary. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -2094,6 +2133,9 @@ fn compaction_deletes_blob_on_tombstone(#[case] mmap: bool) -> Result<()> { /// Test that compaction deletes blob files for MultiValue families when a /// tombstone prunes older blob entries. +// The first access-mode variant exceeded 20 minutes under Miri while processing the production-size +// blob boundary. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -2143,6 +2185,9 @@ fn compaction_deletes_blob_multi_value_tombstone(#[case] mmap: bool) -> Result<( /// Test that compaction preserves blob files that are still referenced /// (not superseded). +// The first access-mode variant exceeded 20 minutes under Miri while processing the production-size +// blob boundary. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -2457,7 +2502,9 @@ fn count_tombstones( sequence_number: entry.sequence_number, block_count: entry.block_count, }; - for item in StaticSortedFileIter::open(path, sst, Compression::Lz4, AccessMode::Mmap)? { + for item in + StaticSortedFileIter::open(path, sst, Compression::Lz4, crate::mmap_access_mode())? + { if matches!( item?.value, IterValue::KeyDeleted | IterValue::KeyValueDeleted { .. } @@ -2634,6 +2681,8 @@ fn compaction_keeps_tombstone_when_older_sst_has_the_key() -> Result<()> { /// /// This is the case a sequence-number threshold gets wrong — it would treat the skipped SST as /// part of the job and drop a tombstone that is still load-bearing. +// This multi-SST partial-compaction scenario exceeded 20 minutes under Miri. +#[cfg(not(miri))] #[test] fn compaction_keeps_tombstone_when_skipped_sst_has_the_key() -> Result<()> { let tempdir = tempfile::tempdir()?; @@ -2814,6 +2863,8 @@ fn valued_tombstone_rejects_single_value_families() -> Result<()> { Ok(()) } +// This 8,000-key meta-sharding compaction test exceeded 20 minutes under Miri. +#[cfg(not(miri))] #[rstest] #[case(true)] #[case(false)] @@ -2821,7 +2872,7 @@ fn partial_compaction_retires_fully_consumed_meta_files(#[case] mmap: bool) -> R let tempdir = tempfile::tempdir()?; let path = tempdir.path(); let access_mode = if mmap { - AccessMode::Mmap + crate::mmap_access_mode() } else { AccessMode::File }; diff --git a/turbopack/crates/turbo-persistence/src/write_batch.rs b/turbopack/crates/turbo-persistence/src/write_batch.rs index 9c6a065f851e..c03ad1c407dc 100644 --- a/turbopack/crates/turbo-persistence/src/write_batch.rs +++ b/turbopack/crates/turbo-persistence/src/write_batch.rs @@ -533,7 +533,6 @@ impl<'db, K: StoreKey + Send + Sync, S: ParallelScheduler, const FAMILIES: usize use core::panic; use crate::{ - AccessMode, collector_entry::CollectorEntryValue, key::hash_key, lookup_entry::LookupValue, @@ -551,7 +550,7 @@ impl<'db, K: StoreKey + Send + Sync, S: ParallelScheduler, const FAMILIES: usize block_count: meta.block_count, }, self.family_configs[usize_from_u32(family)].compression, - AccessMode::Mmap, + crate::mmap_access_mode(), )?; let cache2 = BlockCache::with( 10, diff --git a/turbopack/crates/turbo-rcstr/src/dynamic.rs b/turbopack/crates/turbo-rcstr/src/dynamic.rs index 81de8a7838c9..80b08344613e 100644 --- a/turbopack/crates/turbo-rcstr/src/dynamic.rs +++ b/turbopack/crates/turbo-rcstr/src/dynamic.rs @@ -77,12 +77,12 @@ pub(crate) fn new_atom(text: Cow<'_, str>) -> RcStr { /// Construct a new dynamic RcStr from a DynamicPrehashedString pub(crate) fn new_atom_from_prehashed(prehashed: DynamicPrehashedString) -> RcStr { let entry: Arc = Arc::new(prehashed); - let mut entry = Arc::into_raw(entry); - debug_assert!(0 == entry as u8 & TAG_MASK); - entry = ((entry as usize) | DYNAMIC_TAG as usize) as *mut DynamicPrehashedString; + let entry = Arc::into_raw(entry); + debug_assert_eq!(entry.addr() & TAG_MASK as usize, 0); + let entry = entry.map_addr(|addr| addr | DYNAMIC_TAG as usize); let ptr: NonNull = unsafe { // Safety: Arc::into_raw returns a non-null pointer - NonNull::new_unchecked(entry as *mut _) + NonNull::new_unchecked(entry.cast_mut()) }; RcStr { diff --git a/turbopack/crates/turbo-rcstr/src/lib.rs b/turbopack/crates/turbo-rcstr/src/lib.rs index 7fd8ae6a878f..6a09869e41e8 100644 --- a/turbopack/crates/turbo-rcstr/src/lib.rs +++ b/turbopack/crates/turbo-rcstr/src/lib.rs @@ -951,6 +951,8 @@ mod tests { let decoded: RcStr = turbo_bincode_decode(&encoded).unwrap(); assert_eq!(decoded.as_str(), STATIC_STR); // Decoded via peek_read path should find the static constant + // Miri does not populate the linker section used by scattered-collect. + #[cfg(not(miri))] assert_eq!(decoded.tag(), STATIC_TAG); } diff --git a/turbopack/crates/turbo-rcstr/src/tagged_value.rs b/turbopack/crates/turbo-rcstr/src/tagged_value.rs index 7d200e7b871d..0eeeef1c3142 100644 --- a/turbopack/crates/turbo-rcstr/src/tagged_value.rs +++ b/turbopack/crates/turbo-rcstr/src/tagged_value.rs @@ -37,6 +37,13 @@ //! Reading the value back as an integer (`get_ptr`, `get_value`, `tag_byte`) only ever happens at //! run time, where pointer → integer is perfectly legal. +#[cfg(not(any( + target_pointer_width = "32", + target_pointer_width = "16", + feature = "atom_size_64", + feature = "atom_size_128" +)))] +use std::num::NonZeroUsize; use std::{num::NonZeroU8, os::raw::c_void, ptr::NonNull, slice}; use self::raw_types::*; @@ -209,11 +216,35 @@ impl TaggedValue { #[inline(always)] pub const fn new_tag(value: NonZeroU8) -> Self { - // An integer → pointer transmute, which const evaluation permits. - let value = value.get() as RawTaggedValue; - Self { - #[allow(clippy::transmute_int_to_non_zero)] - value: unsafe { std::mem::transmute::(value) }, + #[cfg(not(any( + target_pointer_width = "32", + target_pointer_width = "16", + feature = "atom_size_64", + feature = "atom_size_128" + )))] + { + // A provenance-free integer → pointer conversion for the pointer representation. + Self { + value: NonNull::without_provenance( + NonZeroUsize::new(value.get() as usize).unwrap(), + ), + } + } + #[cfg(any( + target_pointer_width = "32", + target_pointer_width = "16", + feature = "atom_size_64", + feature = "atom_size_128" + ))] + { + // An integer → pointer transmute, which const evaluation permits. + let value = value.get() as RawTaggedValue; + Self { + #[allow(clippy::transmute_int_to_non_zero)] + value: unsafe { + std::mem::transmute::(value) + }, + } } } @@ -240,12 +271,26 @@ impl TaggedValue { feature = "atom_size_128" )))] { - (self.value.as_ptr() as usize & !(TAG_MASK as usize)) as _ + self.value + .as_ptr() + .map_addr(|addr| addr & !(TAG_MASK as usize)) + .cast_const() + .cast() } } #[inline(always)] fn get_value(&self) -> RawTaggedValue { + #[cfg(not(any( + target_pointer_width = "32", + target_pointer_width = "16", + feature = "atom_size_64", + feature = "atom_size_128" + )))] + { + // Read the pointer representation's address without exposing provenance. + self.value.addr().get() + } #[cfg(all( any(target_pointer_width = "32", target_pointer_width = "16"), not(feature = "atom_size_128") @@ -255,10 +300,18 @@ impl TaggedValue { // value as an integer is well defined. Again, run time only. unsafe { std::mem::transmute::(self.value) } } - #[cfg(not(all( - any(target_pointer_width = "32", target_pointer_width = "16"), - not(feature = "atom_size_128") - )))] + #[cfg(all( + any( + target_pointer_width = "32", + target_pointer_width = "16", + feature = "atom_size_64", + feature = "atom_size_128" + ), + not(all( + any(target_pointer_width = "32", target_pointer_width = "16"), + not(feature = "atom_size_128") + )) + ))] { unsafe { std::mem::transmute::, RawTaggedValue>(Some( diff --git a/turbopack/crates/turbo-tasks-backend/Cargo.toml b/turbopack/crates/turbo-tasks-backend/Cargo.toml index b4ad77873dd1..6743fb49e54e 100644 --- a/turbopack/crates/turbo-tasks-backend/Cargo.toml +++ b/turbopack/crates/turbo-tasks-backend/Cargo.toml @@ -59,13 +59,18 @@ smallvec = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } turbo-bincode = { workspace = true } -turbo-persistence = { workspace = true } turbo-rcstr = { workspace = true } turbo-tasks = { workspace = true } turbo-tasks-hash = { workspace = true } turbo-tasks-malloc = { workspace = true, default-features = false } thread_local = { workspace = true } +[target.'cfg(not(target_family = "wasm"))'.dependencies] +turbo-persistence = { workspace = true } + +[target.'cfg(target_family = "wasm")'.dependencies] +turbo-persistence = { workspace = true, default-features = false } + [dev-dependencies] async-trait = { workspace = true } criterion = { workspace = true, features = ["async_tokio"] } diff --git a/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs b/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs index 134204a0f9bc..a979cfceea4e 100644 --- a/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs +++ b/turbopack/crates/turbo-tasks-backend/src/kv_backing_storage.rs @@ -620,6 +620,8 @@ mod tests { /// Tests that multiple distinct keys written in a single batch with flush can be read back. /// This mirrors the actual save_snapshot pattern: write many TaskCache entries, flush, commit. + // This test is too slow to run under Miri. + #[cfg(not(miri))] #[tokio::test(flavor = "multi_thread")] async fn test_batch_write_with_flush_and_reopen() -> Result<()> { let tempdir = tempfile::tempdir()?; diff --git a/turbopack/crates/turbo-tasks-backend/src/utils/dash_map_multi.rs b/turbopack/crates/turbo-tasks-backend/src/utils/dash_map_multi.rs index d1d5cf43216d..fe58815b5094 100644 --- a/turbopack/crates/turbo-tasks-backend/src/utils/dash_map_multi.rs +++ b/turbopack/crates/turbo-tasks-backend/src/utils/dash_map_multi.rs @@ -228,6 +228,8 @@ mod tests { use super::*; + // This test is too slow to run under Miri. + #[cfg(not(miri))] #[test] fn stress_deadlock() { const N: usize = 100000; diff --git a/turbopack/crates/turbo-tasks-backend/tests/inline_read_execution.rs b/turbopack/crates/turbo-tasks-backend/tests/inline_read_execution.rs index c35fccfb1db2..1445cdfc9e1f 100644 --- a/turbopack/crates/turbo-tasks-backend/tests/inline_read_execution.rs +++ b/turbopack/crates/turbo-tasks-backend/tests/inline_read_execution.rs @@ -358,6 +358,8 @@ async fn counted_leaf(nonce: u32) -> Result> { /// Inline execution nests: reading the deepest task of a chain of uncomputed tasks executes them /// one inside the other. The nesting cap keeps that from growing the stack without bounds. +// This test is too slow to run under Miri. +#[cfg(not(miri))] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn test_deep_dependency_chain() { let mut nonce = 0; diff --git a/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs b/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs index f9f3239fdf2e..bdc042c89576 100644 --- a/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs +++ b/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs @@ -1134,6 +1134,9 @@ mod tests { /// `recursive_mode` is set explicitly rather than left to the platform default so that both /// watching strategies are covered on every host. `TURBO_TASKS_FORCE_WATCH_MODE` still /// overrides it, collapsing these into two cases. + // Miri cannot run the native cases because inotify is unsupported, while the polling cases + // require Turbo Tasks' link-section registry, which is unavailable under Miri. + #[cfg(not(miri))] #[rstest] #[case::native_recursive(None, DiskWatcherRecursiveMode::Recursive)] #[case::native_non_recursive(None, DiskWatcherRecursiveMode::NonRecursive)] diff --git a/turbopack/crates/turbo-tasks-macros-tests/tests/trybuild.rs b/turbopack/crates/turbo-tasks-macros-tests/tests/trybuild.rs index 314299db6f6a..730a02c21923 100644 --- a/turbopack/crates/turbo-tasks-macros-tests/tests/trybuild.rs +++ b/turbopack/crates/turbo-tasks-macros-tests/tests/trybuild.rs @@ -1,3 +1,6 @@ +// Miri cannot run trybuild because its subprocesses require the unsupported `posix_spawnattr_init`. +#![cfg(not(miri))] + // Unset RUSTC_WRAPPER before trybuild tests run. When sccache wraps rustc, it // emits "warning: ignoring -C extra-filename flag due to -o flag" which pollutes // trybuild's stderr snapshot comparisons. Unsetting it here means only the diff --git a/turbopack/crates/turbo-tasks-malloc/Cargo.toml b/turbopack/crates/turbo-tasks-malloc/Cargo.toml index 2fff14ae6dd2..5c5b208854c0 100644 --- a/turbopack/crates/turbo-tasks-malloc/Cargo.toml +++ b/turbopack/crates/turbo-tasks-malloc/Cargo.toml @@ -18,18 +18,18 @@ harness = false [dev-dependencies] criterion = { workspace = true } -[target.'cfg(not(target_family = "wasm"))'.dependencies] +[target.'cfg(all(not(target_family = "wasm"), not(miri)))'.dependencies] libmimalloc-sys = { version = "0.1.44", features = [ "extended", ], optional = true } -[target.'cfg(not(any(target_os = "linux", target_family = "wasm")))'.dependencies] +[target.'cfg(not(any(target_os = "linux", target_family = "wasm", miri)))'.dependencies] mimalloc = { version = "0.1.48", features = [ "v3", "extended", ], optional = true } -[target.'cfg(all(target_os = "linux", not(target_family = "wasm")))'.dependencies] +[target.'cfg(all(target_os = "linux", not(target_family = "wasm"), not(miri)))'.dependencies] mimalloc = { version = "0.1.48", features = [ "v3", "extended", diff --git a/turbopack/crates/turbo-tasks-malloc/src/counter.rs b/turbopack/crates/turbo-tasks-malloc/src/counter.rs index 4a94073ae86a..a864af80958b 100644 --- a/turbopack/crates/turbo-tasks-malloc/src/counter.rs +++ b/turbopack/crates/turbo-tasks-malloc/src/counter.rs @@ -11,7 +11,7 @@ use std::{cell::UnsafeCell, ptr::NonNull}; -#[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] +#[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] pub use self::global::get; use crate::AllocationCounters; @@ -20,7 +20,7 @@ use crate::AllocationCounters; /// Only compiled without the `custom_allocator` feature; see the module docs. Each thread holds /// its buffer in its own [`ThreadLocalCounter`] and passes it in, so the counter's state lives in /// exactly one place. -#[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] +#[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] mod global { use std::sync::atomic::{AtomicUsize, Ordering}; @@ -105,7 +105,7 @@ struct ThreadLocalCounter { /// global counter desprite not being allocated yet. It is unsigned so that /// means the global counter is always equal or greater than the real /// value. - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] buffer: usize, allocation_counters: AllocationCounters, } @@ -113,7 +113,11 @@ struct ThreadLocalCounter { impl ThreadLocalCounter { const fn new() -> Self { Self { - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all( + feature = "custom_allocator", + not(target_family = "wasm"), + not(miri) + )))] buffer: 0, allocation_counters: AllocationCounters::new(), } @@ -124,7 +128,7 @@ impl ThreadLocalCounter { self.allocation_counters.allocations += size; self.allocation_counters.allocation_count += 1; - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] self.buffered_add(size); } @@ -133,7 +137,7 @@ impl ThreadLocalCounter { self.allocation_counters.deallocations += size; self.allocation_counters.deallocation_count += 1; - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] self.buffered_remove(size); } @@ -144,7 +148,7 @@ impl ThreadLocalCounter { self.allocation_counters.allocations += new_size; self.allocation_counters.allocation_count += 1; - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] { match old_size.cmp(&new_size) { std::cmp::Ordering::Equal => {} @@ -155,7 +159,7 @@ impl ThreadLocalCounter { } fn unload(&mut self) { - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] global::flush_all(&mut self.buffer); self.allocation_counters = AllocationCounters::default(); } @@ -279,7 +283,7 @@ mod tests { /// read — it is process-wide, and this binary installs [`crate::TurboMalloc`] as its global /// allocator, so every other thread moves it concurrently. `buffer` is thread-local and /// exact. - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] #[test] fn counting() { use super::global::{MAX_BUFFER, TARGET_BUFFER}; diff --git a/turbopack/crates/turbo-tasks-malloc/src/lib.rs b/turbopack/crates/turbo-tasks-malloc/src/lib.rs index 70e16c7cd577..ad5bcb96db12 100644 --- a/turbopack/crates/turbo-tasks-malloc/src/lib.rs +++ b/turbopack/crates/turbo-tasks-malloc/src/lib.rs @@ -96,7 +96,7 @@ impl TurboMalloc { /// Without the `custom_allocator` feature this is a process-wide live-bytes counter instead, /// which is approximate because threads buffer their updates. pub fn memory_usage() -> usize { - #[cfg(all(feature = "custom_allocator", not(target_family = "wasm")))] + #[cfg(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri)))] { // `current_commit` is a relaxed atomic load, but `mi_process_info` also calls // `_mi_prim_process_info`, which is a `getrusage` (plus a `task_info` on macOS). All @@ -117,7 +117,7 @@ impl TurboMalloc { } current_commit } - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] { self::counter::get() } @@ -139,11 +139,11 @@ impl TurboMalloc { /// force=true: do all the work of `process=false` and then process global shared structures and /// return memory to the OS if possible, this is much slower and should only be done rarely. pub fn collect(force: bool) { - #[cfg(all(feature = "custom_allocator", not(target_family = "wasm")))] + #[cfg(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri)))] unsafe { libmimalloc_sys::mi_collect(force); } - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] { let _ = force; } @@ -178,17 +178,17 @@ impl TurboMalloc { /// Get the allocator for this platform that we should wrap with TurboMalloc. #[inline] fn base_alloc() -> &'static impl GlobalAlloc { - #[cfg(all(feature = "custom_allocator", not(target_family = "wasm")))] + #[cfg(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri)))] return &mimalloc::MiMalloc; - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] return &std::alloc::System; } #[allow(unused_variables)] unsafe fn base_alloc_size(ptr: *const u8, layout: Layout) -> usize { - #[cfg(all(feature = "custom_allocator", not(target_family = "wasm")))] + #[cfg(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri)))] return unsafe { mimalloc::MiMalloc.usable_size(ptr) }; - #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"))))] + #[cfg(not(all(feature = "custom_allocator", not(target_family = "wasm"), not(miri))))] return layout.size(); } @@ -271,7 +271,7 @@ mod tests { // On all supported platforms the value must be reported. #[cfg(any( - all(target_os = "linux", not(target_family = "wasm")), + all(target_os = "linux", not(target_family = "wasm"), not(miri)), target_os = "macos", windows, ))] @@ -279,7 +279,7 @@ mod tests { // On unsupported platforms we expect None and have nothing further to assert. #[cfg(not(any( - all(target_os = "linux", not(target_family = "wasm")), + all(target_os = "linux", not(target_family = "wasm"), not(miri)), target_os = "macos", windows, )))] diff --git a/turbopack/crates/turbo-tasks-malloc/src/memory_pressure.rs b/turbopack/crates/turbo-tasks-malloc/src/memory_pressure.rs index 8ace0be8bf11..3eeb5a8d1e0d 100644 --- a/turbopack/crates/turbo-tasks-malloc/src/memory_pressure.rs +++ b/turbopack/crates/turbo-tasks-malloc/src/memory_pressure.rs @@ -18,7 +18,7 @@ fn clamp_percent(value: f64) -> u8 { value.round().clamp(0.0, 100.0) as u8 } -#[cfg(all(target_os = "linux", not(target_family = "wasm")))] +#[cfg(all(target_os = "linux", not(target_family = "wasm"), not(miri)))] mod platform { use super::clamp_percent; @@ -184,7 +184,7 @@ mod platform { } #[cfg(not(any( - all(target_os = "linux", not(target_family = "wasm")), + all(target_os = "linux", not(target_family = "wasm"), not(miri)), target_os = "macos", windows, )))] diff --git a/turbopack/crates/turbo-tasks/src/priority_runner.rs b/turbopack/crates/turbo-tasks/src/priority_runner.rs index 16c6ba03f1c7..d8d0e0994fa1 100644 --- a/turbopack/crates/turbo-tasks/src/priority_runner.rs +++ b/turbopack/crates/turbo-tasks/src/priority_runner.rs @@ -697,6 +697,7 @@ mod tests { assert_eq!(runner.total_queued(), 3); } + #[cfg(not(miri))] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_cpu_bound_tasks() { struct ExecutorImpl; diff --git a/turbopack/crates/turbo-tasks/src/scope_unbounded.rs b/turbopack/crates/turbo-tasks/src/scope_unbounded.rs index 44f19864559c..8d2a44834c89 100644 --- a/turbopack/crates/turbo-tasks/src/scope_unbounded.rs +++ b/turbopack/crates/turbo-tasks/src/scope_unbounded.rs @@ -916,6 +916,8 @@ mod tests { } /// Sustained work keeps workers alive rather than churning them: + // This test is too slow to run under Miri. + #[cfg(not(miri))] #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn test_unbounded_busy_queue_does_not_churn_workers() { const ITEMS: usize = 20_000; diff --git a/turbopack/crates/turbopack-css/src/process.rs b/turbopack/crates/turbopack-css/src/process.rs index 96deade3c2ff..6c9053c793d7 100644 --- a/turbopack/crates/turbopack-css/src/process.rs +++ b/turbopack/crates/turbopack-css/src/process.rs @@ -880,6 +880,8 @@ mod tests { assert_ne!(lint_lightningcss(code), vec![], "lightningcss: {code}"); } + // Lightning CSS currently triggers a Miri Stacked Borrows violation in its string parser. + #[cfg(not(miri))] #[test] fn css_module_pure_lint() { assert_lint_success( @@ -1008,6 +1010,8 @@ mod tests { ); } + // Lightning CSS currently triggers a Miri Stacked Borrows violation in its string parser. + #[cfg(not(miri))] #[test] fn strip_bom_lets_lightningcss_parse() { let with_bom = "\u{feff}@layer a {}"; diff --git a/turbopack/crates/turbopack-ecmascript/src/analyzer/jsvalue/predicates.rs b/turbopack/crates/turbopack-ecmascript/src/analyzer/jsvalue/predicates.rs index cf5e82bf7c4e..9478fce6ebde 100644 --- a/turbopack/crates/turbopack-ecmascript/src/analyzer/jsvalue/predicates.rs +++ b/turbopack/crates/turbopack-ecmascript/src/analyzer/jsvalue/predicates.rs @@ -481,108 +481,172 @@ mod tests { use crate::analyzer::{Bump, ConstantValue, JsValue, ThreadLocal, graph::EvalContext}; - // A leaked arena for building test `JsValue`s with a `'static` lifetime. Tests are - // short-lived processes, so the leak is inconsequential. - fn test_arena() -> &'static Bump { - Box::leak(Box::new(Bump::new())) + #[derive(Clone, Copy)] + enum TestAtom { + Num(f64), + Str(&'static str), + True, + False, + Null, + Undefined, + } + + impl TestAtom { + fn build<'a>(self, _arena: &'a Bump) -> JsValue<'a> { + match self { + TestAtom::Num(value) => JsValue::from(value), + TestAtom::Str(value) => JsValue::from(value), + TestAtom::True => ConstantValue::True.into(), + TestAtom::False => ConstantValue::False.into(), + TestAtom::Null => ConstantValue::Null.into(), + TestAtom::Undefined => ConstantValue::Undefined.into(), + } + } } - // `construct_test_ternary(cons, alt)` builds a ternary with an unknown test condition. - fn construct_test_ternary(cons: JsValue<'static>, alt: JsValue<'static>) -> JsValue<'static> { - JsValue::tenary( - test_arena(), - JsValue::unknown_empty(false, rcstr!("test")), - cons, - alt, - ) + #[derive(Clone, Copy)] + enum TestValue { + Atom(TestAtom), + PromiseNull, + Ternary(TestAtom, TestAtom), + } + + impl TestValue { + fn build(self, arena: &Bump) -> JsValue<'_> { + match self { + TestValue::Atom(value) => value.build(arena), + TestValue::PromiseNull => JsValue::promise(arena, ConstantValue::Null.into()), + TestValue::Ternary(consequent, alternate) => JsValue::tenary( + arena, + JsValue::unknown_empty(false, rcstr!("test")), + consequent.build(arena), + alternate.build(arena), + ), + } + } } + use TestAtom::*; + use TestValue::*; + #[rstest] - #[case(JsValue::from(1.0))] - #[case(JsValue::from("hi"))] - #[case(ConstantValue::True.into())] - #[case(JsValue::promise(test_arena(), ConstantValue::Null.into()))] - #[case(construct_test_ternary(JsValue::from(1.0), JsValue::from("hi")))] - fn is_truthy_positive(#[case] v: JsValue<'static>) { - assert_eq!(v.is_truthy(), Some(true), "expected '{v}' to be truthy"); + #[case(Atom(Num(1.0)))] + #[case(Atom(Str("hi")))] + #[case(Atom(True))] + #[case(PromiseNull)] + #[case(Ternary(Num(1.0), Str("hi")))] + fn is_truthy_positive(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); + assert_eq!( + value.is_truthy(), + Some(true), + "expected '{value}' to be truthy" + ); } #[rstest] - #[case(JsValue::from(0.0))] - #[case(JsValue::from(""))] - #[case(ConstantValue::False.into())] - #[case(ConstantValue::Null.into())] - #[case(ConstantValue::Undefined.into())] - #[case(construct_test_ternary(JsValue::from(0.0), JsValue::from("")))] - fn is_truthy_negative(#[case] v: JsValue<'static>) { - assert_eq!(v.is_truthy(), Some(false), "expected '{v}' to be falsy"); + #[case(Atom(Num(0.0)))] + #[case(Atom(Str("")))] + #[case(Atom(False))] + #[case(Atom(Null))] + #[case(Atom(Undefined))] + #[case(Ternary(Num(0.0), Str("")))] + fn is_truthy_negative(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); + assert_eq!( + value.is_truthy(), + Some(false), + "expected '{value}' to be falsy" + ); } #[rstest] - #[case(ConstantValue::Null.into())] - #[case(ConstantValue::Undefined.into())] - #[case(construct_test_ternary(ConstantValue::Null.into(), ConstantValue::Undefined.into()))] - fn is_nullish_positive(#[case] v: JsValue<'static>) { - assert_eq!(v.is_nullish(), Some(true), "expected '{v}' to be nullish"); + #[case(Atom(Null))] + #[case(Atom(Undefined))] + #[case(Ternary(Null, Undefined))] + fn is_nullish_positive(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); + assert_eq!( + value.is_nullish(), + Some(true), + "expected '{value}' to be nullish" + ); } #[rstest] - #[case(JsValue::from(0.0))] - #[case(JsValue::from(""))] - #[case(JsValue::from("hi"))] - #[case(ConstantValue::True.into())] - #[case(JsValue::promise(test_arena(), ConstantValue::Null.into()))] - #[case(construct_test_ternary(JsValue::from(0.0), JsValue::from("hi")))] - fn is_nullish_negative(#[case] v: JsValue<'static>) { + #[case(Atom(Num(0.0)))] + #[case(Atom(Str("")))] + #[case(Atom(Str("hi")))] + #[case(Atom(True))] + #[case(PromiseNull)] + #[case(Ternary(Num(0.0), Str("hi")))] + fn is_nullish_negative(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); assert_eq!( - v.is_nullish(), + value.is_nullish(), Some(false), - "expected '{v}' not to be nullish" + "expected '{value}' not to be nullish" ); } #[rstest] - #[case(JsValue::from("hi"))] - #[case(JsValue::from(""))] - #[case(construct_test_ternary(JsValue::from("a"), JsValue::from("b")))] - fn is_string_positive(#[case] v: JsValue<'static>) { - assert_eq!(v.is_string(), Some(true), "expected '{v}' to be a string"); + #[case(Atom(Str("hi")))] + #[case(Atom(Str("")))] + #[case(Ternary(Str("a"), Str("b")))] + fn is_string_positive(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); + assert_eq!( + value.is_string(), + Some(true), + "expected '{value}' to be a string" + ); } #[rstest] - #[case(JsValue::from(1.0))] - #[case(ConstantValue::True.into())] - #[case(ConstantValue::Null.into())] - #[case(construct_test_ternary(JsValue::from(1.0), JsValue::from(2.0)))] - fn is_string_negative(#[case] v: JsValue<'static>) { + #[case(Atom(Num(1.0)))] + #[case(Atom(True))] + #[case(Atom(Null))] + #[case(Ternary(Num(1.0), Num(2.0)))] + fn is_string_negative(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); assert_eq!( - v.is_string(), + value.is_string(), Some(false), - "expected '{v}' not to be a string" + "expected '{value}' not to be a string" ); } #[rstest] - #[case(JsValue::from(""))] - #[case(construct_test_ternary(JsValue::from(""), JsValue::from("")))] - fn is_empty_string_positive(#[case] v: JsValue<'static>) { + #[case(Atom(Str("")))] + #[case(Ternary(Str(""), Str("")))] + fn is_empty_string_positive(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); assert_eq!( - v.is_empty_string(), + value.is_empty_string(), Some(true), - "expected '{v}' to be an empty string" + "expected '{value}' to be an empty string" ); } #[rstest] - #[case(JsValue::from("hi"))] - #[case(JsValue::from(1.0))] - #[case(ConstantValue::True.into())] - #[case(construct_test_ternary(JsValue::from("a"), JsValue::from("b")))] - fn is_empty_string_negative(#[case] v: JsValue<'static>) { + #[case(Atom(Str("hi")))] + #[case(Atom(Num(1.0)))] + #[case(Atom(True))] + #[case(Ternary(Str("a"), Str("b")))] + fn is_empty_string_negative(#[case] value: TestValue) { + let arena = Bump::new(); + let value = value.build(&arena); assert_eq!( - v.is_empty_string(), + value.is_empty_string(), Some(false), - "expected '{v}' not to be an empty string" + "expected '{value}' not to be an empty string" ); }