Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WriteableCollection.delete() should use IDBObjectStore.delete() #208

Closed
nifgraup opened this issue Mar 23, 2016 · 12 comments · Fixed by #210
Closed

WriteableCollection.delete() should use IDBObjectStore.delete() #208

nifgraup opened this issue Mar 23, 2016 · 12 comments · Fixed by #210

Comments

@nifgraup
Copy link

speeds up operations like db.table.where(id).between(1, 100000).delete()

@dfahlander
Copy link
Collaborator

Interesting. How did you get that info? Are there any jsperf tests that confirms that?

@dfahlander
Copy link
Collaborator

I suppose doing the deletes after the keycursor has fetched all keys (or doing fetches in chunks) would be best. But I actually had the same theory regarding updates and started testing it but it turned out to be slower than cursor.update() so I threw away that idea. It would really be interesting with a jsperf test for this.

@nifgraup
Copy link
Author

I'm having problems with jsperf.com so here is a snippet:

const db = new Dexie("database");
db.version(1).stores({
  storage: "id",
});

const MAX = 100000;
var data = [];
for(let i = 0; i<MAX; i++) {
  data.push({id: i});
}
const insertData = function() {
  return db.transaction("rw", [db.storage], () => {
    return Promise.all(data.map(d => db.storage.put(d)));
  })
}
console.log(Date.now(), "Inserting data:");
insertData()
.then(() => console.log(Date.now(), "done. Deleting data with dexie:"))
.then(() => db.storage.where("id").between(100, MAX-100).delete())
.then(() => console.log(Date.now(), "done. Insert data again:"))
.then(() => insertData())
.then(() => console.log(Date.now(), "done. Deleting data directly with IndexedDB:"))
.then(() => new Promise((resolve, reject) => {
  const idbtrans = db.backendDB().transaction(["storage"], "readwrite");
  const req = idbtrans.objectStore("storage")
  .delete(IDBKeyRange.bound(100, MAX-100));
  req.onsuccess = function(e) {
    if (e.type === "success") {
      resolve();
    } else {
      reject();
    }
  };
}))
.then(() => console.log(Date.now(), "done."));

@dfahlander
Copy link
Collaborator

Thanks! It had slipped my eyes that IDbObjectStore.delete() accepts key ranges! Knowing that fact, it is obvious that it is the better approach. Thanks for sharing. I'll see what I could do to use this

dfahlander added a commit that referenced this issue Mar 23, 2016
…bleCollection where applicable:

* WhereClause must use primary key
* No algoritm applied, such as ignoreCase, anyOf, noneOf, inAnyRange, etc.
* No filter applied
* or() not used

closes #208 WriteableCollection.delete() should use IDBObjectStore.delete()
@dfahlander
Copy link
Collaborator

I put your sample in a jsfidde: https://jsfiddle.net/dfahlander/L7dL8xrv/
Will run it again after releasing 1.3.5.

@dfahlander
Copy link
Collaborator

The fiddle shows that the new beta is much faster doing deletes. However, since it needs to count the KeyRange first, it is a little slower than raw IDBObjectStore.delete().

The fiddle works perfectly with chrome, opera and FF, but on Edge it doesnt seem to handle the large number of deletes. On Edge browser, it will hang on "Deleting data with dexie...", no matter if using Dexie 1.3.4 or 1.3.5-beta.2. But if using the latter, the database will go into a locked state for hours, which is not the case with 1.3.4.

I suppose Edge's indexedDB implementation got too much to do when calling IDBObjectStore.delete(keyRange) but when doing IDBCursor.delete(), the flow will cancel when reloading the page whilst IDBObjectStore.delete(keyRange) will continue in the background for ages. However, when using smaller number of items, Edge browser benefit from the change. It's just when the number of items are large that this hang happens.

I'm thinking of performing the delete operation in chunks instead, of max 5000 items at a time, because then we would give Edge/IE some cancellation points. It's not nice when the database is locked for hours and it actually got worse and worse. After trying the sample again and agina, Edge browser stopped launching and I had to reboot my machine.

@nifgraup
Copy link
Author

Does Edge behave better if you delete inside a transaction?

@dfahlander
Copy link
Collaborator

It's always within a transaction in indexedDB. Don't think encapsulating the put operations in the same transaction would do any better and that would be a non realistic use case. I will check for an alternate solution for ie/edge. Or just don't optimize delete() there at all. Hope to get something out this week.

@nifgraup
Copy link
Author

FIY: if I don't encapsulate the put operations in the same transaction on Chrome/Linux, the CPU goes to the roof, and if I wait long enough, I get a file system error and it remounts read-only. It sounds like something as catastrophic is going on when mass-deleting on Edge.

@dfahlander
Copy link
Collaborator

Yeah, that's because there will be 100 000 parallel transactions of you don't, since indexedDB requires a transaction and Dexie will launch one for each operation of you don't use db.transaction with Dexie.

@dfahlander
Copy link
Collaborator

The Edge bug is really reproducable on different virtual machines. I am now working on another solution that use bulk deletion in chunks instead. Turns out to also be much more efficient than the current one but not as efficient as the keyrange one. But this solution works also for any query such as equalsIgnoreCase on an index. Not just primaryKey ranges. I'll release another beta when done.

The edge bug is reported to Microsoft: https://connect.microsoft.com/IE/feedback/details/2530333/nasty-bug-in-indexeddb-hangs-database-service-and-makes-system-instable

I published a gist about it as well https://gist.github.com/dfahlander/5a39328f029de18222cf2125d56c38f7

@timbru31
Copy link
Contributor

timbru31 commented Dec 8, 2017

.bulkPut() and .bulkDelete() is still incredibly slow for us. Safari (11.0.2) takes 6 minutes on a MacBook Pro to delete 4000 entries, while Chrome is around 5-6s seconds.
This applies only to items that are already present, a .bulkAdd() is fast in Safari, too.

iPads are even worse with up to 30 minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants