feat(core): add startup manifest for faster cold starts#5842
feat(core): add startup manifest for faster cold starts#5842
Conversation
Add a ManifestStore system that caches file I/O metadata (resolveModule paths, globby file discovery, tegg module descriptors) to .egg/manifest.json. On subsequent startups, the manifest is loaded and validated (via stat-based fingerprinting of lockfile and config directory), skipping expensive filesystem scanning operations. Key changes: - ManifestStore class with load/validate/generate/write/clean APIs - EggLoader: manifest-aware resolveModule() and FileLoader.parse() - tegg: ModuleLoader accepts precomputed decorated files from manifest - tegg config plugin: skips deep globby scan when manifest available - egg-bin: new `manifest generate|validate|clean` CLI command - Auto-generates manifest on first startup (dumpManifest in ready hook) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Deploying egg with
|
| Latest commit: |
3ddfe9d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://abb136ac.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-startup-manifest.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
3ddfe9d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ad5f5ad4.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-startup-manifest.egg-v3.pages.dev |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #5842 +/- ##
==========================================
- Coverage 85.64% 85.63% -0.01%
==========================================
Files 665 666 +1
Lines 13004 13144 +140
Branches 1495 1515 +20
==========================================
+ Hits 11137 11256 +119
- Misses 1743 1763 +20
- Partials 124 125 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces a startup manifest system designed to accelerate application cold starts by caching expensive file discovery and module resolution results. Key changes include the implementation of a ManifestStore for managing manifest lifecycle, integration of caching logic into EggLoader and FileLoader, and the addition of a manifest command in egg-bin for manual management. Review feedback focuses on refining error handling within the manifest loading and fingerprinting utilities to ensure that system errors (like permission issues) are not silently swallowed and treated as missing files.
| } catch { | ||
| debug('manifest not found at %s', manifestPath); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The catch block is too broad and will treat any file read error as 'file not found'. This can hide underlying issues like file permission errors. It's better to specifically handle the ENOENT (file not found) case and log other errors for easier debugging.
} catch (err: any) {
if (err.code === 'ENOENT') {
debug('manifest not found at %s', manifestPath);
} else {
debug('failed to read manifest at %s: %o', manifestPath, err);
}
return null;
}| try { | ||
| fs.unlinkSync(manifestPath); | ||
| debug('manifest removed: %s', manifestPath); | ||
| } catch { | ||
| // file doesn't exist, nothing to do | ||
| } |
There was a problem hiding this comment.
The catch block is too broad and will swallow any error from fs.unlinkSync, not just when the file doesn't exist. This can hide permission issues or other problems, and the comment '// file doesn't exist, nothing to do' is misleading. It's better to only ignore ENOENT errors and log others for debugging purposes.
try {
fs.unlinkSync(manifestPath);
debug('manifest removed: %s', manifestPath);
} catch (err: any) {
if (err.code !== 'ENOENT') {
debug('Error removing manifest file at %s: %o', manifestPath, err);
}
}| try { | ||
| const stat = fs.statSync(filepath); | ||
| return `${stat.mtimeMs}:${stat.size}`; | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This try-catch block swallows all errors from fs.statSync. While returning null for a non-existent file is reasonable, this will also return null for other errors like permission denied. This might be acceptable as the caller treats null as 'missing', but logging the error for non-ENOENT cases would improve diagnostics.
try {
const stat = fs.statSync(filepath);
return stat.mtimeMs + ':' + stat.size;
} catch (err: any) {
if (err.code !== 'ENOENT') {
debug('Failed to stat file %s: %o', filepath, err);
}
return null;
}| try { | ||
| realPath = fs.realpathSync(dirpath); | ||
| } catch { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This try-catch block silently ignores errors from fs.realpathSync. This can lead to an incomplete fingerprint if there are, for example, permission errors. An incomplete fingerprint might not correctly detect changes, causing a stale manifest to be considered valid. It would be better to log these errors for debugging purposes. A similar issue exists for the fs.readdirSync call on lines 232-236.
| try { | |
| realPath = fs.realpathSync(dirpath); | |
| } catch { | |
| return; | |
| } | |
| try { | |
| realPath = fs.realpathSync(dirpath); | |
| } catch (err: any) { | |
| debug('Failed to get realpath for %s: %o', dirpath, err); | |
| return; | |
| } |
- Add `metadataOnly` option to EggCore/EggLoader for manifest generation without triggering external SDK connections - Add `loadMetadata` lifecycle hook to ILifecycleBoot — called instead of configWillLoad/configDidLoad/didLoad/willReady in metadataOnly mode - Short-circuit AppWorkerLoader.load() after loadCustomApp in metadataOnly - Skip agent creation in startEgg() when metadataOnly - Prevent didReady from firing in metadataOnly mode - tegg config plugin: implement loadMetadata for module scanning - tegg plugin: implement loadMetadata using shared buildTeggManifestData() - Extract buildRequireExecArgv() to BaseCommand (shared by dev + manifest) - egg-bin manifest generate: fork manifest-generate.mjs with metadataOnly - Remove baseDir validation from manifest (build env ≠ runtime env) - Add E2E tests for manifest cache usage during app loading Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- AppWorkerLoader: collect router resolveModule in metadataOnly mode instead of skipping loadRouter entirely - Fix agent?.close() for metadataOnly mode (agent not created) - Split manifest.test.ts into 4 focused test files + shared helper - Remove inline E2E tests from packages/core/test/ - Add ecosystem-ci/scripts/verify-manifest.mjs for E2E verification - E2E workflow: generate manifest → boot cnpmcore → verify cache hits Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@eggjs/core is used by the manifest command and scripts/manifest-generate.mjs but tsdown doesn't detect it since the script is copied, not bundled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous test copied files between directories which loses mtime precision (sub-ms floating point drift). Instead, test by tampering the stored baseDir in an existing manifest to prove it's not validated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
WIP: 通过静态化启动时的文件 I/O metadata 来优化 Egg/tegg 框架冷启动性能。
ManifestStore类,将 resolveModule 路径、globby 文件发现结果、tegg 模块描述符缓存到.egg/manifest.jsonModuleLoader接受预计算的 decorated files 列表,跳过 70-80% 的import()调用globby.sync('**/package.json')扫描manifest generate|validate|cleanCLI 命令dumpManifest)主要优化点
TODO
Test plan
@eggjs/core351 tests passedegg352 tests passed🤖 Generated with Claude Code