Skip to content

feat(cache): warn when recompute is cheaper than cache retrieval#4401

Open
raminjafary wants to merge 1 commit into
nitrojs:mainfrom
raminjafary:feat/cache-efficiency-warning
Open

feat(cache): warn when recompute is cheaper than cache retrieval#4401
raminjafary wants to merge 1 commit into
nitrojs:mainfrom
raminjafary:feat/cache-efficiency-warning

Conversation

@raminjafary

Copy link
Copy Markdown

🔗 Linked issue

Closes #2559

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

cachedEventHandler / cachedFunction can be slower than the code they cache when the value is cheap to compute — the store/retrieve round-trip (and JSON (de)serialization) costs more than just recomputing it (see #2558).

This adds a dev-only warning to surface that. At the storage layer we measure, per cache:

  • retrieval time — the duration of a hit getItem, and
  • recompute time — the interval from a miss to its following setItem.

When median recompute is cheaper than median retrieval, we log a single warning suggesting the cache may be adding overhead.

Notes:

  • Gated behind import.meta.dev, so the whole path is tree-shaken out of production builds.
  • The user function is not wrapped, so ocache's integrity hash (used to invalidate on code edits) is unchanged and production behavior is identical.
  • Warns at most once per cache; per-key tracking is bounded (256 keys) to cap dev memory.

Tested with unit tests covering the warn / no-warn / threshold / warn-once cases, plus a regression guard that the function reaches ocache unwrapped. Full suite passes on both the rollup and rolldown builders.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@raminjafary raminjafary requested a review from pi0 as a code owner July 2, 2026 11:08
@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fa366c45-4bcd-4f18-afc7-d84339409f65

📥 Commits

Reviewing files that changed from the base of the PR and between 3983d1b and 5d35a94.

📒 Files selected for processing (3)
  • src/runtime/internal/cache.ts
  • test/unit/cache.test.ts
  • vitest.config.ts
👮 Files not reviewed due to content moderation or server errors (3)
  • src/runtime/internal/cache.ts
  • vitest.config.ts
  • test/unit/cache.test.ts

📝 Walkthrough

[!WARNING]

Walkthrough skipped

File diffs could not be summarized.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pi0

pi0 commented Jul 2, 2026

Copy link
Copy Markdown
Member

Do you think we can port this to https://github.com/unjs/ocache ?

@raminjafary

Copy link
Copy Markdown
Author

Yeah, I will port it to ocache. By doing so, I think the measurement would be more accurate as well.

Enable ocache's `warnWhenSlower` option globally in dev (via import.meta.dev)
for cached functions and handlers, so ocache measures recompute vs. retrieval
directly. Overridable per cache; stripped from production builds.

Requires the ocache `warnWhenSlower` option — bump the ocache dependency once
that lands before merging.
@raminjafary raminjafary force-pushed the feat/cache-efficiency-warning branch from 3983d1b to 5d35a94 Compare July 2, 2026 12:24
@raminjafary

raminjafary commented Jul 2, 2026

Copy link
Copy Markdown
Author

Moved the detection logic into ocache as an opt-in warnWhenSlower option, so ocache measures recompute vs. retrieval directly instead of Nitro instrumenting the storage adapter.

Nitro just enables it globally in dev via import.meta.dev (stripped in prod, overridable per cache).

Blocked on the ocache side: ocache PR must be merged and its version bumped in nitro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warn when cache computation is cheaper than cache retrival

2 participants