Support cas to return nodes #133
Conversation
|
@mafintosh Ready! |
LuKks
changed the title
|
@mafintosh Ready! |
index.js
Outdated
| @@ -109,7 +109,10 @@ class TreeNode { | |||
| c = b4a.compare(key.value, await this.getKey(mid)) | |||
|
|
|||
| if (c === 0) { | |||
| if (cas && !(await cas((await this.getKeyNode(mid)).final(encoding), node))) return true | |||
| const older = (await this.getKeyNode(mid)).final(encoding) | |||
There was a problem hiding this comment.
I think this change makes inserts slower for the case where no cas is defined, because it now always runs getKeyNode(), which I believe runs in logarithmic time. I'm not convinced that's a good trade-off to make for this feature--can't we special case the cas-case and keep the old code when no cas is specified?
There was a problem hiding this comment.
yea this needs to only run on cas
There was a problem hiding this comment.
just make an if
if (cas) {
const older = ....
const swap = await cas(older, node)
if (!swap || swap === older) return true
}There was a problem hiding this comment.
if it swaps you need to update the value you insert also
There was a problem hiding this comment.
if it swaps you need to update the value you insert also
Where am I missing this?
index.js
Outdated
| @@ -793,7 +802,7 @@ class Batch { | |||
| } | |||
| } | |||
|
|
|||
| return this._append(root, seq, key, value) | |||
| return this._append(root, seq, key, enc(encoding.value, newNode.value)) | |||
There was a problem hiding this comment.
move the encoded value to const value = enc(....) before this line
There was a problem hiding this comment.
value exists as arg so I named it valueEncoded
No description provided.