Skip to content

Commit d73840d

Browse files
committed
fix: optimize diff deduplication- use reference identity when diffs are from the same document pair
1 parent adc8992 commit d73840d

File tree

2 files changed

+12
-4
lines changed

2 files changed

+12
-4
lines changed

src/components/compare/compare.operations.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,18 @@ async function compareCurrentApiType(
140140
const operationsMap = createPairOperationsMap(previousGroupSlug, currentGroupSlug, prevOperationsWithPrefix, currOperationsWithPrefix, apiBuilder)
141141
const operationPairs = Object.values(operationsMap)
142142
const pairedDocs = await calculatePairedDocs(operationPairs, pairContext)
143-
const [operationChanges, tags] = await comparePairedDocs(operationsMap, pairedDocs, apiBuilder, pairContext)
143+
const [operationChanges, uniqueDiffsForDocPairs, tags] = await comparePairedDocs(operationsMap, pairedDocs, apiBuilder, pairContext)
144144
// Duplicates could happen in rare case when document for added/deleted operation was mapped to several documents in other version
145145
const uniqueOperationChanges = removeObjectDuplicates(
146146
operationChanges,
147147
(item) => `${item.apiType}:${item.operationId ?? ''}:${item.previousOperationId ?? ''}`,
148148
)
149149

150-
const uniqueDiffs = removeObjectDuplicates(uniqueOperationChanges.flatMap(({ diffs }) => diffs), calculateDiffId)
150+
// We only need to additionally deduplicate diffs if there are multiple document pairs
151+
// because diffs coming from the same apiDiff call are already deduplicated in comparePairedDocs
152+
// This is performance optimization for common case when there is only one document pair
153+
const uniqueDiffs = uniqueDiffsForDocPairs.length === 1 ? Array.from(uniqueDiffsForDocPairs[0])
154+
: removeObjectDuplicates(uniqueDiffsForDocPairs.flatMap(set => Array.from(set)), calculateDiffId)
151155
const changesSummary = calculateChangeSummary(uniqueDiffs)
152156
const numberOfImpactedOperations = calculateTotalImpactedSummary(
153157
uniqueOperationChanges.map(({ impactedSummary }) => impactedSummary),

src/components/compare/compare.utils.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,9 @@ export const comparePairedDocs = async (
234234
pairedDocs: [ResolvedVersionDocument | undefined, ResolvedVersionDocument | undefined][],
235235
apiBuilder: ApiBuilder,
236236
ctx: CompareOperationsPairContext,
237-
): Promise<[OperationChanges[], string[]]> => {
237+
): Promise<[OperationChanges[], Set<Diff>[], string[]]> => {
238238
const operationChanges: OperationChanges[] = []
239+
const uniqueDiffsForDocPairs: Set<Diff>[] = []
239240
const tags = new Set<string>()
240241

241242
for (const [prevDoc, currDoc] of pairedDocs) {
@@ -244,11 +245,14 @@ export const comparePairedDocs = async (
244245
tags: docsPairTags,
245246
} = await apiBuilder.compareDocuments!(operationsMap, prevDoc, currDoc, ctx)
246247

248+
// We can remove duplicates for diffs coming from the same apiDiff call using simple identity
249+
uniqueDiffsForDocPairs.push(new Set(docsPairOperationChanges.flatMap(({ diffs }) => diffs ?? [])))
250+
247251
operationChanges.push(...docsPairOperationChanges)
248252
docsPairTags.forEach(tag => tags.add(tag))
249253
}
250254

251-
return [operationChanges, Array.from(tags).sort()]
255+
return [operationChanges, uniqueDiffsForDocPairs, Array.from(tags).sort()]
252256
}
253257

254258
export function createOperationChange(

0 commit comments

Comments
 (0)