feat(perf): optimize materialization and sync caching#14
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughNode.js versions updated across GitHub Actions workflows (audit, CI, and package preview), benchmarks directory added to gitignore, and source code refactored to optimize file handling through entries array precomputation, docsPresence caching, and stat-based verification. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
commit: |
There was a problem hiding this comment.
Pull request overview
This pull request implements performance optimizations in the caching and materialization logic. The changes focus on reducing redundant filesystem operations and path computations.
Changes:
- Replaced duplicate filesystem existence checks with try-catch pattern in verification
- Added caching for docs presence checks to avoid repeated lookups during sync operations
- Optimized path normalization by computing once before sorting instead of multiple times
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/verify.ts | Replaced exists() + stat() calls with single stat() call in try-catch for better performance |
| src/sync.ts | Added Map-based cache for docs presence checks to avoid redundant filesystem lookups |
| src/materialize.ts | Pre-computed normalized paths once before sorting instead of computing them multiple times during sort and processing |
| .gitignore | Added benchmarks/ directory to ignore benchmark data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/verify.ts`:
- Around line 49-61: The inner catch currently rethrows non-ENOENT/ENOTDIR
errors but the outer catch converts any thrown error into a "missing manifest"
result, masking real IO/permission failures; update the outer catch to inspect
the thrown error (cast to NodeJS.ErrnoException and read .code) and only
increment missingCount/continue when code === "ENOENT" or code === "ENOTDIR",
otherwise rethrow the error so permission/IO errors from stat/filePath are
propagated; reference the stat(filePath) call, the inner error handling (error
as NodeJS.ErrnoException).code, and the missingCount variable when making this
change.
Summary
Cache docs presence checks and optimize path normalization.
Changes
Summary by CodeRabbit
Chores
Refactor
Performance