Skip to content

Commit 196a2b9

Browse files
committed
Show a large comparison before computing its differences
TextMergeViewer computed the line diff and the token diffs of every change before the documents were painted, so the editor stayed blank for the whole comparison. Once both sides together exceed 2000 lines the first comparison now runs in a follow-up UI event, the same way the re-diff after an edit already does, and the change highlighting plus the jump to the first difference follow when it is done. Smaller inputs are still compared right away: there the diff costs next to nothing and showing the text and jumping to the first change in two steps would only flicker. A refresh is never deferred either, because it restores the cached selection and scroll position which are dropped as soon as the refresh returns. Measured on Linux with Xvfb, medians over 15 repetitions, time from openCompareEditor until the text is visible: 5000 lines, 100 changes: 237ms -> 77ms 5000 lines, 1250 changes: 491ms -> 96ms 50000 lines, 1000 changes: 427ms -> 135ms Contributes to #2795
1 parent f1f10bb commit 196a2b9

2 files changed

Lines changed: 127 additions & 29 deletions

File tree

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java

Lines changed: 78 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,18 @@ private static RGB interpolate(RGB fg, RGB bg, double scale) {
502502
private boolean isConfigured = false;
503503
private boolean fRedoDiff = false;
504504

505+
/**
506+
* Combined line count of both sides from which on the first comparison is
507+
* deferred, so that the documents are painted before they are compared. Smaller
508+
* inputs are compared right away: the diff costs next to nothing there, and
509+
* showing the text and jumping to the first change in two steps would only
510+
* flicker.
511+
*/
512+
private static final int DEFER_DIFF_LINE_COUNT = 2000;
513+
514+
/** Pending first diff of the current input, scheduled after the documents are set. */
515+
private UIJob fInitialDiffJob;
516+
505517
private double fCurrMagni = 0;
506518

507519
private int fCurrentHeight;
@@ -2103,6 +2115,11 @@ protected String getDocumentPartitioning() {
21032115
protected void handleDispose(DisposeEvent event) {
21042116
OperationHistoryFactory.getOperationHistory().removeOperationHistoryListener(operationHistoryListener);
21052117

2118+
if (fInitialDiffJob != null) {
2119+
fInitialDiffJob.cancel();
2120+
fInitialDiffJob = null;
2121+
}
2122+
21062123
if (fHandlerService != null) {
21072124
fHandlerService.dispose();
21082125
}
@@ -3211,41 +3228,73 @@ protected void updateContent(Object ancestor, Object left, Object right) {
32113228

32123229
setSyncScrolling(fPreferenceStore.getBoolean(ComparePreferencePage.SYNCHRONIZE_SCROLLING));
32133230

3214-
update(false);
3215-
3216-
if (!fHasErrors && !emptyInput && !fComposite.isDisposed()) {
3217-
if (isRefreshing()) {
3218-
fLeftContributor.updateSelection(fLeft, !fSynchronizedScrolling);
3219-
fRightContributor.updateSelection(fRight, !fSynchronizedScrolling);
3220-
fAncestorContributor.updateSelection(fAncestor, !fSynchronizedScrolling);
3221-
if (fSynchronizedScrolling && fSynchronziedScrollPosition != -1) {
3222-
synchronizedScrollVertical(fSynchronziedScrollPosition);
3223-
}
3224-
} else {
3225-
if (isPatchHunk()) {
3226-
if (right != null && Adapters.adapt(right, IHunk.class) != null) {
3227-
fLeft.getSourceViewer().setTopIndex(getHunkStart());
3228-
} else {
3229-
fRight.getSourceViewer().setTopIndex(getHunkStart());
3231+
// A refresh restores the cached selection and scroll position, and that cache is
3232+
// dropped as soon as the refresh returns, so a refresh is never deferred.
3233+
if (isRefreshing() || fLeftLineCount + fRightLineCount <= DEFER_DIFF_LINE_COUNT) {
3234+
update(false);
3235+
if (!fHasErrors && !emptyInput && !fComposite.isDisposed()) {
3236+
if (isRefreshing()) {
3237+
fLeftContributor.updateSelection(fLeft, !fSynchronizedScrolling);
3238+
fRightContributor.updateSelection(fRight, !fSynchronizedScrolling);
3239+
fAncestorContributor.updateSelection(fAncestor, !fSynchronizedScrolling);
3240+
if (fSynchronizedScrolling && fSynchronziedScrollPosition != -1) {
3241+
synchronizedScrollVertical(fSynchronziedScrollPosition);
32303242
}
32313243
} else {
3232-
Diff selectDiff= null;
3233-
if (FIX_47640) {
3234-
if (leftRange != null) {
3235-
selectDiff= fMerger.findDiff(LEFT_CONTRIBUTOR, leftRange);
3236-
} else if (rightRange != null) {
3237-
selectDiff= fMerger.findDiff(RIGHT_CONTRIBUTOR, rightRange);
3238-
}
3239-
}
3240-
if (selectDiff != null) {
3241-
setCurrentDiff(selectDiff, true);
3242-
} else {
3243-
selectFirstDiff(true);
3244-
}
3244+
revealInitialDiff(right, leftRange, rightRange);
32453245
}
32463246
}
3247+
return;
32473248
}
32483249

3250+
// The documents are set, so let them be painted before comparing them. The diff
3251+
// and everything derived from it follows in a separate UI event.
3252+
final boolean isEmptyInput = emptyInput;
3253+
final Object rightElement = right;
3254+
final Position leftSelectRange = leftRange;
3255+
final Position rightSelectRange = rightRange;
3256+
if (fInitialDiffJob != null) {
3257+
fInitialDiffJob.cancel();
3258+
}
3259+
fInitialDiffJob = new UIJob(CompareMessages.DocumentMerger_0) {
3260+
@Override
3261+
public IStatus runInUIThread(IProgressMonitor monitor) {
3262+
fInitialDiffJob = null;
3263+
if (fComposite == null || fComposite.isDisposed()) {
3264+
return Status.OK_STATUS;
3265+
}
3266+
update(false);
3267+
if (!fHasErrors && !isEmptyInput && !fComposite.isDisposed()) {
3268+
revealInitialDiff(rightElement, leftSelectRange, rightSelectRange);
3269+
}
3270+
return Status.OK_STATUS;
3271+
}
3272+
};
3273+
fInitialDiffJob.schedule();
3274+
}
3275+
3276+
private void revealInitialDiff(Object right, Position leftRange, Position rightRange) {
3277+
if (isPatchHunk()) {
3278+
if (right != null && Adapters.adapt(right, IHunk.class) != null) {
3279+
fLeft.getSourceViewer().setTopIndex(getHunkStart());
3280+
} else {
3281+
fRight.getSourceViewer().setTopIndex(getHunkStart());
3282+
}
3283+
return;
3284+
}
3285+
Diff selectDiff= null;
3286+
if (FIX_47640) {
3287+
if (leftRange != null) {
3288+
selectDiff= fMerger.findDiff(LEFT_CONTRIBUTOR, leftRange);
3289+
} else if (rightRange != null) {
3290+
selectDiff= fMerger.findDiff(RIGHT_CONTRIBUTOR, rightRange);
3291+
}
3292+
}
3293+
if (selectDiff != null) {
3294+
setCurrentDiff(selectDiff, true);
3295+
} else {
3296+
selectFirstDiff(true);
3297+
}
32493298
}
32503299

32513300
private void configureSourceViewer(SourceViewer sourceViewer, boolean editable, ContributorInfo contributor) {

team/tests/org.eclipse.compare.tests/src/org/eclipse/compare/tests/TextMergeViewerTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,55 @@ public void testCopyEmptyLeftToRightAndModify() throws Exception {
459459
}
460460
}
461461

462+
/**
463+
* A large input must be shown before it is compared, so the differences are only
464+
* available once the deferred comparison ran.
465+
*/
466+
@Test
467+
public void testLargeInputIsShownBeforeItIsCompared() throws Exception {
468+
DiffNode parentNode = new DiffNode(new ParentTestElement(), new ParentTestElement());
469+
DiffNode testNode = new DiffNode(parentNode, Differencer.CHANGE, null,
470+
new EditableTestElement(manyLines("line", 3000).getBytes()),
471+
new EditableTestElement(manyLines("LINE", 3000).getBytes()));
472+
473+
runInDialog(testNode, () -> {
474+
IMergeViewerTestAdapter ta = viewer.getAdapter(IMergeViewerTestAdapter.class);
475+
assertEquals(0, ta.getChangesCount(), "a large input must not be compared before it is shown");
476+
waitForChanges(ta);
477+
assertTrue(ta.getChangesCount() > 0, "the deferred comparison did not produce differences");
478+
});
479+
}
480+
481+
private static String manyLines(String prefix, int count) {
482+
StringBuilder content = new StringBuilder();
483+
for (int i = 0; i < count; i++) {
484+
content.append(prefix).append(' ').append(i).append('\n');
485+
}
486+
return content.toString();
487+
}
488+
489+
private static void waitForChanges(IMergeViewerTestAdapter ta) {
490+
Display display = Display.getCurrent();
491+
long deadline = System.currentTimeMillis() + 30_000;
492+
// A self-rescheduling timer keeps the loop waking so the deadline is enforced
493+
// even while blocked in Display.sleep().
494+
Runnable[] wake = new Runnable[1];
495+
wake[0] = () -> display.timerExec(50, wake[0]);
496+
display.timerExec(50, wake[0]);
497+
try {
498+
while (ta.getChangesCount() == 0) {
499+
if (System.currentTimeMillis() > deadline) {
500+
fail("no differences within 30000ms");
501+
}
502+
if (!display.readAndDispatch()) {
503+
display.sleep();
504+
}
505+
}
506+
} finally {
507+
display.timerExec(-1, wake[0]);
508+
}
509+
}
510+
462511
@Test
463512
public void testCompareFilter() throws Exception {
464513
DiffNode parentNode = new DiffNode(new ParentTestElement(),

0 commit comments

Comments
 (0)