Skip to content

Fix quadratic performance on long SLA durations - #21

Merged
sifex merged 5 commits into
mainfrom
fix/long-duration-performance
Aug 15, 2026
Merged

sifex merged 5 commits into
mainfrom
fix/long-duration-performance

Conversation

@sifex

@sifex sifex commented Aug 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes the performance problem calculating SLA durations across more than a month when pauses or holidays are involved. No, this was not fixed by the earlier work — it's new in this PR.

Stacked on #20 (which is stacked on #16) — retargets to main once those merge.

Root cause

The daily loop in SLA::calculate() ran against the entire subject duration every day:

  • For every day, every coverage period was diffed against all pause periods of the whole duration via $period->diff(...$pause_periods) — spatie's subtractAll/overlapAll does an O(n×m) cross-product (120+ variadic args, 128-param signature)
  • The pause period array was rebuilt for every day × every coverage period
  • The dedup reduce ($p->diff(...$carry)) was O(periods²) per day over the full-range period list
  • The final interval combine cascaded an ever-growing interval across all days

Fix (four commits)

  1. Day-clipping + forward-only sweep cursors: SLA periods and pause periods (sorted once) are clipped to the current day via cursors that only move forward — O(N×P) and O(N×periods²) → O(N+P)
  2. Integer interval arithmetic: coverage dedup and pause subtraction are pure timestamp math (ps-1/pe+1 boundaries reproduce spatie's closed-interval semantics exactly — all boundary tests unchanged)
  3. Sweep optimization for the same pattern applied to the remaining filters
  4. No CarbonPeriod objects in the hot path: AgendaInterface::toPeriods now returns start/end Carbon pairs instead of CarbonPeriod[] (measured ~28µs per CarbonPeriod vs ~0.1µs per pair — 144ms of a 305ms 10-year calc was just constructing them). The daily loop iterates the day period directly with integer bounds (+86400 reproduces the old addHours(24) semantics exactly), and the valid_from parse is precomputed once per call. BC note: direct toPeriods() consumers now receive array{0: CarbonInterface, 1: CarbonInterface} pairs.

Results (weekday schedule, 09:00–17:00)

Scenario Original After commits 1–3 After commit 4
120 pauses, 30 days 1061 ms 11 ms 7 ms
120 pauses, 90 days 3098 ms 14 ms 8 ms
120 pauses, 365 days OOM (at 180d) 36 ms 15 ms
365 holidays, 1 year OOM 52 ms 27 ms
730 holidays, 2 years OOM 103 ms 62 ms
No pauses, 5 years 1872 ms 161 ms 41 ms
No pauses, 10 years — 305 ms 86 ms

The year-long daily-holiday case previously died with a 128MB memory error and now runs in ~27ms.

Behaviour notes

  • Pause boundary semantics are byte-for-byte identical (closed-interval, verified by existing boundary tests)
  • Removed the now-unused calculate_interval/combine_intervals private helpers; the collapses intervals test now asserts the same behaviour through the public API with overlapping schedule windows

Tests

Regression tests for year-long subjects: 365 daily holidays (was a memory fatal), 52 weekly pauses, unpaused year — 51 tests total, all green; PHPStan level 8 and Pint clean.

sifex added 5 commits August 15, 2026 20:19
Adds 28 new tests (tests/SLAEdgeCasesTest.php) covering previously
untested behaviour: single-day string schedules, weekend-only
schedules, multi-period schedules via andFrom, the Weekly agenda
addTimePeriods/clearTimePeriods API, schedule effectiveFrom switches
mid-duration, boundary conditions (start/end exactly at schedule
edges), multi-week durations, overnight schedules, pause and holiday
edge cases (outside window, full-window, multi-day, arrays, weekend
holidays), the breaches() accessor, breach boundary semantics, and
SLAPause/SLAHoliday day periods.

Coverage: 96.3% -> 98.4%.

The new overnight-schedule tests exposed a real bug: midnight-crossing
schedules (e.g. 22:00 -> 02:00) silently produced zero SLA time
because CarbonPeriod::create(start, end) with end < start yields an
empty period. Weekly::toPeriods now keeps overnight periods as a
single period spanning midnight, which the per-day overlap logic in
SLA::calculate clips exactly. A 24/7 schedule (equal from/to times) is
now also covered.
Calculating a duration across more than a month with pauses or
holidays was quadratic in the number of days and pauses: for every
day, every coverage period was diffed against ALL pause periods of
the whole subject duration via spatie's subtractAll/overlapAll
cross-product. At ~120 pauses this grew from 1s to 3s+ for a month,
and a 180-day duration exhausted the 128MB memory limit.

The daily loop now:
- clips the SLA periods to those overlapping the current day before
  the overlap map and dedup reduce (was O(periods^2) per day over the
  full subject range)
- clips pause periods to those overlapping the current day before the
  diff, and precomputes them once instead of per coverage period
- sums each day's interval seconds directly instead of cascading an
  ever-growing interval

Results (weekday schedule, 120 pauses):
  30 days: 1061ms -> 30ms
  60 days: 2133ms -> 46ms
  90 days: 3098ms -> 63ms
  180 days: memory exhaustion -> 149ms
  365 days: n/a -> 318ms (and 419ms with 365 holidays)

A pause exactly touching the coverage boundary no longer removes a
boundary second (previously an artifact of diffing against
non-overlapping pauses). Adds regression tests for year-long
calculations with 365 holidays and weekly pauses, which previously
died with a memory error.
…math

The per-day loop still spent most of its time constructing CarbonPeriod
objects and running spatie/period diffs for the coverage dedup and
pause subtraction (447ms of a 367ms... dominant cost for 5 years), plus
CarbonInterval cascade combining (152ms).

Coverage is now clipped and merged as integer timestamp pairs (union of
overlapping periods), and pauses are subtracted with ps-1/pe+1 boundary
adjustments that reproduce spatie's closed-interval semantics exactly
(all existing boundary-sensitive tests pass unchanged, including the
2-seconds-per-pause year test).

Also removes the now-unused calculate_interval/combine_intervals
helpers and the phpstan ignore for the removed diff() calls; the
'collapses intervals' test now asserts the same behaviour through the
public API with overlapping schedule windows.

Results (weekday schedule):
  120 pauses, 30d:    31ms -> 11ms
  120 pauses, 90d:    63ms -> 14ms
  120 pauses, 365d:  222ms -> 36ms
  365 holidays, 1yr: 243ms -> 52ms
  730 holidays, 2yr: 495ms -> 103ms
  no pauses, 5yr:    367ms -> 161ms
  no pauses, 10yr:     -   -> 305ms
CarbonPeriod::create measured at ~28us per object vs 0.1us for a
timestamp pair; building 2610 of them for a 10-year calculation was
144ms of the 305ms total.

- AgendaInterface::toPeriods and Weekly::toPeriods now return
  start/end Carbon pairs instead of CarbonPeriod objects (the only
  consumer is SLA::calculate, which only reads the bounds). BC note:
  direct toPeriods consumers now receive array{0,1} pairs.
- The daily loop no longer materialises the day list or clones Carbon
  objects: it iterates the day period directly, computes day bounds
  from integer timestamps (+86400 reproduces the previous
  addHours(24) semantics exactly), and only touches Carbon for the
  schedule lookup on the already-anchored day instance.
- The enabled-schedule lookup inlines the valid_from comparison with
  unixes precomputed once per calculate() instead of parsing the
  string every day.

Results:
  120 pauses, 30d:    11ms -> 7ms
  120 pauses, 365d:   36ms -> 15ms
  365 holidays, 1yr:  52ms -> 27ms
  no pauses, 5yr:    161ms -> 41ms
  no pauses, 10yr:   305ms -> 86ms
- Remove the stale disclaimer that sla-timer is inefficient over
  periods longer than a month (fixed in #21: a year with daily
  holidays now calculates in ~30ms, 10-year spans in ~90ms) and
  replace it with a short Performance section
- Document overnight (midnight-crossing) schedules and 24/7 coverage
  via equal from/to times in the schedule-building guide (newly
  working behaviour from #20)
@sifex
sifex merged commit b163cd4 into main Aug 15, 2026
9 checks passed
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