Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #609 +/- ##
==========================================
- Coverage 78.66% 78.62% -0.04%
==========================================
Files 128 128
Lines 12456 12461 +5
Branches 883 902 +19
==========================================
Hits 9798 9798
- Misses 2653 2658 +5
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
| File | Base | Head | Diff |
|---|---|---|---|
orama-db.json |
8.03 MB | 8.03 MB | -1.00 B (-0.00%) |
|
cc @nodejs/web-infra |
There was a problem hiding this comment.
Pull request overview
This pull request introduces lazy loading for generators to improve module load time performance and reduce strain on Node.js module resolution. Instead of eagerly importing all generators at module load time, generators are now loaded on-demand using dynamic imports.
Changes:
- Converted all generator imports to lazy loaders using dynamic import() wrapped in a
lazyDefaulthelper - Updated type definitions to handle Promise-wrapped generator loaders with new
LazyGeneratorandResolvedGeneratortypes - Modified all generator consumers to properly await lazy-loaded generators
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/generators/index.mjs | Replaced direct imports with lazy loaders using dynamic import() for all 16 generators |
| src/generators/types.d.ts | Added LazyGenerator and ResolvedGenerator type helpers to properly type lazy-loaded generators |
| src/utils/configuration/types.d.ts | Updated Configuration type to use ResolvedGenerator for accessing defaultConfiguration |
| src/utils/configuration/index.mjs | Made getDefaultConfig async and updated to await generator loading when accessing defaultConfiguration |
| src/utils/configuration/tests/index.test.mjs | Updated mock generators to return async functions instead of direct objects |
| src/threading/parallel.mjs | Made createParallelWorker async to await generator loading |
| src/threading/chunk-worker.mjs | Updated to await generator loading before accessing processChunk |
| src/threading/tests/parallel.test.mjs | Added await to all createParallelWorker calls in tests |
| src/generators/tests/index.test.mjs | Updated tests to properly resolve lazy generators before assertions |
| src/generators.mjs | Updated scheduleGenerator to await generator loading; properly handles async worker creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6ef07c8 to
251f2cc
Compare
Co-authored-by: Aviv Keller <me@aviv.sh>
Do you have stat on that ? |
Benchmark ResultsCommand:
|
| Metric | main |
feat/lazy-generators |
Δ |
|---|---|---|---|
| Mean | 18.787s ± 0.772s | 18.848s ± 0.619s | +0.3% |
| Median | 18.507s | 18.595s | +0.5% |
| Min | 18.202s | 18.499s | +1.6% |
| Max | 20.133s | 19.945s | -0.9% |
| User time | 30.057s | 29.954s | -0.3% |
| System time | 1.028s | 1.031s | +0.3% |
| Peak memory | 4,806 MB | 4,758 MB | -1.0% |
--threads 4
| Metric | main |
feat/lazy-generators |
Δ |
|---|---|---|---|
| Mean | 14.232s ± 0.109s | 13.902s ± 0.181s | -2.3% |
| Median | 14.197s | 13.935s | -1.8% |
| Min | 14.105s | 13.653s | -3.2% |
| Max | 14.385s | 14.134s | -1.7% |
| User time | 40.818s | 39.344s | -3.6% |
| System time | 1.565s | 1.597s | +2.0% |
| Peak memory | 6,443 MB | 6,540 MB | +1.5% |
Default threads (system CPUs)
| Metric | main |
feat/lazy-generators |
Δ |
|---|---|---|---|
| Mean | 14.064s ± 0.339s | 13.812s ± 0.365s | -1.8% |
| Median | 14.018s | 13.714s | -2.2% |
| Min | 13.737s | 13.567s | -1.2% |
| Max | 14.584s | 14.447s | -0.9% |
| User time | 51.736s | 48.278s | -6.7% |
| System time | 2.376s | 1.890s | -20.5% |
| Peak memory | 8,665 MB | 7,801 MB | -10.0% |
Summary
- Single-threaded: Performance is effectively identical — no regression.
- 4 threads:
feat/lazy-generatorsis ~2.3% faster wall-clock, with 3.6% less CPU time. - Default threads (all CPUs):
feat/lazy-generatorsis ~1.8% faster wall-clock, with 6.7% less CPU user time and ~10% lower peak memory.
The lazy generators approach shows consistent modest improvements in multi-threaded scenarios, with the biggest wins in reduced CPU time and memory usage at higher thread counts.
|
(Interestingly enough my initial tests yesterday were wildly different with this PR branch being almost 6-7 seconds faster, but hey, a win is a win. |
This PR increases performance and reduces complexity at module load time as we only load the generators and its dependencies as we need them, reducing strain on workers and Node.js module resolution.