Skip to content

Commit d0eb476

Browse files
committed
perf(findChain): Cache findChain calls more aggressively
improves processing times for large diffs by up to 29% Signed-off-by: Marcel Klehr <mklehr@gmx.net>
1 parent db02934 commit d0eb476

File tree

1 file changed

+25
-12
lines changed

1 file changed

+25
-12
lines changed

src/lib/strategies/Default.ts

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -561,10 +561,11 @@ export default class SyncProcess {
561561
REORDER: new Diff(),
562562
}
563563

564+
const findChainCacheForRemovals = {}
564565
await Parallel.each(sourceScanResult.REMOVE.getActions(), async(action) => {
565566
const concurrentRemoval = targetRemovals.find(targetRemoval =>
566567
(action.payload.type === targetRemoval.payload.type && Mappings.mappable(mappingsSnapshot, action.payload, targetRemoval.payload)) ||
567-
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, targetRemoval))
568+
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, targetRemoval, findChainCacheForRemovals))
568569
if (concurrentRemoval) {
569570
// Already deleted on target, do nothing.
570571
return
@@ -581,6 +582,7 @@ export default class SyncProcess {
581582
targetPlan.REMOVE.commit(action)
582583
}, ACTION_CONCURRENCY)
583584

585+
const findChainCacheForCreations = {}
584586
await Parallel.each(sourceScanResult.CREATE.getActions(), async(action) => {
585587
const concurrentCreation = targetCreations.find(a => (
586588
action.payload.parentId === Mappings.mapParentId(mappingsSnapshot, a.payload, action.payload.location) &&
@@ -611,9 +613,10 @@ export default class SyncProcess {
611613
// TODO: subScanner may contain residual CREATE/REMOVE actions that need to be added to mappings
612614
return
613615
}
616+
614617
const concurrentRemoval = targetScanResult.REMOVE.getActions().find(targetRemoval =>
615618
// target removal removed this creation's target (via some chain)
616-
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval)
619+
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval, findChainCacheForCreations)
617620
)
618621
if (concurrentRemoval) {
619622
avoidTargetReorders[action.payload.parentId] = true
@@ -633,20 +636,24 @@ export default class SyncProcess {
633636
return
634637
}
635638
}
639+
let findChainCache = {}
636640
// FInd out if there's a removal in the target diff which already deletes this item (via some chain of MOVE|CREATEs)
637641
const complexTargetTargetRemoval = targetRemovals.find(targetRemoval => {
638-
return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval)
642+
return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval, findChainCache)
639643
})
644+
findChainCache = {}
640645
const concurrentTargetOriginRemoval = targetRemovals.find(targetRemoval =>
641646
(action.payload.type === targetRemoval.payload.type && Mappings.mappable(mappingsSnapshot, action.payload, targetRemoval.payload)) ||
642-
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.oldItem, targetRemoval)
647+
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.oldItem, targetRemoval, findChainCache)
643648
)
649+
findChainCache = {}
644650
const concurrentSourceOriginRemoval = sourceRemovals.find(sourceRemoval => {
645-
return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.oldItem, sourceRemoval)
651+
return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.oldItem, sourceRemoval, findChainCache)
646652
})
653+
findChainCache = {}
647654
const concurrentSourceTargetRemoval = sourceRemovals.find(sourceRemoval =>
648655
// We pass an empty folder here, because we don't want direct deletions of the moved folder's parent to count, as it's moved away anyway
649-
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, new Folder({id: 0, location: targetLocation}), action.payload, sourceRemoval)
656+
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, new Folder({id: 0, location: targetLocation}), action.payload, sourceRemoval, findChainCache)
650657
)
651658
if (complexTargetTargetRemoval) {
652659
// target already deleted by a target|source REMOVE (connected via source MOVE|CREATEs)
@@ -697,15 +704,18 @@ export default class SyncProcess {
697704
}
698705
return
699706
}
707+
let findChainCache1 = {}, findChainCache2 = {}
700708
// Find concurrent moves that form a hierarchy reversal together with this one
701709
const concurrentHierarchyReversals = targetMoves.filter(targetMove => {
702-
return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetMove) &&
703-
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, targetMove.payload, action)
710+
return Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetMove, findChainCache1) &&
711+
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, targetMove.payload, action, findChainCache2)
704712
})
705713
if (concurrentHierarchyReversals.length) {
706714
if (targetLocation !== this.masterLocation) {
707715
targetPlan.MOVE.commit(action)
708716

717+
findChainCache1 = {}
718+
findChainCache2 = {}
709719
concurrentHierarchyReversals.forEach(a => {
710720
// moved sourcely but moved in reverse hierarchical order on target
711721
const payload = a.oldItem.copyWithLocation(false, action.payload.location)
@@ -718,8 +728,8 @@ export default class SyncProcess {
718728
targetPlan.MOVE.getActions().find(move => String(move.payload.id) === String(payload.id)) ||
719729
sourceMoves.find(move => String(move.payload.id) === String(payload.id)) ||
720730
// Don't move back into removed territory
721-
targetRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, remove)) ||
722-
sourceRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, remove))
731+
targetRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, remove, findChainCache)) ||
732+
sourceRemovals.find(remove => Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, targetTree, action.payload, remove, findChainCache))
723733
) {
724734
return
725735
}
@@ -770,8 +780,9 @@ export default class SyncProcess {
770780
}
771781
}
772782

783+
const findChainCache = {}
773784
const concurrentRemoval = targetRemovals.find(targetRemoval =>
774-
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval)
785+
Diff.findChain(mappingsSnapshot, allCreateAndMoveActions, sourceTree, action.payload, targetRemoval, findChainCache)
775786
)
776787
if (concurrentRemoval) {
777788
// Already deleted on target, do nothing.
@@ -1117,6 +1128,8 @@ export default class SyncProcess {
11171128

11181129
const newReorders = new Diff<L2, TItemLocation, ReorderAction<L2, TItemLocation>>
11191130

1131+
const findChainCache = {}
1132+
11201133
targetReorders
11211134
.getActions()
11221135
// MOVEs have oldItem from cacheTree and payload now mapped to their corresponding target tree
@@ -1128,7 +1141,7 @@ export default class SyncProcess {
11281141
const removed = targetRemovals
11291142
.filter(removal =>
11301143
removal.payload.findItem(reorderAction.payload.type, reorderAction.payload.id) ||
1131-
Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, reorderAction.payload, removal))
1144+
Diff.findChain(mappingSnapshot, targetCreationsAndMoves, targetTree, reorderAction.payload, removal, findChainCache))
11321145
if (removed.length) {
11331146
return
11341147
}

0 commit comments

Comments
 (0)