Merge branch '232-app-is-unusably-slow-with-200-cycles' into 'master'

Resolve "app is unusably slow with ~200 cycles"

Closes #232

See merge request bloodyhealth/drip!99

components/calendar.js

@@ -5,7 +5,7 @@ import { getOrCreateCycleDay, getBleedingDaysSortedByDate } from '../db'
 import cycleModule from '../lib/cycle'
 import {shadesOfRed} from '../styles/index'
 import styles from '../styles/index'
-
+import nothingChanged from '../db/db-unchanged'
 
 export default class CalendarView extends Component {
   constructor(props) {
@@ -21,7 +21,8 @@ export default class CalendarView extends Component {
     this.bleedingDays.addListener(this.setStateWithCalFormattedDays)
   }
 
-  setStateWithCalFormattedDays = () => {
+  setStateWithCalFormattedDays = (_, changes) => {
+    if (nothingChanged(changes)) return
     const predictedMenses = cycleModule().getPredictedMenses()
     this.setState({
       bleedingDaysInCalFormat: toCalFormat(this.bleedingDays),

components/chart/chart.js

@@ -19,6 +19,7 @@ import MucusIcon from '../../assets/mucus'
 import NoteIcon from '../../assets/note'
 import PainIcon from '../../assets/pain'
 import SexIcon from '../../assets/sex'
+import nothingChanged from '../../db/db-unchanged'
 
 export default class CycleChart extends Component {
   constructor(props) {
@@ -47,8 +48,7 @@ export default class CycleChart extends Component {
   onLayout = ({ nativeEvent }) => {
     if (this.state.chartHeight) return
     const height = nativeEvent.layout.height
-    this.setState({ chartHeight: height })
+    const reCalculateChartInfo = () => {
-    this.reCalculateChartInfo = () => {
       // how many symptoms need to be displayed on the chart's upper symptom row?
       this.symptomRowSymptoms = [
         'bleeding',
@@ -64,8 +64,8 @@ export default class CycleChart extends Component {
         })
       })
 
-      this.xAxisHeight = this.state.chartHeight * config.xAxisHeightPercentage
+      this.xAxisHeight = height * config.xAxisHeightPercentage
-      const remainingHeight = this.state.chartHeight - this.xAxisHeight
+      const remainingHeight = height - this.xAxisHeight
       this.symptomHeight = config.symptomHeightPercentage * remainingHeight
       this.symptomRowHeight = this.symptomRowSymptoms.length *
         this.symptomHeight
@@ -76,16 +76,35 @@ export default class CycleChart extends Component {
         this.chartSymptoms.push('temperature')
       }
 
-      const columnData = this.makeColumnInfo()
+      const columnData = this.makeColumnInfo(nfpLines(), this.chartSymptoms)
-      this.setState({ columns: columnData })
+      this.setState({
+        columns: columnData,
+        chartHeight: height
+      })
     }
 
-    this.cycleDaysSortedByDate.addListener(this.reCalculateChartInfo)
+    reCalculateChartInfo()
-    this.removeObvListener = scaleObservable(this.reCalculateChartInfo, false)
+    this.updateListeners(reCalculateChartInfo)
+  }
+
+  updateListeners(dataUpdateHandler) {
+    // remove existing listeners
+    if(this.handleDbChange) {
+      this.cycleDaysSortedByDate.removeListener(this.handleDbChange)
+    }
+    if (this.removeObvListener) this.removeObvListener()
+
+    this.handleDbChange = (_, changes) => {
+      if (nothingChanged(changes)) return
+      dataUpdateHandler()
+    }
+
+    this.cycleDaysSortedByDate.addListener(this.handleDbChange)
+    this.removeObvListener = scaleObservable(dataUpdateHandler, false)
   }
 
   componentWillUnmount() {
-    this.cycleDaysSortedByDate.removeListener(this.reCalculateChartInfo)
+    this.cycleDaysSortedByDate.removeListener(this.handleDbChange)
     this.removeObvListener()
   }
 

components/home.js

@@ -11,6 +11,7 @@ import { getOrCreateCycleDay, getCycleDaysSortedByDate } from '../db'
 import { getFertilityStatusForDay } from '../lib/sympto-adapter'
 import styles from '../styles'
 import AppText, { AppTextLight } from './app-text'
+import nothingChanged from '../db/db-unchanged'
 
 export default class Home extends Component {
   constructor(props) {
@@ -32,7 +33,8 @@ export default class Home extends Component {
     this.cycleDays.addListener(this.updateState)
   }
 
-  updateState = () => {
+  updateState = (_, changes) => {
+    if (nothingChanged(changes)) return
     const prediction = this.getBleedingPrediction()
     const fertilityStatus = getFertilityStatusForDay(this.todayDateString)
     this.setState({

db/db-unchanged.js

@@ -0,0 +1,7 @@
+// when data changes, realm gives us an object with updates
+// https://realm.io/docs/javascript/latest/#collection-notifications
+// but it sometimes gets fired even though there are no changes
+// - we want to check for that and see if all arrays are empty
+export default function (dbChanges) {
+  return Object.values(dbChanges).every(changeArray => changeArray.length === 0)
+}

lib/cycle.js

@@ -26,70 +26,49 @@ export default function config(opts) {
     minCyclesForPrediction = opts.minCyclesForPrediction || 3
   }
 
-  function getLastMensesStart(targetDateString) {
+  function findLatestMensesStart(bleedingDays) {
-    const targetDate = LocalDate.parse(targetDateString)
+    if (!bleedingDays.length) return null
-    const withWrappedDates = bleedingDaysSortedByDate
-      .filter(day => !day.bleeding.exclude)
-      .map(day => {
-        day.wrappedDate = LocalDate.parse(day.date)
-        return day
-      })
 
-    // the index of the first bleeding day before the target day
+    // assumes bleeding days are ordered latest first, and
-    const index = withWrappedDates.findIndex(day => {
+    // excluded values already removed
-      return (
+    const lastMensesStart = bleedingDays.find((day, i) => {
-        day.wrappedDate.isEqual(targetDate) ||
+      return noBleedingDayWithinThreshold(day, bleedingDays.slice(i + 1))
-        day.wrappedDate.isBefore(targetDate)
-      )
-    })
-
-    if (index < 0) {
-      withWrappedDates.forEach(day => delete day.wrappedDate)
-      return null
-    }
-
-    const prevBleedingDays = withWrappedDates.slice(index)
-
-    const lastMensesStart = prevBleedingDays.find((day, i) => {
-      return noBleedingDayWithinThreshold(day, prevBleedingDays.slice(i + 1))
     })
 
     function noBleedingDayWithinThreshold(day, previousBleedingDays) {
-      const periodThreshold = day.wrappedDate.minusDays(maxBreakInBleeding + 1)
+      const localDate = LocalDate.parse(day.date)
-      return !previousBleedingDays.some(({ wrappedDate }) => {
+      const threshold = localDate.minusDays(maxBreakInBleeding + 1).toString()
-        return (
+      return !previousBleedingDays.some(({ date }) => date >= threshold)
-          wrappedDate.equals(periodThreshold) ||
-          wrappedDate.isAfter(periodThreshold)
-        )
-      })
     }
 
-    withWrappedDates.forEach(day => delete day.wrappedDate)
     return lastMensesStart
   }
 
-  function getFollowingMensesStart(targetDateString) {
+  function getLastMensesStartForDay(targetDateString) {
-    const targetDate = LocalDate.parse(targetDateString)
+    // the index of the first bleeding day before the target day
-    const withWrappedDates = bleedingDaysSortedByDate
+    const index = bleedingDaysSortedByDate.findIndex(day => {
+      return day.date <= targetDateString && !day.bleeding.exclude
+    })
+
+    if (index < 0) return null
+
+    const prevBleedingDays = bleedingDaysSortedByDate.slice(index)
+    return findLatestMensesStart(prevBleedingDays)
+  }
+
+  function getFollowingMensesStartForDay(targetDateString) {
+    const followingBleedingDays = bleedingDaysSortedByDate
       .filter(day => !day.bleeding.exclude)
-      .map(day => {
-        day.wrappedDate = LocalDate.parse(day.date)
-        return day
-      })
-
-    const firstBleedingDayAfterTargetDay = withWrappedDates
       .reverse()
-      .find(day => {
-        return day.wrappedDate.isAfter(targetDate)
-      })
 
-    withWrappedDates.forEach(day => delete day.wrappedDate)
+    const firstBleedingDayAfterTargetDay = followingBleedingDays
+      .find(day => day.date > targetDateString)
 
     return firstBleedingDayAfterTargetDay
   }
 
   function getCycleDayNumber(targetDateString) {
-    const lastMensesStart = getLastMensesStart(targetDateString)
+    const lastMensesStart = getLastMensesStartForDay(targetDateString)
     if (!lastMensesStart) return null
     const targetDate = LocalDate.parse(targetDateString)
     const lastMensesLocalDate = LocalDate.parse(lastMensesStart.date)
@@ -111,7 +90,7 @@ export default function config(opts) {
   }
 
   function getPreviousCycle(dateString) {
-    const startOfCycle = getLastMensesStart(dateString)
+    const startOfCycle = getLastMensesStartForDay(dateString)
     if (!startOfCycle) return null
     const dateBeforeStartOfCycle = LocalDate
       .parse(startOfCycle.date)
@@ -123,10 +102,10 @@ export default function config(opts) {
 
   function getCycleForDay(dayOrDate) {
     const dateString = typeof dayOrDate === 'string' ? dayOrDate : dayOrDate.date
-    const cycleStart = getLastMensesStart(dateString)
+    const cycleStart = getLastMensesStartForDay(dateString)
     if (!cycleStart) return null
     const cycleStartIndex = cycleDaysSortedByDate.indexOf(cycleStart)
-    const nextMensesStart = getFollowingMensesStart(dateString)
+    const nextMensesStart = getFollowingMensesStartForDay(dateString)
     if (nextMensesStart) {
       return cycleDaysSortedByDate.slice(
         cycleDaysSortedByDate.indexOf(nextMensesStart) + 1,
@@ -137,16 +116,20 @@ export default function config(opts) {
     }
   }
 
-  function getAllMensesStarts(day, collectedDates) {
+  function getAllMensesStarts(initialBleedingDays = bleedingDaysSortedByDate) {
-    day = day || LocalDate.now().toString()
+    return recurse(initialBleedingDays.filter(d => !d.bleeding.exclude))
+
+    function recurse(bleedingDays, collectedDates) {
       collectedDates = collectedDates || []
-    const lastStart = getLastMensesStart(day)
+      const lastStart = findLatestMensesStart(bleedingDays)
       if (!lastStart) {
         return collectedDates
       } else {
-      const newDay = LocalDate.parse(lastStart.date).minusDays(1).toString()
         collectedDates.push(lastStart.date)
-      return getAllMensesStarts(newDay, collectedDates)
+        const index = bleedingDays.indexOf(lastStart)
+        const remainingDays = bleedingDays.slice(index + 1)
+        return recurse(remainingDays, collectedDates)
+      }
     }
   }
 

lib/notifications.js

@@ -5,6 +5,7 @@ import Moment from 'moment'
 import { settings as labels } from '../components/labels'
 import { getOrCreateCycleDay, getBleedingDaysSortedByDate } from '../db'
 import cycleModule from './cycle'
+import nothingChanged from '../db/db-unchanged'
 
 export default function setupNotifications(navigate) {
   Notification.configure({
@@ -40,14 +41,16 @@ export default function setupNotifications(navigate) {
         repeatType: 'day'
       })
     }
-  })
+  }, false)
 
   periodReminderObservable(reminder => {
     Notification.cancelLocalNotifications({id: '2'})
     if (reminder.enabled) setupPeriodReminder()
-  })
+  }, false)
 
-  getBleedingDaysSortedByDate().addListener(() => {
+  getBleedingDaysSortedByDate().addListener((_, changes) => {
+    // the listener fires on setup, so we check if there were actually any changes
+    if (nothingChanged(changes)) return
     Notification.cancelLocalNotifications({id: '2'})
     if (periodReminderObservable.value.enabled) setupPeriodReminder()
   })

test/cycle.spec.js

@@ -1,6 +1,7 @@
 import chai from 'chai'
 import dirtyChai from 'dirty-chai'
 import cycleModule from '../lib/cycle'
+import { LocalDate } from 'js-joda'
 
 const expect = chai.expect
 chai.use(dirtyChai)
@@ -625,7 +626,8 @@ describe('getAllMensesStart', () => {
     const result = getAllMensesStarts()
     expect(result.length).to.eql(2)
     expect(result).to.eql(['2018-06-01', '2018-05-01'])
-  }),
+  })
+
   it('works for two cycle starts with excluded data', () => {
     const cycleDaysSortedByDate = [
       {
@@ -649,7 +651,8 @@ describe('getAllMensesStart', () => {
     const result = getAllMensesStarts()
     expect(result.length).to.eql(2)
     expect(result).to.eql(['2018-06-01', '2018-05-01'])
-  }),
+  })
+
   it('returns an empty array if no bleeding days are given', () => {
     const cycleDaysSortedByDate = [ {} ]
 
@@ -661,4 +664,25 @@ describe('getAllMensesStart', () => {
     expect(result.length).to.eql(0)
     expect(result).to.eql([])
   })
+
+  it('is not slow with 500 menses starts', () => {
+    const startDate = LocalDate.parse('2018-10-01')
+    const cycleDaysSortedByDate = Array(500)
+      .fill(null)
+      .map((_, i) => {
+        return {
+          date: startDate.minusMonths(i).toString(),
+          bleeding: { value: 2 }
+        }
+      })
+    const { getAllMensesStarts } = cycleModule({
+      cycleDaysSortedByDate,
+      bleedingDaysSortedByDate: cycleDaysSortedByDate
+    })
+    const start = Date.now()
+    const result = getAllMensesStarts()
+    const duration = Date.now() - start
+    expect(result.length).to.eql(500)
+    expect(duration).to.be.lessThan(100)
+  })
 })