From 88971b4e69b7be080147e0b4f17d6bfe51904a3f Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Mon, 21 Mar 2016 21:00:28 +0100 Subject: [PATCH 1/4] fixed several issues of the gc. I.e. the gc sometimes did not collect the whole subtree when deleting an operation --- src/Database.js | 40 ++++++++++++----- src/SpecHelper.js | 4 +- src/Transaction.js | 110 ++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 129 insertions(+), 25 deletions(-) diff --git a/src/Database.js b/src/Database.js index 5b2daf50..0b55cfec 100644 --- a/src/Database.js +++ b/src/Database.js @@ -110,6 +110,18 @@ module.exports = function (Y /* :any */) { garbageCollect() } } + emptyGarbageCollector () { + return new Promise (resolve => { + var check = () => { + if (this.gc1.length > 0 || this.gc2.length > 0) { + this.garbageCollect().then(check) + } else { + resolve() + } + } + setTimeout(check, 0) + }) + } addToDebug () { if (typeof YConcurrency_TestingMode !== 'undefined') { var command /* :string */ = Array.prototype.map.call(arguments, function (s) { @@ -339,15 +351,7 @@ module.exports = function (Y /* :any */) { } } else { // increase SS - var o = op - var state = yield* transaction.getState(op.id[0]) - while (o != null && o.id[1] === state.clock && op.id[0] === o.id[0]) { - // either its a new operation (1. case), or it is an operation that was deleted, but is not yet in the OS - state.clock++ - yield* transaction.checkDeleteStoreForState(state) - o = yield* transaction.os.findNext(o.id) - } - yield* transaction.setState(state) + yield* transaction.updateState(op.id[0]) // notify whenOperation listeners (by id) var sid = JSON.stringify(op.id) @@ -364,6 +368,15 @@ module.exports = function (Y /* :any */) { } var t = this.initializedTypes[JSON.stringify(op.parent)] + // if parent is deleted, mark as gc'd and return + if (op.parent != null) { + var parentIsDeleted = yield* transaction.isDeleted(op.parent) + if (parentIsDeleted) { + yield* transaction.deleteList(op.id) + return + } + } + // Delete if DS says this is actually deleted var opIsDeleted = yield* transaction.isDeleted(op.id) if (!op.deleted && opIsDeleted) { @@ -375,8 +388,13 @@ module.exports = function (Y /* :any */) { } // notify parent, if it was instanciated as a custom type - if (t != null && !opIsDeleted) { - yield* t._changed(transaction, Y.utils.copyObject(op)) + if (t != null) { + let o = Y.utils.copyObject(op) + if (opIsDeleted && !o.deleted) { + // op did not reflect the created delete op (happens when not using y-memory) + o.deleted = true + } + yield* t._changed(transaction, o) } } } diff --git a/src/SpecHelper.js b/src/SpecHelper.js index d5daca9d..72435424 100644 --- a/src/SpecHelper.js +++ b/src/SpecHelper.js @@ -139,9 +139,9 @@ g.applyRandomTransactionsWithGC = async(function * applyRandomTransactions (user g.garbageCollectAllUsers = async(function * garbageCollectAllUsers (users) { // gc two times because of the two gc phases (really collect everything) + yield wait(100) for (var i in users) { - yield users[i].db.garbageCollect() - yield users[i].db.garbageCollect() + yield users[i].db.emptyGarbageCollector() } }) diff --git a/src/Transaction.js b/src/Transaction.js index ee1556cf..b8ef6a46 100644 --- a/src/Transaction.js +++ b/src/Transaction.js @@ -154,17 +154,32 @@ module.exports = function (Y/* :any */) { } * deleteList (start) { - if (this.store.y.connector.isSynced) { - while (start != null && this.store.y.connector.isSynced) { - start = yield* this.getOperation(start) + while (start != null) { + start = yield* this.getOperation(start) + if (start.gc) { + break + } else { start.gc = true + start.deleted = true yield* this.setOperation(start) - // TODO: will always reset the parent.. - this.store.gc1.push(start.id) + yield* this.markDeleted(start.id, 1) + if (start.opContent != null) { + yield* this.deleteOperation(start.opContent) + /* + yield* this.deleteOperation(start.opContent) + var opContent = yield* this.getOperation(start.opContent) + opContent.gc = true + yield* this.setOperation(opContent) + if (this.store.y.connector.isSynced) { + this.store.gc1.push(opContent.id) + } + */ + } + if (this.store.y.connector.isSynced){ + this.store.gc1.push(start.id) + } start = start.right } - } else { - // TODO: when not possible??? do later in (gcWhenSynced) } } @@ -199,19 +214,24 @@ module.exports = function (Y/* :any */) { if (target.start != null) { // TODO: don't do it like this .. -.- yield* this.deleteList(target.start) - yield* this.deleteList(target.id) + // yield* this.deleteList(target.id) -- do not gc itself because this may still get referenced } if (target.map != null) { for (var name in target.map) { yield* this.deleteList(target.map[name]) } // TODO: here to.. (see above) - yield* this.deleteList(target.id) + // yield* this.deleteList(target.id) -- see above } if (target.opContent != null) { yield* this.deleteOperation(target.opContent) // target.opContent = null } + if (target.requires != null) { + for (var i = 0; i < target.requires.length; i++) { + yield* this.deleteOperation(target.requires[i]) + } + } } var left if (target.left != null) { @@ -377,9 +397,40 @@ module.exports = function (Y/* :any */) { */ * garbageCollectAfterSync () { yield* this.os.iterate(this, null, null, function * (op) { - if (op.deleted && op.left != null) { - var left = yield* this.getOperation(op.left) - this.store.addToGarbageCollector(op, left) + if (op.gc) { + this.store.gc1.push(op.id) + } else { + if (op.parent != null) { + var parentDeleted = yield* this.isDeleted(op.parent) + if (parentDeleted) { + op.gc = true + if (!op.deleted) { + yield* this.markDeleted(op.id, 1) + op.deleted = true + if (op.opContent != null) { + yield* this.deleteOperation(op.opContent) + /* + var opContent = yield* this.getOperation(op.opContent) + opContent.gc = true + yield* this.setOperation(opContent) + this.store.gc1.push(opContent.id) + */ + } + if (op.requires != null) { + for (var i = 0; i < op.requires.length; i++) { + yield* this.deleteOperation(op.requires[i]) + } + } + } + yield* this.setOperation(op) + this.store.gc1.push(op.id) + return + } + } + if (op.deleted && op.left != null) { + var left = yield* this.getOperation(op.left) + this.store.addToGarbageCollector(op, left) + } } }) } @@ -404,6 +455,29 @@ module.exports = function (Y/* :any */) { o = yield* this.getOperation(id) } */ + + var deps = [] + if (o.opContent != null) { + deps.push(o.opContent) + } + if (o.requires != null) { + deps = deps.concat(o.requires) + } + for (var i = 0; i < deps.length; i++) { + var dep = yield* this.getOperation(deps[i]) + if (dep != null) { + if (!dep.deleted) { + yield* this.deleteOperation(dep.id) + dep = yield* this.getOperation(dep.id) + } + dep.gc = true + yield* this.setOperation(dep) + this.store.gc1.push(dep.id) + } else { + yield* this.markGarbageCollected(deps[i], 1) + yield* this.updateState(deps[i][0]) // TODO: unneccessary? + } + } // remove gc'd op from the left op, if it exists if (o.left != null) { @@ -535,6 +609,18 @@ module.exports = function (Y/* :any */) { state.clock = Math.max(state.clock, n.id[1] + n.len) } } + * updateState (user) { + var state = yield* this.getState(user) + yield* this.checkDeleteStoreForState(state) + var o = yield* this.getOperation([user, state.clock]) + while (o != null && o.id[1] === state.clock && user === o.id[0]) { + // either its a new operation (1. case), or it is an operation that was deleted, but is not yet in the OS + state.clock++ + yield* this.checkDeleteStoreForState(state) + o = yield* this.os.findNext(o.id) + } + yield* this.setState(state) + } /* apply a delete set in order to get the state of the supplied ds From 83a42271ad03901f18f16515c6f9e1953c2a9226 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Wed, 23 Mar 2016 14:33:51 +0100 Subject: [PATCH 2/4] fix remaining memory leaks --- README.md | 10 +++++++++- package.json | 2 +- src/Connector.js | 10 +++++++--- src/SpecHelper.js | 4 ++++ src/Transaction.js | 33 +++++++++++---------------------- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 7c79014d..44bfba75 100644 --- a/README.md +++ b/README.md @@ -177,8 +177,16 @@ If you want to see an issue fixed, please subscribe to the thread (or remind me ## Changelog +### 10.0.0 + +* Support for more complex types (a type can be a composition of several types) +* Fixes several memory leaks + ### 9.0.0 -There were several rolling updates from 0.6 to 0.8. We'll now follow the semver versioning scheme. This is all what this jump from 0.8 to 9.0 is about. +There were several rolling updates from 0.6 to 0.8. We consider Yjs stable since a long time, +and intend to continue stable releases. From this release forward y-* modules will implement peer-dependencies for npm, and dependencies for bower. +Furthermore, incompatible yjs instances will now throw errors when syncing - this feature was influenced by #48. The versioning jump was influenced by react (see [here](https://facebook.github.io/react/blog/2016/02/19/new-versioning-scheme.html)) + ### 0.6.0 This is a complete rewrite of the 0.5 version of Yjs. Since Yjs 0.6.0 it is possible to work asynchronously on a persistent database, which enables offline support. diff --git a/package.json b/package.json index 919ad4b6..f64c4085 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "yjs", - "version": "9.1.0", + "version": "10.0.0", "description": "A framework for real-time p2p shared editing on arbitrary complex data types", "main": "./src/y.js", "scripts": { diff --git a/src/Connector.js b/src/Connector.js index 00c74e89..a8fb9a40 100644 --- a/src/Connector.js +++ b/src/Connector.js @@ -19,6 +19,8 @@ module.exports = function (Y/* :any */) { userId: UserId; send: Function; broadcast: Function; + broadcastOpBuffer: Array; + protocolVersion: number; */ /* opts contains the following information: @@ -51,7 +53,7 @@ module.exports = function (Y/* :any */) { this.broadcastedHB = false this.syncStep2 = Promise.resolve() this.broadcastOpBuffer = [] - this.protocolVersion = 8 + this.protocolVersion = 10 } reconnect () { } @@ -150,7 +152,8 @@ module.exports = function (Y/* :any */) { conn.send(syncUser, { type: 'sync step 1', stateSet: stateSet, - deleteSet: deleteSet + deleteSet: deleteSet, + protocolVersion: conn.protocolVersion }) }) } else { @@ -234,7 +237,8 @@ module.exports = function (Y/* :any */) { type: 'sync step 2', os: ops, stateSet: currentStateSet, - deleteSet: ds + deleteSet: ds, + protocolVersion: this.protocolVersion }) if (this.forwardToSyncingClients) { conn.syncingClients.push(sender) diff --git a/src/SpecHelper.js b/src/SpecHelper.js index 72435424..02f4e32d 100644 --- a/src/SpecHelper.js +++ b/src/SpecHelper.js @@ -182,6 +182,10 @@ g.compareAllUsers = async(function * compareAllUsers (users) { for (var uid = 0; uid < users.length; uid++) { var u = users[uid] u.db.requestTransaction(function * () { + var sv = yield* this.getStateVector() + for (var s of sv) { + yield* this.updateState(s.user) + } // compare deleted ops against deleteStore yield* this.os.iterate(this, null, null, function * (o) { if (o.deleted === true) { diff --git a/src/Transaction.js b/src/Transaction.js index b8ef6a46..3a5344a7 100644 --- a/src/Transaction.js +++ b/src/Transaction.js @@ -156,30 +156,19 @@ module.exports = function (Y/* :any */) { * deleteList (start) { while (start != null) { start = yield* this.getOperation(start) - if (start.gc) { - break - } else { + if (!start.gc) { start.gc = true start.deleted = true yield* this.setOperation(start) yield* this.markDeleted(start.id, 1) if (start.opContent != null) { yield* this.deleteOperation(start.opContent) - /* - yield* this.deleteOperation(start.opContent) - var opContent = yield* this.getOperation(start.opContent) - opContent.gc = true - yield* this.setOperation(opContent) - if (this.store.y.connector.isSynced) { - this.store.gc1.push(opContent.id) - } - */ } if (this.store.y.connector.isSynced){ this.store.gc1.push(start.id) } - start = start.right } + start = start.right } } @@ -561,15 +550,15 @@ module.exports = function (Y/* :any */) { // so we have to set right here yield* this.setOperation(right) } - // o may originate in another operation. - // Since o is deleted, we have to reset o.origin's `originOf` property - if (o.origin != null) { - var origin = yield* this.getOperation(o.origin) - origin.originOf = origin.originOf.filter(function (_id) { - return !Y.utils.compareIds(id, _id) - }) - yield* this.setOperation(origin) - } + } + // o may originate in another operation. + // Since o is deleted, we have to reset o.origin's `originOf` property + if (o.origin != null) { + var origin = yield* this.getOperation(o.origin) + origin.originOf = origin.originOf.filter(function (_id) { + return !Y.utils.compareIds(id, _id) + }) + yield* this.setOperation(origin) } var parent if (o.parent != null){ From d4ee8af772a550b778b2cf04377a729fcd4c8c09 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Wed, 23 Mar 2016 14:41:08 +0100 Subject: [PATCH 3/4] Release 10.0.1 --- dist | 2 +- src/Database.js | 2 +- src/Transaction.js | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dist b/dist index 5cfe2096..ffe0ec5a 160000 --- a/dist +++ b/dist @@ -1 +1 @@ -Subproject commit 5cfe209688c31ae496b6c86db0dc5adbf15b8046 +Subproject commit ffe0ec5a38df797b55c3b5bc64c9fd791cb82299 diff --git a/src/Database.js b/src/Database.js index 0b55cfec..650a8f4f 100644 --- a/src/Database.js +++ b/src/Database.js @@ -111,7 +111,7 @@ module.exports = function (Y /* :any */) { } } emptyGarbageCollector () { - return new Promise (resolve => { + return new Promise(resolve => { var check = () => { if (this.gc1.length > 0 || this.gc2.length > 0) { this.garbageCollect().then(check) diff --git a/src/Transaction.js b/src/Transaction.js index 3a5344a7..3ac10c24 100644 --- a/src/Transaction.js +++ b/src/Transaction.js @@ -164,7 +164,7 @@ module.exports = function (Y/* :any */) { if (start.opContent != null) { yield* this.deleteOperation(start.opContent) } - if (this.store.y.connector.isSynced){ + if (this.store.y.connector.isSynced) { this.store.gc1.push(start.id) } } @@ -415,7 +415,7 @@ module.exports = function (Y/* :any */) { this.store.gc1.push(op.id) return } - } + } if (op.deleted && op.left != null) { var left = yield* this.getOperation(op.left) this.store.addToGarbageCollector(op, left) @@ -444,7 +444,7 @@ module.exports = function (Y/* :any */) { o = yield* this.getOperation(id) } */ - + var deps = [] if (o.opContent != null) { deps.push(o.opContent) @@ -561,7 +561,7 @@ module.exports = function (Y/* :any */) { yield* this.setOperation(origin) } var parent - if (o.parent != null){ + if (o.parent != null) { parent = yield* this.getOperation(o.parent) } // remove gc'd op from parent, if it exists From 24f8616386ef3b3ab4d7e61ca9a77c6c27598368 Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Wed, 23 Mar 2016 14:42:26 +0100 Subject: [PATCH 4/4] Release 10.0.2 --- dist | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dist b/dist index ffe0ec5a..65808156 160000 --- a/dist +++ b/dist @@ -1 +1 @@ -Subproject commit ffe0ec5a38df797b55c3b5bc64c9fd791cb82299 +Subproject commit 658081566b604ed552aeba8960cedbc9e515b946 diff --git a/package.json b/package.json index f64c4085..7a3fc48d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "yjs", - "version": "10.0.0", + "version": "10.0.2", "description": "A framework for real-time p2p shared editing on arbitrary complex data types", "main": "./src/y.js", "scripts": {