Use dirty state rather events to avoid O(N^2) behaviour

This commit is contained in:
James Allen 2017-02-23 11:27:19 +01:00
parent e83c7dfe05
commit f4bbd8ea10
3 changed files with 124 additions and 107 deletions

View file

@ -353,11 +353,12 @@ define [
@ranges.applyOp op, { user_id: track_changes_as } @ranges.applyOp op, { user_id: track_changes_as }
if old_id_seed? if old_id_seed?
@ranges.setIdSeed(old_id_seed) @ranges.setIdSeed(old_id_seed)
@emit "ranges:dirty"
_catchUpRanges: (changes = [], comments = []) -> _catchUpRanges: (changes = [], comments = []) ->
# We've just been given the current server's ranges, but need to apply any local ops we have. # We've just been given the current server's ranges, but need to apply any local ops we have.
# Reset to the server state then apply our local ops again. # Reset to the server state then apply our local ops again.
@ranges.emit "clear" @emit "ranges:clear"
@ranges.changes = changes @ranges.changes = changes
@ranges.comments = comments @ranges.comments = comments
@ranges.track_changes = @doc.track_changes @ranges.track_changes = @doc.track_changes
@ -367,4 +368,4 @@ define [
for op in @doc.getPendingOp() or [] for op in @doc.getPendingOp() or []
@ranges.setIdSeed(@doc.track_changes_id_seeds.pending) @ranges.setIdSeed(@doc.track_changes_id_seeds.pending)
@ranges.applyOp(op, { user_id: @track_changes_as }) @ranges.applyOp(op, { user_id: @track_changes_as })
@ranges.emit "redraw" @emit "ranges:redraw"

View file

@ -14,11 +14,11 @@ define [
return if !track_changes? return if !track_changes?
@setTrackChanges(track_changes) @setTrackChanges(track_changes)
@$scope.$watch "sharejsDoc", (doc) => @$scope.$watch "sharejsDoc", (doc, oldDoc) =>
return if !doc? return if !doc?
@disconnectFromRangesTracker() if oldDoc?
@rangesTracker = doc.ranges @disconnectFromDoc(oldDoc)
@connectToRangesTracker() @connectToDoc(doc)
@$scope.$on "comment:add", (e, thread_id, offset, length) => @$scope.$on "comment:add", (e, thread_id, offset, length) =>
@addCommentToSelection(thread_id, offset, length) @addCommentToSelection(thread_id, offset, length)
@ -36,10 +36,10 @@ define [
@removeCommentId(comment_id) @removeCommentId(comment_id)
@$scope.$on "comment:resolve_threads", (e, thread_ids) => @$scope.$on "comment:resolve_threads", (e, thread_ids) =>
@resolveCommentByThreadIds(thread_ids) @hideCommentsByThreadIds(thread_ids)
@$scope.$on "comment:unresolve_thread", (e, thread_id) => @$scope.$on "comment:unresolve_thread", (e, thread_id) =>
@unresolveCommentByThreadId(thread_id) @showCommentByThreadId(thread_id)
@$scope.$on "review-panel:recalculate-screen-positions", () => @$scope.$on "review-panel:recalculate-screen-positions", () =>
@recalculateReviewEntriesScreenPositions() @recalculateReviewEntriesScreenPositions()
@ -92,18 +92,11 @@ define [
else else
unbindFromAce() unbindFromAce()
disconnectFromRangesTracker: () -> disconnectFromDoc: (doc) ->
@changeIdToMarkerIdMap = {} @changeIdToMarkerIdMap = {}
doc.off "ranges:clear"
if @rangesTracker? doc.off "ranges:redraw"
@rangesTracker.off "insert:added" doc.off "ranges:dirty"
@rangesTracker.off "insert:removed"
@rangesTracker.off "delete:added"
@rangesTracker.off "delete:removed"
@rangesTracker.off "changes:moved"
@rangesTracker.off "comment:added"
@rangesTracker.off "comment:moved"
@rangesTracker.off "comment:removed"
setTrackChanges: (value) -> setTrackChanges: (value) ->
if value if value
@ -111,56 +104,15 @@ define [
else else
@$scope.sharejsDoc?.track_changes_as = null @$scope.sharejsDoc?.track_changes_as = null
connectToRangesTracker: () -> connectToDoc: (doc) ->
@rangesTracker = doc.ranges
@setTrackChanges(@$scope.trackChanges) @setTrackChanges(@$scope.trackChanges)
# Add a timeout because on remote ops, we get these notifications before doc.on "ranges:dirty", () =>
# ace has updated @updateAnnotations()
@rangesTracker.on "insert:added", (change) => doc.on "ranges:clear", () =>
sl_console.log "[insert:added]", change
setTimeout () =>
@_onInsertAdded(change)
@broadcastChange()
@rangesTracker.on "insert:removed", (change) =>
sl_console.log "[insert:removed]", change
setTimeout () =>
@_onInsertRemoved(change)
@broadcastChange()
@rangesTracker.on "delete:added", (change) =>
sl_console.log "[delete:added]", change
setTimeout () =>
@_onDeleteAdded(change)
@broadcastChange()
@rangesTracker.on "delete:removed", (change) =>
sl_console.log "[delete:removed]", change
setTimeout () =>
@_onDeleteRemoved(change)
@broadcastChange()
@rangesTracker.on "changes:moved", (changes) =>
sl_console.log "[changes:moved]", changes
setTimeout () =>
@_onChangesMoved(changes)
@broadcastChange()
@rangesTracker.on "comment:added", (comment) =>
sl_console.log "[comment:added]", comment
setTimeout () =>
@_onCommentAdded(comment)
@broadcastChange()
@rangesTracker.on "comment:moved", (comment) =>
sl_console.log "[comment:moved]", comment
setTimeout () =>
@_onCommentMoved(comment)
@broadcastChange()
@rangesTracker.on "comment:removed", (comment) =>
sl_console.log "[comment:removed]", comment
setTimeout () =>
@_onCommentRemoved(comment)
@broadcastChange()
@rangesTracker.on "clear", () =>
@clearAnnotations() @clearAnnotations()
@rangesTracker.on "redraw", () => doc.on "ranges:redraw", () =>
@redrawAnnotations() @redrawAnnotations()
clearAnnotations: () -> clearAnnotations: () ->
@ -181,6 +133,55 @@ define [
@_onCommentAdded(comment) @_onCommentAdded(comment)
@broadcastChange() @broadcastChange()
_doneUpdateThisLoop: false
_pendingUpdates: false
updateAnnotations: () ->
# Doc updates with multiple ops, like search/replace or block comments
# will call this with every individual op in a single event loop. So only
# do the first this loop, then schedule an update for the next loop for the rest.
if !@_doneUpdateThisLoop
@_doUpdateAnnotations()
@_doneUpdateThisLoop = true
setTimeout () =>
if @_pendingUpdates
@_doUpdateAnnotations()
@_doneUpdateThisLoop = false
@_pendingUpdates = false
else
@_pendingUpdates = true
_doUpdateAnnotations: () ->
dirty = @rangesTracker.getDirtyState()
updateMarkers = false
for id, change of dirty.change.added
if change.op.i?
@_onInsertAdded(change)
else if change.op.d?
@_onDeleteAdded(change)
for id, change of dirty.change.removed
if change.op.i?
@_onInsertRemoved(change)
else if change.op.d?
@_onDeleteRemoved(change)
for id, change of dirty.change.moved
updateMarkers = true
@_onChangeMoved(change)
for id, comment of dirty.comment.added
@_onCommentAdded(comment)
for id, comment of dirty.comment.removed
@_onCommentRemoved(comment)
for id, comment of dirty.comment.moved
updateMarkers = true
@_onCommentMoved(comment)
@rangesTracker.resetDirtyState()
if updateMarkers
@editor.renderer.updateBackMarkers()
@broadcastChange()
addComment: (offset, content, thread_id) -> addComment: (offset, content, thread_id) ->
op = { c: content, p: offset, t: thread_id } op = { c: content, p: offset, t: thread_id }
@ -200,6 +201,7 @@ define [
acceptChangeId: (change_id) -> acceptChangeId: (change_id) ->
@rangesTracker.removeChangeId(change_id) @rangesTracker.removeChangeId(change_id)
@updateAnnotations()
rejectChangeId: (change_id) -> rejectChangeId: (change_id) ->
change = @rangesTracker.getChange(change_id) change = @rangesTracker.getChange(change_id)
@ -221,8 +223,9 @@ define [
removeCommentId: (comment_id) -> removeCommentId: (comment_id) ->
@rangesTracker.removeCommentId(comment_id) @rangesTracker.removeCommentId(comment_id)
@updateAnnotations()
resolveCommentByThreadIds: (thread_ids) -> hideCommentsByThreadIds: (thread_ids) ->
resolve_ids = {} resolve_ids = {}
for id in thread_ids for id in thread_ids
resolve_ids[id] = true resolve_ids[id] = true
@ -231,7 +234,7 @@ define [
@_onCommentRemoved(comment) @_onCommentRemoved(comment)
@broadcastChange() @broadcastChange()
unresolveCommentByThreadId: (thread_id) -> showCommentByThreadId: (thread_id) ->
for comment in @rangesTracker?.comments or [] for comment in @rangesTracker?.comments or []
if comment.op.t == thread_id if comment.op.t == thread_id
@_onCommentAdded(comment) @_onCommentAdded(comment)
@ -421,23 +424,18 @@ define [
lines = @editor.getSession().getDocument().getAllLines() lines = @editor.getSession().getDocument().getAllLines()
return AceShareJsCodec.shareJsOffsetToAcePosition(offset, lines) return AceShareJsCodec.shareJsOffsetToAcePosition(offset, lines)
_onChangesMoved: (changes) -> _onChangeMoved: (change) ->
# TODO: PERFORMANCE: Only run through the Ace lines once, and calculate all start = @_shareJsOffsetToAcePosition(change.op.p)
# change positions as we go. if change.op.i?
for change in changes end = @_shareJsOffsetToAcePosition(change.op.p + change.op.i.length)
start = @_shareJsOffsetToAcePosition(change.op.p) else
if change.op.i? end = start
end = @_shareJsOffsetToAcePosition(change.op.p + change.op.i.length) @_updateMarker(change.id, start, end)
else
end = start
@_updateMarker(change.id, start, end)
@editor.renderer.updateBackMarkers()
_onCommentMoved: (comment) -> _onCommentMoved: (comment) ->
start = @_shareJsOffsetToAcePosition(comment.op.p) start = @_shareJsOffsetToAcePosition(comment.op.p)
end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length) end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length)
@_updateMarker(comment.id, start, end) @_updateMarker(comment.id, start, end)
@editor.renderer.updateBackMarkers()
_updateMarker: (change_id, start, end) -> _updateMarker: (change_id, start, end) ->
return if !@changeIdToMarkerIdMap[change_id]? return if !@changeIdToMarkerIdMap[change_id]?

View file

@ -1,5 +1,5 @@
load = (EventEmitter) -> load = () ->
class RangesTracker extends EventEmitter class RangesTracker
# The purpose of this class is to track a set of inserts and deletes to a document, like # The purpose of this class is to track a set of inserts and deletes to a document, like
# track changes in Word. We store these as a set of ShareJs style ranges: # track changes in Word. We store these as a set of ShareJs style ranges:
# {i: "foo", p: 42} # Insert 'foo' at offset 42 # {i: "foo", p: 42} # Insert 'foo' at offset 42
@ -36,6 +36,7 @@ load = (EventEmitter) ->
# middle of a previous insert by the first user, the original insert will be split into two. # middle of a previous insert by the first user, the original insert will be split into two.
constructor: (@changes = [], @comments = []) -> constructor: (@changes = [], @comments = []) ->
@setIdSeed(RangesTracker.generateIdSeed()) @setIdSeed(RangesTracker.generateIdSeed())
@resetDirtyState()
getIdSeed: () -> getIdSeed: () ->
return @id_seed return @id_seed
@ -75,7 +76,7 @@ load = (EventEmitter) ->
comment = @getComment(comment_id) comment = @getComment(comment_id)
return if !comment? return if !comment?
@comments = @comments.filter (c) -> c.id != comment_id @comments = @comments.filter (c) -> c.id != comment_id
@emit "comment:removed", comment @_markAsDirty comment, "comment", "removed"
getChange: (change_id) -> getChange: (change_id) ->
change = null change = null
@ -103,7 +104,11 @@ load = (EventEmitter) ->
@addComment(op, metadata) @addComment(op, metadata)
else else
throw new Error("unknown op type") throw new Error("unknown op type")
applyOps: (ops, metadata = {}) ->
for op in ops
@applyOp(op, metadata)
addComment: (op, metadata) -> addComment: (op, metadata) ->
# TODO: Don't allow overlapping comments? # TODO: Don't allow overlapping comments?
@comments.push comment = { @comments.push comment = {
@ -114,18 +119,18 @@ load = (EventEmitter) ->
t: op.t t: op.t
metadata metadata
} }
@emit "comment:added", comment @_markAsDirty comment, "comment", "added"
return comment return comment
applyInsertToComments: (op) -> applyInsertToComments: (op) ->
for comment in @comments for comment in @comments
if op.p <= comment.op.p if op.p <= comment.op.p
comment.op.p += op.i.length comment.op.p += op.i.length
@emit "comment:moved", comment @_markAsDirty comment, "comment", "moved"
else if op.p < comment.op.p + comment.op.c.length else if op.p < comment.op.p + comment.op.c.length
offset = op.p - comment.op.p offset = op.p - comment.op.p
comment.op.c = comment.op.c[0..(offset-1)] + op.i + comment.op.c[offset...] comment.op.c = comment.op.c[0..(offset-1)] + op.i + comment.op.c[offset...]
@emit "comment:moved", comment @_markAsDirty comment, "comment", "moved"
applyDeleteToComments: (op) -> applyDeleteToComments: (op) ->
op_start = op.p op_start = op.p
@ -138,7 +143,7 @@ load = (EventEmitter) ->
if op_end <= comment_start if op_end <= comment_start
# delete is fully before comment # delete is fully before comment
comment.op.p -= op_length comment.op.p -= op_length
@emit "comment:moved", comment @_markAsDirty comment, "comment", "moved"
else if op_start >= comment_end else if op_start >= comment_end
# delete is fully after comment, nothing to do # delete is fully after comment, nothing to do
else else
@ -161,7 +166,7 @@ load = (EventEmitter) ->
comment.op.p = Math.min(comment_start, op_start) comment.op.p = Math.min(comment_start, op_start)
comment.op.c = remaining_before + remaining_after comment.op.c = remaining_before + remaining_after
@emit "comment:moved", comment @_markAsDirty comment, "comment", "moved"
applyInsertToChanges: (op, metadata) -> applyInsertToChanges: (op, metadata) ->
op_start = op.p op_start = op.p
@ -206,12 +211,12 @@ load = (EventEmitter) ->
# If this is an insert op at the end of an existing insert with a delete following, and it cancels out the following # If this is an insert op at the end of an existing insert with a delete following, and it cancels out the following
# delete then we shouldn't append it to this insert, but instead only cancel the following delete. # delete then we shouldn't append it to this insert, but instead only cancel the following delete.
# E.g. # E.g.
# foo|<--- about to insert 'b' here # foo|<--- about to insert 'bar' here
# inserted 'foo' --^ ^-- deleted 'bar' # inserted 'foo' --^ ^-- deleted 'bar'
# should become just 'foo' not 'foob' (with the delete marker becoming just 'ar'), . # should become just 'foo' not 'foobar' (with the delete marker disappearing), .
next_change = @changes[i+1] next_change = @changes[i+1]
is_op_adjacent_to_next_delete = next_change? and next_change.op.d? and op.p == change_end and next_change.op.p == op.p is_op_adjacent_to_next_delete = next_change? and next_change.op.d? and op.p == change_end and next_change.op.p == op.p
will_op_cancel_next_delete = is_op_adjacent_to_next_delete and next_change.op.d.slice(0, op.i.length) == op.i will_op_cancel_next_delete = is_op_adjacent_to_next_delete and next_change.op.d == op.i
# If there is a delete at the start of the insert, and we're inserting # If there is a delete at the start of the insert, and we're inserting
# at the start, we SHOULDN'T merge since the delete acts as a partition. # at the start, we SHOULDN'T merge since the delete acts as a partition.
@ -281,8 +286,8 @@ load = (EventEmitter) ->
for change in remove_changes for change in remove_changes
@_removeChange change @_removeChange change
if moved_changes.length > 0 for change in moved_changes
@emit "changes:moved", moved_changes @_markAsDirty change, "change", "moved"
applyDeleteToChanges: (op, metadata) -> applyDeleteToChanges: (op, metadata) ->
op_start = op.p op_start = op.p
@ -406,8 +411,8 @@ load = (EventEmitter) ->
@_removeChange change @_removeChange change
moved_changes = moved_changes.filter (c) -> c != change moved_changes = moved_changes.filter (c) -> c != change
if moved_changes.length > 0 for change in moved_changes
@emit "changes:moved", moved_changes @_markAsDirty change, "change", "moved"
_addOp: (op, metadata) -> _addOp: (op, metadata) ->
change = { change = {
@ -427,17 +432,11 @@ load = (EventEmitter) ->
else else
return -1 return -1
if op.d? @_markAsDirty(change, "change", "added")
@emit "delete:added", change
else if op.i?
@emit "insert:added", change
_removeChange: (change) -> _removeChange: (change) ->
@changes = @changes.filter (c) -> c.id != change.id @changes = @changes.filter (c) -> c.id != change.id
if change.op.d? @_markAsDirty change, "change", "removed"
@emit "delete:removed", change
else if change.op.i?
@emit "insert:removed", change
_applyOpModifications: (content, op_modifications) -> _applyOpModifications: (content, op_modifications) ->
# Put in descending position order, with deleting first if at the same offset # Put in descending position order, with deleting first if at the same offset
@ -486,13 +485,32 @@ load = (EventEmitter) ->
previous_change = change previous_change = change
return { moved_changes, remove_changes } return { moved_changes, remove_changes }
resetDirtyState: () ->
@_dirtyState = {
comment: {
moved: {}
removed: {}
added: {}
}
change: {
moved: {}
removed: {}
added: {}
}
}
getDirtyState: () ->
return @_dirtyState
_markAsDirty: (object, type, action) ->
@_dirtyState[type][action][object.id] = object
_clone: (object) -> _clone: (object) ->
clone = {} clone = {}
(clone[k] = v for k,v of object) (clone[k] = v for k,v of object)
return clone return clone
if define? if define?
define ["utils/EventEmitter"], load define [], load
else else
EventEmitter = require("events").EventEmitter module.exports = load()
module.exports = load(EventEmitter)