Use IndexedDB indexes on gatherLocalChanges() #315
Merged
Conversation
This file contains 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
| @@ -205,12 +205,30 @@ export default class IDB extends BaseAdapter { | |||
| * @override | |||
| * @return {Promise} | |||
| */ | |||
| list() { | |||
| list(params={}) { | |||
There was a problem hiding this comment.
we should provide default property values for the params object; eg Object.keys(params.filters) can fail when no filters object is provided.
leplatrem
force-pushed
the
use-idb-keyrange
branch
from
c29d855
to
6cb2f66
Compare
|
Ok, I just realized we were interating on the whole database even when no change was fetched from server! Sync() is now 33 msec when no change is fetched (=138-105 network) :)
|
| return Promise.all([ | ||
| this.list({order: "", filters: {_status: "deleted"}}, {includeDeleted: true}), | ||
| this.list({order: "", filters: {_status: "synced"}}, {includeDeleted: true}), | ||
| ]) |
There was a problem hiding this comment.
Nit: refactoring suggestion:
const listStatus = (_status) => {
return this.list({order: "", filters: {_status}}, {includeDeleted: true});
};
return Promise.all([listStatus("deleted"), listStatus("synced")])There was a problem hiding this comment.
As we're using the same query type later, we could even extract the function as something reusable:
function listStatus(coll, status, options={includeDeleted: true}) {
return coll.list({order: "", filters: {_status: status}}, options);
};Or if we want to pick all records matching provided statuses:
function listStatuses(coll, statuses, options={includeDeleted: true}) {
return Promise.all(statuses.map(status => {
return coll.list({order: "", filters: {_status: status}}, options);
})).then((...results) => [].concat.apply([], results));
};Usage:
return listStatuses(this, ["deleted", "synced"]);Optimized synchronization at several places: - during scan of un-synced records - when no change was made on server In terms of API, the adapters are now in charge of adapting #list() parameters (filters and sort). This PR does not break any API, the filtering and sorting is done like before using a reduce from utils.js
leplatrem
force-pushed
the
use-idb-keyrange
branch
from
fcb2a4a
to
9c19c10
Compare
leplatrem
changed the title
|
Ready for review/merge. |
| @@ -205,12 +206,33 @@ export default class IDB extends BaseAdapter { | |||
| * @override | |||
| * @return {Promise} | |||
| */ | |||
| list() { | |||
| list(params={filters: {}, order: ""}) { | |||
There was a problem hiding this comment.
Can't we simply avoid passing order here?
|
r+ |
n1k0
added a commit
that referenced
this pull request
Feb 2, 2016
Use IndexedDB indexes on gatherLocalChanges()
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.
Small benchmark: 4500 records in local store. None of them changed.
Before (1000+):
After (<50):