Refactor loadHomeCachedLogs to use an IDB index

[jgaehring]

So I just made a first pass at the issue of loading a subset of logs for the Home screen, as first outlined in https://github.com/farmOS/farmOS-client/issues/286#issuecomment-589025391. It works, but it's a monster:

https://github.com/farmOS/farmOS-client/blob/9e40b756ceb8a62e8e98317cd05558bf9bdee663/src/core/store/idb/module.js#L69-L155

I don't know the O notation for this algorithm, but I'm sure it isn't good. One positive note, however, is that it shouldn't consume a lot of memory, because although it iterates through all the logs in the IDB store, it only ever holds as many as 11 of them in memory at one time. So there's that.

Talking with @mstenta though I realized a much better solution to this is to set up an IDB index, similar to an SQL index like farmOS itself uses. I could probably just use the index in place of the object store in the getRecords function:

https://github.com/farmOS/farmOS-client/blob/9e40b756ceb8a62e8e98317cd05558bf9bdee663/src/core/store/idb/idb.js#L26-L50

I could then use the index.getAll() method, which is pretty much the same as store.getAll(), but I could pass in an IDBKeyRange as the first parameter, and count as the second parameter to limit how many records are retrieved.

Ideally, with getRecords refactored to take some sort of configuration object, I could then have a loadHomeCachedLogs that looks something like this:

const loadLogs = query => openDatabase()
  .then(db => getRecords(db, logStore.name, query))
  .then((results) => {
    const cachedLogs = results.map(log => (
      updateLog(log, { isCachedLocally: true })
    ));
    commit('addLogs', cachedLogs);
  });

const lateLogsQuery = {
  // Some configuration to only retrieve undone logs with timestamps before today.
};
const upcomingLogsQuery = {
  // Some configuration to only retrieve any undone logs timestamped from
  // today to next month.
};
const doneLogsQuery = {
  // Some configuration to only retrieve the 10 most recent done logs.
};
[lateLogsQuery, upcomingLogsQuery, doneLogsQuery].forEach(loadLogs);

For that to work as desired, I might have to use some combination of index.getAll() and index.openCursor(), the latter being controlled by some sort of predicate function that would be supplied in the configuration object. We'll see.

Finally, I'm not sure if I want to do this refactoring right away, since I do at least have a method that works, albeit slowly. I think for the sake of expediency I will hold off until I've either completed the Spraying module, which is past due, or until the need arises while working on that module. I'm adding this to the 1.1 milestone for now, though it could get bumped up in priority as things progress.

[tool172]

Only non completed logs are open right? I'm just starting to scan the source and api.

On Sun, Apr 12, 2020, 15:37 Jamie Gaehring notifications@github.com wrote:

So I just made a first pass at the issue of loading a subset of logs for the Home screen, as first outlined in #286 (comment) https://github.com/farmOS/farmOS-client/issues/286#issuecomment-589025391. It works, but it's a monster:

https://github.com/farmOS/farmOS-client/blob/9e40b756ceb8a62e8e98317cd05558bf9bdee663/src/core/store/idb/module.js#L69-L155

I don't know the O notation for this algorithm, but I'm sure it isn't good. One positive note, however, is that it shouldn't consume a lot of memory, because although it iterates through all the logs in the IDB store, it only ever holds as many as 11 of them in memory at one time. So there's that.

Talking with @mstenta https://github.com/mstenta though I realized a much better solution to this is to set up an IDB index https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB#Using_an_index, similar to an SQL index like farmOS itself uses. I could probably just use the index in place of the object store in the getRecords function:

https://github.com/farmOS/farmOS-client/blob/9e40b756ceb8a62e8e98317cd05558bf9bdee663/src/core/store/idb/idb.js#L26-L50

I could then use the index.getAll() method, which is pretty much the same as store.getAll(), but I could pass in an IDBKeyRange https://developer.mozilla.org/en-US/docs/Web/API/IDBKeyRange as the first parameter, and count as the second parameter to limit how many records are retrieved.

Ideally, with getRecords refactored to take some sort of configuration object, I could then have a loadHomeCachedLogs that looks something like this:

const loadLogs = query => openDatabase() .then(db => getRecords(db, logStore.name, query)) .then((results) => { const cachedLogs = results.map(log => ( updateLog(log, { isCachedLocally: true }) )); commit('addLogs', cachedLogs); }); const lateLogsQuery = { // Some configuration to only retrieve undone logs with timestamps before today. };const upcomingLogsQuery = { // Some configuration to only retrieve any undone logs timestamped from // today to next month. };const doneLogsQuery = { // Some configuration to only retrieve the 10 most recent done logs. }; [lateLogsQuery, upcomingLogsQuery, doneLogsQuery].forEach(loadLogs);

For that to work as desired, I might have to use some combination of index.getAll() and index.openCursor(), the latter being controlled by some sort of predicate function that would be supplied in the configuration object. We'll see.

Finally, I'm not sure if I want to do this refactoring right away, since I do at least have a method that works, albeit slowly. I think for the sake of expediency I will hold off until I've either completed the Spraying module, which is past due, or until the need arises while working on that module. I'm adding this to the 1.1 milestone for now, though it could get bumped up in priority as things progress.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/farmOS/farmOS-client/issues/339, or unsubscribe https://github.com/notifications/unsubscribe-auth/APERD7CUEB6VSGK67IK4523RMIRBJANCNFSM4MGRTRGQ .

[jgaehring]

Only non completed logs are open right?

Only non-completed logs are pulled from the server, but it's possible to create completed logs locally, so both complete and incomplete logs are being stored in the local database.

I'm just starting to scan the source and api.

Awesome, thanks for taking an interest, @tool172! If you want to clone the repo and experiment on your own, be sure to check out the development documentation. If you're interested in customizing the interface, I suggest reading up on the Field Module system we're just about done implementing: #217. If you have any questions, please follow up on our forum or the farmOS Riot channel.

[jgaehring]

I believe this is no longer necessary with the new syncing and caching API. Something like this may still prove helpful if we run into performance issues in the future with the loadCachedLogs action, but for now that seems like premature optimization.