perf(webapp): resolve schedule list run times per expression, not per row - #4703
Open
ericallam wants to merge 2 commits into
Open
perf(webapp): resolve schedule list run times per expression, not per row#4703ericallam wants to merge 2 commits into
ericallam wants to merge 2 commits into
Conversation
… row Listing schedules walked the cron expression three times for every row: once backwards to approximate last run, and twice forwards to get the next run and the interval after it. Each walk steps the calendar unit by unit, so a full page of timezone-aware schedules could block the event loop for seconds. Run times now resolve for the whole page at once. Nominal times are cached per (cron, timezone) against a single pinned now, so cost scales with the number of distinct expressions rather than the number of rows. The backwards walk is opt-in and only the dashboard, which renders the column, asks for it. Windowless schedules take one step instead of two, since with no window the interval to the following occurrence cannot affect the result.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (20)
🧰 Additional context used📓 Path-based instructions (12)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/**/package.json📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/package.json📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (23)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-06-23T13:04:21.413ZApplied to files:
📚 Learning: 2026-05-01T15:45:08.099ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-06-25T18:21:51.905ZApplied to files:
📚 Learning: 2026-07-03T17:10:21.498ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-06-09T17:58:04.699ZApplied to files:
📚 Learning: 2026-06-16T09:19:52.037ZApplied to files:
📚 Learning: 2026-05-01T15:45:12.777ZApplied to files:
📚 Learning: 2026-04-27T16:46:03.861ZApplied to files:
📚 Learning: 2026-05-07T12:25:18.271ZApplied to files:
📚 Learning: 2026-05-28T20:02:10.647ZApplied to files:
📚 Learning: 2026-07-30T18:43:56.874ZApplied to files:
📚 Learning: 2026-08-08T12:49:17.489ZApplied to files:
📚 Learning: 2026-05-18T14:40:02.173ZApplied to files:
📚 Learning: 2026-06-16T09:19:47.637ZApplied to files:
📚 Learning: 2025-11-27T16:26:37.432ZApplied to files:
🔇 Additional comments (5)
WalkthroughThe change adds a batch schedule timing resolver with cron occurrence caching, phase and window handling, and optional last-run resolution. Schedule listings now use the resolver, and scheduled task listings request last-run data. Cron timestamp generation now reuses parsed intervals. The change adds functional and performance tests, plus separate Vitest configuration and scripts for performance tests. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Wall-clock speedup ratios are single-sample and swing well past the asserted bounds on a shared runner, so the benchmarks now live behind a *.perf.test.ts suffix with their own config and a test:perf script, following the pattern the e2e suites already use. Correctness stays in the ordinary suite. Also splits the malformed-expression test, whose name claimed a lastRun degradation that its assertions did not cover.
ericallam
marked this pull request as ready for review
August 19, 2026 11:51
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Listing schedules could block the event loop for seconds. A page of 100 timezone-aware schedules spent over two seconds on cron arithmetic alone, after the database work was already done, which stalls every other request on that process. The same page now resolves in tens of milliseconds.
Root cause and fix
cron-parserwalks the calendar unit by unit, and under a named timezone every step goes through luxon. Parsing an expression is cheap (single-digit microseconds); stepping it is not, ranging from a couple of hundred microseconds for a common expression to several milliseconds for a sparse one like0 0 29 2 *. The presenter did three independent walks per row, one backwards for "last run" and two forwards (re-parsing each time) for the next run and the occurrence after it. At 100 rows that is 300 calendar walks in one uninterrupted tick.Run times now resolve for the whole page in one pass, in a new
resolveScheduleTimingsthat takes plain values rather than Prisma rows so it can be tested and benchmarked on its own.(cron, timezone)against a singlenowpinned for the batch, so cost scales with the number of distinct expressions instead of the number of rows. Rows in one response also stop disagreeing about the current time.min(intervalMs, max(MINIMUM_SCHEDULE_RANGE_MS, windowMs)). With no windowwindowMsis 0, andCronPatternrejects expressions with a seconds field, so occurrences are always at leastMINIMUM_SCHEDULE_RANGE_MSapart and thatmincan never bind. It is also the costlier step, since it walks a whole period rather than the remainder of the current one.nextScheduledTimestampssteps one parsed expression instead of re-parsing per step, which also helps the single-schedule callers.Behaviour is unchanged, error semantics included: a malformed expression still throws for the next run and still degrades to an undefined last run.
Verification
Measured inside a real request against a live environment, 100 schedules: sparse expressions went from 2250-2652 ms to 23-30 ms, and five distinct timezone expressions from 463-500 ms to 9.7-10.6 ms.
The new suite checks the optimized code against an inline copy of the previous implementation across eleven cron and timezone combinations plus five DST transitions, so the rewrite is verified as behaviour-preserving rather than just faster. Separate tests pin the invariant the single-step path depends on, so if sub-minute crons are ever allowed they fail loudly instead of the timings quietly going wrong.
Worth knowing for later:
cron-parserv5 is a much faster rewrite on exactly this workload (prev()under a timezone drops from roughly 2700 to 60 microseconds), but it is a breaking API change across several call sites including the schedule engine, so it belongs on its own. The differential test added here is the tool to de-risk it.