From def291f2600bfa6530541b01e6d756260c735c58 Mon Sep 17 00:00:00 2001 From: Marcus Tillmanns Date: Tue, 25 Jul 2023 08:17:13 +0200 Subject: [PATCH] Terminal: Improve paste performance Pasting large amounts of data on macos would block the App indefinitely. The issue was a blocking call to ::write. The first fix for that was to set O_NONBLOCK on the tty stdout fd. The second fix was to pass the actual result of the write back to the caller so they can react to it. In the TerminalSurface we now check if the write was successful and if not we buffer the data and try again later. Change-Id: Ibc92cce57fad88b5e9aa325197b42e17bec5e746 Reviewed-by: Cristian Adam Reviewed-by: Christian Stenger --- src/libs/3rdparty/libptyqt/unixptyprocess.cpp | 9 +-- src/libs/utils/process.cpp | 8 ++- src/plugins/terminal/terminalsurface.cpp | 67 ++++++++++++++++++- src/plugins/terminal/terminalsurface.h | 3 +- src/plugins/terminal/terminalwidget.cpp | 12 ++-- src/plugins/terminal/terminalwidget.h | 2 +- 6 files changed, 84 insertions(+), 17 deletions(-) diff --git a/src/libs/3rdparty/libptyqt/unixptyprocess.cpp b/src/libs/3rdparty/libptyqt/unixptyprocess.cpp index 8c018daf8c0..4c67ee28fb0 100644 --- a/src/libs/3rdparty/libptyqt/unixptyprocess.cpp +++ b/src/libs/3rdparty/libptyqt/unixptyprocess.cpp @@ -54,6 +54,10 @@ bool UnixPtyProcess::startProcess(const QString &shellPath, int rc = 0; m_shellProcess.m_handleMaster = ::posix_openpt(O_RDWR | O_NOCTTY); + + int flags = fcntl(m_shellProcess.m_handleMaster, F_GETFL, 0); + fcntl(m_shellProcess.m_handleMaster, F_SETFL, flags | O_NONBLOCK); + if (m_shellProcess.m_handleMaster <= 0) { m_lastError = QString("UnixPty Error: unable to open master -> %1").arg(QLatin1String(strerror(errno))); kill(); @@ -308,10 +312,7 @@ QByteArray UnixPtyProcess::readAll() qint64 UnixPtyProcess::write(const QByteArray &byteArray) { - int result = ::write(m_shellProcess.m_handleMaster, byteArray.constData(), byteArray.size()); - Q_UNUSED(result) - - return byteArray.size(); + return ::write(m_shellProcess.m_handleMaster, byteArray.constData(), byteArray.size()); } bool UnixPtyProcess::isAvailable() diff --git a/src/libs/utils/process.cpp b/src/libs/utils/process.cpp index 43d57ded891..5f8e3612f7f 100644 --- a/src/libs/utils/process.cpp +++ b/src/libs/utils/process.cpp @@ -1564,9 +1564,11 @@ qint64 Process::writeRaw(const QByteArray &input) QTC_ASSERT(state() == QProcess::Running, return -1); QTC_ASSERT(QThread::currentThread() == thread(), return -1); qint64 result = -1; - QMetaObject::invokeMethod(d->m_process.get(), [this, input] { - d->m_process->write(input); - }, d->connectionType(), &result); + QMetaObject::invokeMethod( + d->m_process.get(), + [this, input] { return d->m_process->write(input); }, + d->connectionType(), + &result); return result; } diff --git a/src/plugins/terminal/terminalsurface.cpp b/src/plugins/terminal/terminalsurface.cpp index caab0a93702..698f3589b56 100644 --- a/src/plugins/terminal/terminalsurface.cpp +++ b/src/plugins/terminal/terminalsurface.cpp @@ -11,6 +11,7 @@ #include #include +#include namespace Terminal::Internal { @@ -21,6 +22,8 @@ QColor toQColor(const VTermColor &c) return QColor(qRgb(c.rgb.red, c.rgb.green, c.rgb.blue)); }; +constexpr int batchFlushSize = 256; + struct TerminalSurfacePrivate { TerminalSurfacePrivate(TerminalSurface *surface, @@ -33,13 +36,64 @@ struct TerminalSurfacePrivate , q(surface) {} + void flush() + { + if (m_writeBuffer.isEmpty()) + return; + + QByteArray data = m_writeBuffer.left(batchFlushSize); + qint64 result = m_writeToPty(data); + + if (result != data.size()) { + // Not all data was written, remove the unwritten data from the array + data.resize(qMax(0, result)); + } + + // Remove the written data from the buffer + if (data.size() > 0) + m_writeBuffer = m_writeBuffer.mid(data.size()); + + if (!m_writeBuffer.isEmpty()) + m_delayWriteTimer.start(); + } + void init() { + m_delayWriteTimer.setInterval(1); + m_delayWriteTimer.setSingleShot(true); + + QObject::connect(&m_delayWriteTimer, &QTimer::timeout, &m_delayWriteTimer, [this] { + flush(); + }); + vterm_set_utf8(m_vterm.get(), true); static auto writeToPty = [](const char *s, size_t len, void *user) { auto p = static_cast(user); - emit p->q->writeToPty(QByteArray(s, static_cast(len))); + QByteArray d(s, len); + + // If its just a couple of chars, or we already have data in the writeBuffer, + // add the new data to the write buffer and start the delay timer + if (d.size() < batchFlushSize || !p->m_writeBuffer.isEmpty()) { + p->m_writeBuffer.append(d); + p->m_delayWriteTimer.start(); + return; + } + + // Try to write the data ... + qint64 result = p->m_writeToPty(d); + + if (result != d.size()) { + // if writing failed, append the data to the writeBuffer and start the delay timer + + // Check if partial data may have already been written ... + if (result <= 0) + p->m_writeBuffer.append(d); + else + p->m_writeBuffer.append(d.mid(result)); + + p->m_delayWriteTimer.start(); + } }; vterm_output_set_callback(m_vterm.get(), writeToPty, this); @@ -313,6 +367,10 @@ struct TerminalSurfacePrivate ShellIntegration *m_shellIntegration{nullptr}; TerminalSurface *q; + QTimer m_delayWriteTimer; + QByteArray m_writeBuffer; + + TerminalSurface::WriteToPty m_writeToPty; }; TerminalSurface::TerminalSurface(QSize initialGridSize, ShellIntegration *shellIntegration) @@ -385,7 +443,7 @@ void TerminalSurface::clearAll() vterm_input_write(d->m_vterm.get(), data.constData(), data.size()); // Send Ctrl+L which will clear the screen - emit writeToPty(QByteArray("\f")); + d->m_writeToPty(QByteArray("\f")); } void TerminalSurface::resize(QSize newSize) @@ -493,6 +551,11 @@ ShellIntegration *TerminalSurface::shellIntegration() const return d->m_shellIntegration; } +void TerminalSurface::setWriteToPty(WriteToPty writeToPty) +{ + d->m_writeToPty = writeToPty; +} + CellIterator TerminalSurface::begin() const { auto res = CellIterator(this, {0, 0}); diff --git a/src/plugins/terminal/terminalsurface.h b/src/plugins/terminal/terminalsurface.h index a6fc7425d48..3b737836f6b 100644 --- a/src/plugins/terminal/terminalsurface.h +++ b/src/plugins/terminal/terminalsurface.h @@ -95,8 +95,9 @@ public: ShellIntegration *shellIntegration() const; + using WriteToPty = std::function; + void setWriteToPty(WriteToPty writeToPty); signals: - void writeToPty(const QByteArray &data); void invalidated(QRect grid); void fullSizeChanged(QSize newSize); void cursorChanged(Cursor oldCursor, Cursor newCursor); diff --git a/src/plugins/terminal/terminalwidget.cpp b/src/plugins/terminal/terminalwidget.cpp index 4b6442753e1..dd105223536 100644 --- a/src/plugins/terminal/terminalwidget.cpp +++ b/src/plugins/terminal/terminalwidget.cpp @@ -327,10 +327,12 @@ void TerminalWidget::closeTerminal() deleteLater(); } -void TerminalWidget::writeToPty(const QByteArray &data) +qint64 TerminalWidget::writeToPty(const QByteArray &data) { if (m_process && m_process->isRunning()) - m_process->writeRaw(data); + return m_process->writeRaw(data); + + return data.size(); } void TerminalWidget::setupSurface() @@ -351,10 +353,8 @@ void TerminalWidget::setupSurface() } }); - connect(m_surface.get(), - &Internal::TerminalSurface::writeToPty, - this, - &TerminalWidget::writeToPty); + m_surface->setWriteToPty([this](const QByteArray &data) { return writeToPty(data); }); + connect(m_surface.get(), &Internal::TerminalSurface::fullSizeChanged, this, [this] { updateScrollBars(); }); diff --git a/src/plugins/terminal/terminalwidget.h b/src/plugins/terminal/terminalwidget.h index 11cd9335cc9..2d0f6b16df7 100644 --- a/src/plugins/terminal/terminalwidget.h +++ b/src/plugins/terminal/terminalwidget.h @@ -129,7 +129,7 @@ protected: void setupColors(); void setupActions(); - void writeToPty(const QByteArray &data); + qint64 writeToPty(const QByteArray &data); int paintCell(QPainter &p, const QRectF &cellRect,