perf(month): bucket events by day before laying out the scrolling grids (#38)
Opening Continuous stayed slow, and the slowness tracked the number of enabled calendars rather than the window size — which pointed past the provider query to the layout behind it. layoutCalendarWeek opens by filtering the instances it is handed down to the ones touching its seven days. For a single month's grid that is nothing; for the scrolling styles it was a scan of every instance in an eleven-month window, repeated for each of a hundred-odd rows, with a time-zone conversion per check. The work grew with events × rows, so switching every calendar on multiplied it. - DayIndex buckets the window's events by the dates they cover, once, and each row takes the handful on its own seven days. Membership is decided by coversDay itself rather than re-derived from the timestamps — the all-day and timed cases have enough edge cases between them (UTC anchoring, exclusive ends, zero-length events at midnight) that a second implementation would drift. DayIndexTest pins row-for-row equality with the old full scan, in a zone east of UTC. - Only the style on screen is laid out. Adding Dense had quietly doubled the work, since both layouts were built from every load and one was always thrown away. - The paged flow no longer queries under a scrolling style, mirroring the gate the scrolling flow already had. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -85,8 +85,12 @@ class MonthViewModel @Inject constructor(
|
||||
val month: StateFlow<YearMonth> = _month
|
||||
|
||||
val state: StateFlow<MonthUiState> =
|
||||
combine(_month, weekStart) { ym, ws -> ym to ws }
|
||||
.flatMapLatest { (ym, ws) ->
|
||||
combine(_month, weekStart, viewStyle) { ym, ws, style -> Triple(ym, ws, style) }
|
||||
.flatMapLatest { (ym, ws, style) ->
|
||||
// The mirror of the gate below: the paged grid is what Paged and
|
||||
// Split draw, so under a scrolling style this query is a month
|
||||
// of provider work for a view that isn't on screen.
|
||||
if (style.isScrolling) return@flatMapLatest flowOf(MonthUiState.Loading)
|
||||
val range = monthGridRange(ym, ws, zone)
|
||||
combine(
|
||||
repository.calendars(),
|
||||
@@ -156,7 +160,7 @@ class MonthViewModel @Inject constructor(
|
||||
repository.calendars(),
|
||||
repository.instances(range),
|
||||
) { calendars, instances ->
|
||||
buildContinuousState(window, ws, calendars, instances)
|
||||
buildContinuousState(window, ws, style, calendars, instances)
|
||||
}
|
||||
}
|
||||
// A load landing is the cue to reach further out. Widening from here
|
||||
@@ -198,23 +202,35 @@ class MonthViewModel @Inject constructor(
|
||||
private fun buildContinuousState(
|
||||
window: IntRange,
|
||||
weekStart: DayOfWeek,
|
||||
style: MonthViewStyle,
|
||||
calendars: List<CalendarSource>,
|
||||
instances: List<EventInstance>,
|
||||
): ContinuousMonthUiState {
|
||||
if (calendars.isEmpty()) {
|
||||
return ContinuousMonthUiState.Failure(FailureReason.NoCalendarsConfigured)
|
||||
}
|
||||
val months = window.associateWith { index ->
|
||||
// Bucketed once for the whole window: a row then takes the handful of
|
||||
// events on its seven days instead of re-scanning a year of them.
|
||||
val byDay = DayIndex(instances, zone)
|
||||
// Only the style on screen is laid out. Building both doubled the work
|
||||
// for a layout nothing was going to draw.
|
||||
val months = if (style == MonthViewStyle.Continuous) {
|
||||
window.associateWith { index ->
|
||||
val ym = yearMonthForIndex(index)
|
||||
layoutMonthWeeks(ym, weekStart, instances, zone).map { clipWeekToMonth(it, ym) }
|
||||
layoutMonthWeeks(ym, weekStart, byDay, zone).map { clipWeekToMonth(it, ym) }
|
||||
}
|
||||
// The Dense style's rows, from the same query: every week the window's
|
||||
// months touch, laid out whole rather than clipped to a month.
|
||||
val weeks = weekWindowFor(window, weekStart).associateWith { index ->
|
||||
} else {
|
||||
emptyMap()
|
||||
}
|
||||
val weeks = if (style == MonthViewStyle.Dense) {
|
||||
weekWindowFor(window, weekStart).associateWith { index ->
|
||||
val days = (0 until 7).map {
|
||||
weekStartForIndex(index, weekStart).plus(it, DateTimeUnit.DAY)
|
||||
}
|
||||
layoutCalendarWeek(days, instances, zone)
|
||||
layoutCalendarWeek(days, byDay.eventsOn(days), zone)
|
||||
}
|
||||
} else {
|
||||
emptyMap()
|
||||
}
|
||||
return ContinuousMonthUiState.Success(
|
||||
today = todayDate,
|
||||
@@ -308,13 +324,68 @@ internal fun layoutMonthWeeks(
|
||||
weekStart: DayOfWeek,
|
||||
instances: List<EventInstance>,
|
||||
zone: TimeZone,
|
||||
): List<MonthWeek> = layoutMonthWeeks(ym, weekStart, DayIndex(instances, zone), zone)
|
||||
|
||||
/**
|
||||
* As above, but reusing a [DayIndex] already built for a wider range — what the
|
||||
* scrolling styles need, since they lay out many months from one query and
|
||||
* rebuilding the index per month would put the cost straight back.
|
||||
*/
|
||||
internal fun layoutMonthWeeks(
|
||||
ym: YearMonth,
|
||||
weekStart: DayOfWeek,
|
||||
byDay: DayIndex,
|
||||
zone: TimeZone,
|
||||
): List<MonthWeek> {
|
||||
val gridStart = firstOfMonth(ym).startOfGridWeek(weekStart)
|
||||
val weekCount = weekRowsInMonth(ym, weekStart)
|
||||
|
||||
return (0 until weekCount).map { row ->
|
||||
val days = (0 until 7).map { gridStart.plus(row * 7 + it, DateTimeUnit.DAY) }
|
||||
layoutCalendarWeek(days, instances, zone)
|
||||
layoutCalendarWeek(days, byDay.eventsOn(days), zone)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Events bucketed by the dates they cover, built once per load.
|
||||
*
|
||||
* [layoutCalendarWeek] opens by filtering the instances it was handed down to
|
||||
* the ones touching its seven days. That is fine for a single month's grid, but
|
||||
* the scrolling styles lay out a hundred-odd rows from one query, and scanning
|
||||
* *every* instance in an eleven-month window for each of them made the work grow
|
||||
* with the number of enabled calendars until opening the view stalled.
|
||||
*
|
||||
* Membership is decided by [coversDay] itself rather than by re-deriving the
|
||||
* rule from the timestamps: the all-day and timed cases have enough edge cases
|
||||
* between them (UTC anchoring, exclusive ends, zero-length events at midnight)
|
||||
* that a second implementation would drift. The walk only visits an event's own
|
||||
* candidate days, so the check runs a couple of times per event rather than once
|
||||
* per event per row.
|
||||
*/
|
||||
internal class DayIndex(instances: List<EventInstance>, private val zone: TimeZone) {
|
||||
|
||||
private val byDay: Map<LocalDate, List<EventInstance>> = buildMap<LocalDate, MutableList<EventInstance>> {
|
||||
instances.forEach { event ->
|
||||
// All-day events are anchored to UTC midnights; timed ones to the
|
||||
// device zone. Either way the candidate span is the event's own
|
||||
// dates, which is one or two days for almost everything.
|
||||
val anchor = if (event.isAllDay) TimeZone.UTC else zone
|
||||
var day = event.start.toLocalDateTime(anchor).date
|
||||
val lastCandidate = event.end.toLocalDateTime(anchor).date
|
||||
while (day <= lastCandidate) {
|
||||
if (event.coversDay(day, zone)) getOrPut(day) { mutableListOf() }.add(event)
|
||||
day = day.plus(1, DateTimeUnit.DAY)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Every event touching any of [days], each listed once, in input order. */
|
||||
fun eventsOn(days: List<LocalDate>): List<EventInstance> {
|
||||
if (days.isEmpty()) return emptyList()
|
||||
val hits = days.flatMap { byDay[it].orEmpty() }
|
||||
// A multi-day event is bucketed on each date it covers, so a row that
|
||||
// holds several of its days would otherwise list it several times.
|
||||
return if (hits.size < 2) hits else hits.distinctBy { it.instanceId }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
package de.jeanlucmakiola.calendula.ui.month
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import de.jeanlucmakiola.calendula.domain.EventInstance
|
||||
import kotlinx.datetime.DateTimeUnit
|
||||
import kotlinx.datetime.DayOfWeek
|
||||
import kotlinx.datetime.LocalDate
|
||||
import kotlinx.datetime.Month
|
||||
import kotlinx.datetime.TimeZone
|
||||
import kotlinx.datetime.YearMonth
|
||||
import kotlinx.datetime.atTime
|
||||
import kotlinx.datetime.minus
|
||||
import kotlinx.datetime.plus
|
||||
import kotlinx.datetime.toInstant
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
/**
|
||||
* [DayIndex] exists purely to make the scrolling styles cheap, so what it owes
|
||||
* is that cheapness changes *nothing*: a row laid out from the index must be the
|
||||
* row that would have come from scanning every instance.
|
||||
*
|
||||
* Run in a zone east of UTC, where the all-day anchoring and the day boundaries
|
||||
* disagree — the case that has bitten this codebase before (#65).
|
||||
*/
|
||||
class DayIndexTest {
|
||||
|
||||
private val zone = TimeZone.of("Europe/Berlin")
|
||||
private val mon = LocalDate(2026, 6, 8)
|
||||
private val week = (0..6).map { mon.plus(it, DateTimeUnit.DAY) }
|
||||
|
||||
private var nextId = 0L
|
||||
|
||||
private fun timed(from: LocalDate, startHour: Int, to: LocalDate, endHour: Int): EventInstance {
|
||||
val id = ++nextId
|
||||
return EventInstance(
|
||||
instanceId = id,
|
||||
eventId = id,
|
||||
calendarId = 1L,
|
||||
title = "T$id",
|
||||
start = from.atTime(startHour, 0).toInstant(zone),
|
||||
end = to.atTime(endHour, 0).toInstant(zone),
|
||||
isAllDay = false,
|
||||
color = 0xFF112233.toInt(),
|
||||
location = null,
|
||||
)
|
||||
}
|
||||
|
||||
private fun allDay(from: LocalDate, toInclusive: LocalDate): EventInstance {
|
||||
val id = ++nextId
|
||||
return EventInstance(
|
||||
instanceId = id,
|
||||
eventId = id,
|
||||
calendarId = 1L,
|
||||
title = "A$id",
|
||||
start = from.atTime(0, 0).toInstant(TimeZone.UTC),
|
||||
end = toInclusive.plus(1, DateTimeUnit.DAY).atTime(0, 0).toInstant(TimeZone.UTC),
|
||||
isAllDay = true,
|
||||
color = 0xFF445566.toInt(),
|
||||
location = null,
|
||||
)
|
||||
}
|
||||
|
||||
/** Every shape the layout distinguishes, plus the boundary cases around them. */
|
||||
private fun awkwardEvents(): List<EventInstance> = listOf(
|
||||
timed(mon, 9, mon, 10),
|
||||
// Crosses midnight into the next day.
|
||||
timed(mon.plus(1, DateTimeUnit.DAY), 23, mon.plus(2, DateTimeUnit.DAY), 1),
|
||||
// Ends exactly at midnight: covers its own day, not the next.
|
||||
timed(mon.plus(3, DateTimeUnit.DAY), 22, mon.plus(4, DateTimeUnit.DAY), 0),
|
||||
// Zero length, at midnight: covers nothing at all.
|
||||
timed(mon.plus(4, DateTimeUnit.DAY), 0, mon.plus(4, DateTimeUnit.DAY), 0),
|
||||
allDay(mon.plus(2, DateTimeUnit.DAY), mon.plus(2, DateTimeUnit.DAY)),
|
||||
// Runs in from before the row and out past its end.
|
||||
allDay(mon.minus(3, DateTimeUnit.DAY), mon.plus(9, DateTimeUnit.DAY)),
|
||||
// Wholly outside the row, both sides.
|
||||
timed(mon.minus(10, DateTimeUnit.DAY), 9, mon.minus(10, DateTimeUnit.DAY), 10),
|
||||
allDay(mon.plus(20, DateTimeUnit.DAY), mon.plus(21, DateTimeUnit.DAY)),
|
||||
// Two on one day, to pin the ordering the index hands back.
|
||||
timed(mon.plus(5, DateTimeUnit.DAY), 8, mon.plus(5, DateTimeUnit.DAY), 9),
|
||||
timed(mon.plus(5, DateTimeUnit.DAY), 14, mon.plus(5, DateTimeUnit.DAY), 15),
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `a row from the index is the row from a full scan`() {
|
||||
val events = awkwardEvents()
|
||||
val scanned = layoutCalendarWeek(week, events, zone)
|
||||
val indexed = layoutCalendarWeek(week, DayIndex(events, zone).eventsOn(week), zone)
|
||||
assertThat(indexed).isEqualTo(scanned)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a whole month agrees, row for row`() {
|
||||
val events = awkwardEvents()
|
||||
val jun = YearMonth(2026, Month.JUNE)
|
||||
DayOfWeek.entries.forEach { ws ->
|
||||
assertThat(layoutMonthWeeks(jun, ws, DayIndex(events, zone), zone))
|
||||
.isEqualTo(layoutMonthWeeks(jun, ws, events, zone))
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an event covering several of a row's days is listed once`() {
|
||||
// It is bucketed on each date it covers, so the row would otherwise see
|
||||
// it repeatedly and lay out a lane per copy.
|
||||
val long = allDay(mon, mon.plus(4, DateTimeUnit.DAY))
|
||||
val events = DayIndex(listOf(long), zone).eventsOn(week)
|
||||
assertThat(events).containsExactly(long)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `days no event touches come back empty`() {
|
||||
val index = DayIndex(listOf(timed(mon, 9, mon, 10)), zone)
|
||||
assertThat(index.eventsOn(listOf(mon.plus(1, DateTimeUnit.DAY)))).isEmpty()
|
||||
assertThat(index.eventsOn(emptyList())).isEmpty()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an empty calendar indexes to nothing`() {
|
||||
assertThat(DayIndex(emptyList(), zone).eventsOn(week)).isEmpty()
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user