Skip to content

perf(cron): batch advance_next_run for the due-dispatch loop - #25

Open
spfcraze wants to merge 1 commit into
mainfrom
fix/cron-batch-advance
Open

perf(cron): batch advance_next_run for the due-dispatch loop#25
spfcraze wants to merge 1 commit into
mainfrom
fix/cron-batch-advance

Conversation

@spfcraze

@spfcraze spfcraze commented Aug 1, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

The scheduler's pre-dispatch loop called advance_next_run per due job — one full load_jobs() + one full save_jobs() of the jobs file each — so N due jobs cost N reads + N writes of the whole file (gateway-restart catch-up or co-scheduled bursts). Add advance_next_runs(): one load + at most one save for the whole due set with identical per-job semantics; advance_next_run() becomes a thin wrapper.

Related Issue

No direct issue — discovered via code review and reproduced live (see below).
Related PRs reviewed during the duplicate check (none covers this change):

Changes Made

  • fix/cron-batch-advance — 3 file(s) changed vs base:
    • cron/jobs.py
    • cron/scheduler.py
    • tests/cron/test_jobs.py

cron/jobs.py: advance_next_runs() batch primitive (identical skip semantics for one-shots/unknown ids, no save when nothing advances; crash-window tradeoff documented); advance_next_run() is now a thin wrapper preserving its exact contract. cron/scheduler.py: the pre-dispatch loop is one batch call. tests/cron/test_jobs.py: +4 tests — batch semantics (recurring advanced, one-shots skipped), single load+save I/O pin, no-save-when-noop, wrapper contract. Related: NousResearch#60946 and NousResearch#75833 restructure this call site for correctness — neither addresses the I/O cost; the batch primitive composes with either dispatch design.

How to Test

Measured on a real jobs file with 50 due recurring jobs (repo venv): the per-job loop costs 107.9 ms (50 load_jobs + 50 save_jobs); the batch call costs 2.5 ms (1 + 1) — 45x. Burst-only path: gateway-restart catch-up or many co-scheduled jobs firing in one tick.

Validation completed (recorded by prp):

  1. Sabotage check: pre-fix code fails the regression tests (3 failed), with the fix all pass (58 passed, 0 failed) — target tests/cron/test_jobs.py.
  2. Suite tests/cron/: branch 377 passed / 0 failed vs baseline 373 passed / 0 failed — zero branch-only failures.
  3. Duplicate check: 73 potential matches reviewed — none covers this change.
  4. The full repo-wide suite was not run for this change; GitHub CI owns full-suite validation.

Logs

Sabotage verification output:

# base leg (pre-fix code + branch tests):
#   tests: 55 passed, 3 failed
# head leg (with fix):
#   tests: 58 passed, 0 failed

@spfcraze
spfcraze force-pushed the fix/cron-batch-advance branch from ec5e7ec to 1a42b6d Compare August 1, 2026 16:40
The scheduler's pre-dispatch loop called advance_next_run per due job —
one full load_jobs() + one full save_jobs() of the jobs file each — so
N due jobs cost N reads + N writes of the whole file (gateway-restart
catch-up or co-scheduled bursts). advance_next_runs() does one load +
at most one save for the whole due set with identical per-job semantics;
advance_next_run() is now a thin wrapper over it.

Measured (50 due recurring jobs, real jobs file): 107.9 ms -> 2.5 ms
(45x; 50 loads + 50 saves -> 1 + 1).

Tests: batch advances recurring and skips one-shots, single load + save
I/O pin (fails pre-fix — no such function), no save when nothing
advances, and per-job wrapper semantics unchanged. Related: NousResearch#60946 and
NousResearch#75833 both restructure this loop's call site for correctness — neither
addresses the I/O cost, and this batch primitive composes with either
dispatch design; happy to rebase onto whichever lands first.
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.

1 participant