UnifiedDiffEditor: Move showing diff into separate thread

Before, when all the data was finished, we called showDiff()
in main thread. This consisted of 2 parts:

1. Calculating some extra data and generating actual text
   for UnifiedDiffEditor out of input data.
2. Calling setPlainText() with generated text.

For a really big diffs this could freeze the main thread for a
couple of seconds. Like e.g. 05c35356ab
(initial Creator import) - it contained 7 million characters,
part 1. took about 500 ms and part 2. took about 2.5 seconds.

This two tasks are now done in separate thread.
However, since we can't call TextEditorWidget::setPlainText()
directly from non-GUI thread, we create a separate
TextDocument object in the worker thread, fill it with
generated diff input and move the TextDocument object
into the main thread as a result of async computation.
In main thread we replace TextDocument object of the
TextEditorWidget with the one generated in other thread.
This replacement is very fast.

Change-Id: I49a717ced1dc2d5b8946e0fd6bee244b25071f35
Reviewed-by: David Schulz <david.schulz@qt.io>
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

src/plugins/diffeditor/diffeditorplugin.cpp

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0+ OR GPL-3.0 WITH Qt-GPL-exception-1.0
 
 #include "diffeditorplugin.h"
-#include "diffeditor.h"
 #include "diffeditorconstants.h"
 #include "diffeditorcontroller.h"
 #include "diffeditordocument.h"
@@ -24,10 +23,10 @@
 
 #include <texteditor/textdocument.h>
 #include <texteditor/texteditor.h>
-#include "texteditor/texteditoractionhandler.h"
 
 #include <utils/algorithm.h>
 #include <utils/differ.h>
+#include <utils/futuresynchronizer.h>
 #include <utils/mapreduce.h>
 #include <utils/qtcassert.h>
 
@@ -440,12 +439,14 @@ public:
     QAction *m_diffCurrentFileAction = nullptr;
     QAction *m_diffOpenFilesAction = nullptr;
 
-    DiffEditorFactory editorFactory;
-    DiffEditorServiceImpl service;
+    DiffEditorFactory m_editorFactory;
+    DiffEditorServiceImpl m_service;
+    FutureSynchronizer m_futureSynchronizer;
 };
 
 DiffEditorPluginPrivate::DiffEditorPluginPrivate()
 {
+    m_futureSynchronizer.setCancelOnWait(true);
     //register actions
     ActionContainer *toolsContainer
             = ActionManager::actionContainer(Core::Constants::M_TOOLS);
@@ -571,9 +572,17 @@ void DiffEditorPluginPrivate::diffExternalFiles()
     document->reload();
 }
 
+static DiffEditorPlugin *s_instance = nullptr;
+
+DiffEditorPlugin::DiffEditorPlugin()
+{
+    s_instance = this;
+}
+
 DiffEditorPlugin::~DiffEditorPlugin()
 {
     delete d;
+    s_instance = nullptr;
 }
 
 bool DiffEditorPlugin::initialize(const QStringList &arguments, QString *errorMessage)
@@ -586,6 +595,13 @@ bool DiffEditorPlugin::initialize(const QStringList &arguments, QString *errorMe
     return true;
 }
 
+void DiffEditorPlugin::addFuture(const QFuture<void> &future)
+{
+    QTC_ASSERT(s_instance, return);
+    s_instance->d->m_futureSynchronizer.addFuture(future);
+}
+
+
 } // namespace Internal
 } // namespace DiffEditor
 

src/plugins/diffeditor/diffeditorplugin.h

@@ -8,6 +8,11 @@
 #include <coreplugin/diffservice.h>
 #include <extensionsystem/iplugin.h>
 
+QT_BEGIN_NAMESPACE
+template <typename T>
+class QFuture;
+QT_END_NAMESPACE
+
 namespace DiffEditor {
 namespace Internal {
 
@@ -29,10 +34,15 @@ class DiffEditorPlugin final : public ExtensionSystem::IPlugin
     Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QtCreatorPlugin" FILE "DiffEditor.json")
 
 public:
-    ~DiffEditorPlugin() final;
+    DiffEditorPlugin();
+    ~DiffEditorPlugin();
 
     bool initialize(const QStringList &arguments, QString *errorMessage) final;
 
+    template <typename T>
+    static void addFuture(const QFuture<T> &future) { addFuture(QFuture<void>(future)); }
+    static void addFuture(const QFuture<void> &future);
+
 private:
     class DiffEditorPluginPrivate *d = nullptr;
 

src/plugins/diffeditor/diffeditorwidgetcontroller.cpp

@@ -43,6 +43,23 @@ DiffEditorWidgetController::DiffEditorWidgetController(QWidget *diffEditorWidget
     connect(&m_timer, &QTimer::timeout, this, &DiffEditorWidgetController::showProgress);
 }
 
+bool DiffEditorWidgetController::isInProgress() const
+{
+    return m_isBusyShowing || (m_document && m_document->state() == DiffEditorDocument::Reloading);
+}
+
+void DiffEditorWidgetController::toggleProgress(bool wasInProgress)
+{
+    const bool inProgress = isInProgress();
+    if (wasInProgress == inProgress)
+        return;
+
+    if (inProgress)
+        scheduleShowProgress();
+    else
+        hideProgress();
+}
+
 void DiffEditorWidgetController::setDocument(DiffEditorDocument *document)
 {
     if (!m_progressIndicator) {
@@ -59,25 +76,16 @@ void DiffEditorWidgetController::setDocument(DiffEditorDocument *document)
         disconnect(m_document, &IDocument::reloadFinished, this, &DiffEditorWidgetController::onDocumentReloadFinished);
     }
 
-    const bool wasRunning = m_document && m_document->state() == DiffEditorDocument::Reloading;
+    const bool wasInProgress = isInProgress();
 
     m_document = document;
-
     if (m_document) {
         connect(m_document, &IDocument::aboutToReload, this, &DiffEditorWidgetController::scheduleShowProgress);
         connect(m_document, &IDocument::reloadFinished, this, &DiffEditorWidgetController::onDocumentReloadFinished);
         updateCannotDecodeInfo();
     }
 
-    const bool isRunning = m_document && m_document->state() == DiffEditorDocument::Reloading;
-
-    if (wasRunning == isRunning)
-        return;
-
-    if (isRunning)
-        scheduleShowProgress();
-    else
-        hideProgress();
+    toggleProgress(wasInProgress);
 }
 
 DiffEditorDocument *DiffEditorWidgetController::document() const
@@ -85,6 +93,16 @@ DiffEditorDocument *DiffEditorWidgetController::document() const
     return m_document;
 }
 
+void DiffEditorWidgetController::setBusyShowing(bool busy)
+{
+    if (m_isBusyShowing == busy)
+        return;
+
+    const bool wasInProgress = isInProgress();
+    m_isBusyShowing = busy;
+    toggleProgress(wasInProgress);
+}
+
 void DiffEditorWidgetController::scheduleShowProgress()
 {
     m_timer.start();
@@ -107,6 +125,7 @@ void DiffEditorWidgetController::hideProgress()
 void DiffEditorWidgetController::onDocumentReloadFinished()
 {
     updateCannotDecodeInfo();
+    if (!isInProgress())
         hideProgress();
 }
 

src/plugins/diffeditor/diffeditorwidgetcontroller.h

@@ -42,6 +42,7 @@ public:
     void addRevertAction(QMenu *menu, int fileIndex, int chunkIndex);
     void addExtraActions(QMenu *menu, int fileIndex, int chunkIndex, const ChunkSelection &selection);
     void updateCannotDecodeInfo();
+    void setBusyShowing(bool busy);
 
     ChunkData chunkData(int fileIndex, int chunkIndex) const;
 
@@ -56,6 +57,9 @@ public:
     QTextCharFormat m_rightCharFormat;
 
 private:
+    bool isInProgress() const;
+    void toggleProgress(bool wasInProgress);
+
     void patch(bool revert, int fileIndex, int chunkIndex);
     void sendChunkToCodePaster(int fileIndex, int chunkIndex);
     bool chunkExists(int fileIndex, int chunkIndex) const;
@@ -70,6 +74,7 @@ private:
 
     DiffEditorDocument *m_document = nullptr;
 
+    bool m_isBusyShowing = false;
     Utils::ProgressIndicator *m_progressIndicator = nullptr;
     QTimer m_timer;
 };

src/plugins/diffeditor/unifieddiffeditorwidget.cpp

@@ -5,6 +5,7 @@
 
 #include "diffeditorconstants.h"
 #include "diffeditordocument.h"
+#include "diffeditorplugin.h"
 #include "diffutils.h"
 
 #include <QHash>
@@ -14,6 +15,7 @@
 #include <QTextBlock>
 
 #include <coreplugin/icore.h>
+#include <coreplugin/progressmanager/progressmanager.h>
 
 #include <texteditor/textdocument.h>
 #include <texteditor/textdocumentlayout.h>
@@ -21,6 +23,8 @@
 #include <texteditor/fontsettings.h>
 #include <texteditor/displaysettings.h>
 
+#include <utils/qtcassert.h>
+#include <utils/runextensions.h>
 #include <utils/tooltip/tooltip.h>
 
 using namespace Core;
@@ -34,17 +38,18 @@ UnifiedDiffEditorWidget::UnifiedDiffEditorWidget(QWidget *parent)
     : SelectableTextEditorWidget("DiffEditor.UnifiedDiffEditor", parent)
     , m_controller(this)
 {
+    setReadOnly(true);
+
     DisplaySettings settings = displaySettings();
     settings.m_textWrapping = false;
     settings.m_displayLineNumbers = true;
     settings.m_markTextChanges = false;
     settings.m_highlightBlocks = false;
     SelectableTextEditorWidget::setDisplaySettings(settings);
-
-    setReadOnly(true);
     connect(TextEditorSettings::instance(), &TextEditorSettings::displaySettingsChanged,
             this, &UnifiedDiffEditorWidget::setDisplaySettings);
     setDisplaySettings(TextEditorSettings::displaySettings());
+
     setCodeStyle(TextEditorSettings::codeStyle());
 
     connect(TextEditorSettings::instance(), &TextEditorSettings::fontSettingsChanged,
@@ -63,14 +68,20 @@ UnifiedDiffEditorWidget::UnifiedDiffEditorWidget(QWidget *parent)
     setCodeFoldingSupported(true);
 }
 
+UnifiedDiffEditorWidget::~UnifiedDiffEditorWidget()
+{
+    if (m_watcher) {
+        m_watcher->cancel();
+        DiffEditorPlugin::addFuture(m_watcher->future());
+    }
+}
+
 void UnifiedDiffEditorWidget::setDocument(DiffEditorDocument *document)
 {
+    m_controller.setBusyShowing(true);
     m_controller.setDocument(document);
     clear();
-    QList<FileData> diffFileList;
-    if (document)
-        diffFileList = document->diffFiles();
-    setDiff(diffFileList);
+    setDiff(document ? document->diffFiles() : QList<FileData>());
 }
 
 DiffEditorDocument *UnifiedDiffEditorWidget::diffDocument() const
@@ -113,11 +124,15 @@ void UnifiedDiffEditorWidget::setFontSettings(const FontSettings &fontSettings)
 
 void UnifiedDiffEditorWidget::slotCursorPositionChangedInEditor()
 {
+    const int fileIndex = fileIndexForBlockNumber(textCursor().blockNumber());
+    if (fileIndex < 0)
+        return;
+
     if (m_controller.m_ignoreChanges.isLocked())
         return;
 
     const GuardLocker locker(m_controller.m_ignoreChanges);
-    emit currentDiffFileIndexChanged(fileIndexForBlockNumber(textCursor().blockNumber()));
+    emit currentDiffFileIndexChanged(fileIndex);
 }
 
 void UnifiedDiffEditorWidget::mouseDoubleClickEvent(QMouseEvent *e)
@@ -214,6 +229,12 @@ void UnifiedDiffEditorWidget::clear(const QString &message)
 {
     m_data = {};
     setSelections({});
+    if (m_watcher) {
+        m_watcher->cancel();
+        DiffEditorPlugin::addFuture(m_watcher->future());
+        m_watcher.reset();
+        m_controller.setBusyShowing(false);
+    }
 
     const GuardLocker locker(m_controller.m_ignoreChanges);
     SelectableTextEditorWidget::clear();
@@ -279,7 +300,7 @@ void UnifiedDiffData::setChunkIndex(int startBlockNumber, int blockCount, int ch
 void UnifiedDiffEditorWidget::setDiff(const QList<FileData> &diffFileList)
 {
     const GuardLocker locker(m_controller.m_ignoreChanges);
-    clear();
+    clear(tr("Waiting for data..."));
     m_controller.m_contextFileData = diffFileList;
     showDiff();
 }
@@ -457,12 +478,28 @@ QString UnifiedDiffData::showChunk(const DiffEditorInput &input, const ChunkData
     return diffText;
 }
 
-UnifiedDiffOutput UnifiedDiffData::showDiff(const DiffEditorInput &input)
+static int interpolate(int x, int x1, int x2, int y1, int y2)
+{
+    if (x1 == x2)
+        return x1;
+    if (x == x1)
+        return y1;
+    if (x == x2)
+        return y2;
+    const int numerator = (y2 - y1) * x + x2 * y1 - x1 * y2;
+    const int denominator = x2 - x1;
+    return qRound((double)numerator / denominator);
+}
+
+UnifiedDiffOutput UnifiedDiffData::showDiff(QFutureInterface<void> &fi, int progressMin,
+                                            int progressMax, const DiffEditorInput &input)
 {
     UnifiedDiffOutput output;
 
     int blockNumber = 0;
     int charNumber = 0;
+    int i = 0;
+    const int count = input.m_contextFileData.size();
 
     for (const FileData &fileData : qAsConst(input.m_contextFileData)) {
         const QString leftFileInfo = "--- " + fileData.leftFileInfo.fileName + '\n';
@@ -504,6 +541,9 @@ UnifiedDiffOutput UnifiedDiffData::showDiff(const DiffEditorInput &input)
                     setChunkIndex(oldBlockNumber, blockNumber - oldBlockNumber, j);
             }
         }
+        fi.setProgressValue(interpolate(++i, 0, count, progressMin, progressMax));
+        if (fi.isCanceled())
+            return {};
     }
 
     output.diffText.replace('\r', ' ');
@@ -512,24 +552,87 @@ UnifiedDiffOutput UnifiedDiffData::showDiff(const DiffEditorInput &input)
 
 void UnifiedDiffEditorWidget::showDiff()
 {
-    const DiffEditorInput input = {&m_controller};
-    const UnifiedDiffOutput output = m_data.showDiff(input);
-
-    if (output.diffText.isEmpty()) {
+    if (m_controller.m_contextFileData.isEmpty()) {
         setPlainText(tr("No difference."));
         return;
     }
 
+    m_watcher.reset(new QFutureWatcher<ShowResult>());
+    m_controller.setBusyShowing(true);
+    connect(m_watcher.get(), &QFutureWatcherBase::finished, this, [this] {
+        if (m_watcher->isCanceled()) {
+            setPlainText(tr("Retrieving data failed."));
+        } else {
+            const ShowResult result = m_watcher->result();
+            m_data = result.diffData;
+            TextDocumentPtr doc(result.textDocument);
             {
                 const GuardLocker locker(m_controller.m_ignoreChanges);
-        setPlainText(output.diffText);
+                // TextDocument was living in no thread, so it's safe to pull it
+                doc->moveToThread(thread());
+                setTextDocument(doc);
+
+                setReadOnly(true);
 
                 QTextBlock block = document()->firstBlock();
                 for (int b = 0; block.isValid(); block = block.next(), ++b)
-            setFoldingIndent(block, output.foldingIndent.value(b, 3));
+                    setFoldingIndent(block, result.foldingIndent.value(b, 3));
+            }
+            // TODO: this could probably be done in thread, too
+            setSelections(result.selections);
+        }
+        m_watcher.release()->deleteLater();
+        m_controller.setBusyShowing(false);
+    });
+
+    const DiffEditorInput input(&m_controller);
+
+    auto getDocument = [=](QFutureInterface<ShowResult> &futureInterface) {
+        auto cleanup = qScopeGuard([&futureInterface] {
+            if (futureInterface.isCanceled())
+                futureInterface.reportCanceled();
+        });
+        const int progressMax = 100;
+        const int firstPartMax = 20; // showDiff is about 4 times quicker than filling document
+        futureInterface.setProgressRange(0, progressMax);
+        futureInterface.setProgressValue(0);
+        QFutureInterface<void> fi = futureInterface;
+        UnifiedDiffData diffData;
+        const UnifiedDiffOutput output = diffData.showDiff(fi, 0, firstPartMax, input);
+        if (futureInterface.isCanceled())
+            return;
+
+        const ShowResult result = {TextDocumentPtr(new TextDocument("DiffEditor.UnifiedDiffEditor")),
+                                   diffData, output.foldingIndent, output.selections};
+        // No need to store the change history
+        result.textDocument->document()->setUndoRedoEnabled(false);
+
+        // We could do just:
+        //   result.textDocument->setPlainText(output.diffText);
+        // but this would freeze the thread for couple of seconds without progress reporting
+        // and without checking for canceled.
+        const int diffSize = output.diffText.size();
+        const int packageSize = 100000;
+        int currentPos = 0;
+        QTextCursor cursor(result.textDocument->document());
+        while (currentPos < diffSize) {
+            const QString package = output.diffText.mid(currentPos, packageSize);
+            cursor.insertText(package);
+            currentPos += package.size();
+            fi.setProgressValue(interpolate(currentPos, 0, diffSize, firstPartMax, progressMax));
+            if (futureInterface.isCanceled())
+                return;
         }
 
-    setSelections(output.selections);
+        // If future was canceled, the destructor runs in this thread, so we can't move it
+        // to caller's thread. We push it to no thread (make object to have no thread affinity),
+        // and later, in the caller's thread, we pull it back to the caller's thread.
+        result.textDocument->moveToThread(nullptr);
+        futureInterface.reportResult(result);
+    };
+
+    m_watcher->setFuture(runAsync(getDocument));
+    ProgressManager::addTask(m_watcher->future(), tr("Rendering diff"), "DiffEditor");
 }
 
 int UnifiedDiffEditorWidget::blockNumberForFileIndex(int fileIndex) const

src/plugins/diffeditor/unifieddiffeditorwidget.h

@@ -6,6 +6,8 @@
 #include "selectabletexteditorwidget.h"
 #include "diffeditorwidgetcontroller.h"
 
+#include <QFutureWatcher>
+
 namespace Core { class IContext; }
 
 namespace TextEditor {
@@ -38,7 +40,8 @@ public:
 class UnifiedDiffData
 {
 public:
-    UnifiedDiffOutput showDiff(const DiffEditorInput &input);
+    UnifiedDiffOutput showDiff(QFutureInterface<void> &fi, int progressMin, int progressMax,
+                               const DiffEditorInput &input);
 
     // block number, visual line number, chunk row number
     QMap<int, QPair<int, int>> m_leftLineNumbers;
@@ -67,6 +70,7 @@ class UnifiedDiffEditorWidget final : public SelectableTextEditorWidget
     Q_OBJECT
 public:
     UnifiedDiffEditorWidget(QWidget *parent = nullptr);
+    ~UnifiedDiffEditorWidget();
 
     void setDocument(DiffEditorDocument *document);
     DiffEditorDocument *diffDocument() const;
@@ -110,6 +114,16 @@ private:
     UnifiedDiffData m_data;
     DiffEditorWidgetController m_controller;
     QByteArray m_state;
+
+    struct ShowResult
+    {
+        QSharedPointer<TextEditor::TextDocument> textDocument;
+        UnifiedDiffData diffData;
+        QHash<int, int> foldingIndent;
+        QMap<int, QList<DiffSelection>> selections;
+    };
+
+    std::unique_ptr<QFutureWatcher<ShowResult>> m_watcher;
 };
 
 } // namespace Internal