Performance improvements - skipping already finished events - #177
Merged
Conversation
Every call fully augmented every entry in the collection, then threw almost all of it away. On a real site with 548 events, 527 of them had finished long ago, and generating their occurrences accounted for roughly 90% of the ~1.6s an upcoming() call took. Reject those up front, using only raw values. Augmentation is what's expensive here (an augmented grid field costs ~30ms, the raw array 0ms), so the check has to stay off the augmented values to be worth anything. An event is only skipped when its last possible date is known to have passed, which is not the same as having started in the past: - non-recurring: start_date is the only occurrence - recurring: end_date, and no end_date means it recurs forever - multi-day: the last of its days So an event that started two years ago and recurs annually is kept, and querying a past range still returns what it always did. Measured on that site, upcoming(30): ~1646ms -> ~195ms, with byte identical output for both upcoming() and a between() spanning three years of past events.
edalzell
marked this pull request as ready for review
August 5, 2026 22:36
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
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
Events::upcoming()/between()fully augment every entry in the collection, then throw almost all of it away. Collections are mostly made up of events that finished long ago, so the vast majority of that work is wasted.On a real site with 548 events, 527 had already finished. Generating their occurrences accounted for roughly 90% of the ~1.6s an
upcoming(30)call took — no SQL involved, it's all Stache augmentation.Fix
Reject already-ended events up front, in
occurrences(), before the generator runs.The important part is how the check is made: it reads only raw values. Augmentation is the expensive thing here — measured on one multi-day entry, augmented
$entry->dayscosts ~30ms while raw$entry->get('days')costs 0ms — so a check that touched augmented values would just relocate the cost instead of removing it.An event is skipped only when its last possible date is known to have passed, which is deliberately not the same as "started in the past":
start_date— the only occurrenceend_date; noend_datemeans it recurs foreverdaysSo an event that started two years ago and recurs annually is kept, and querying a past range still returns everything it used to. Dates that can't be parsed fall through rather than being excluded, and there's two days of slack because the event's own timezone isn't knowable without augmenting.
Results
Measured on that 548-event site,
upcoming(30):Output is byte-identical — verified by diffing full dumps against unmodified
main: 87 rows forupcoming(30), and 1082 rows for abetween()spanning three years of past events.Test plan
tests/SkipsEndedEventsTest.phpcovering the cases this could plausibly get wrong: unbounded recurring events that started years ago, annually recurring events, recurring events with a future vs passedend_date, multi-day events that are past/partway through, and past-rangebetween()queries.start_dateis in the past" filter instead. 4 of the 8 fail against it, including every unbounded-recurrence case and the past-range query.References https://github.com/transformstudios/zakat.org/issues/2322