From e06dd6fbd9921c8db3b2bcb054b23b387ded4884 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 25 Jan 2018 11:29:48 +0100 Subject: Make PerfStdin behave more like other QIODevices Buffer the data and emit readyRead when new data is available. Avoid the "oops" momemnt when bytesAvailable() returned >0 but trying to read the bytes would instead close the device. In fact, that was the "normal" way to finish the reading from stdin: We would create a garbage event header by trying to read past the end of the input and then signal an error because of that. Now we can actually listen for aboutToClose() on PerfStdin and we will eventually receive that signal. Change-Id: Ib159d186f4ce6c69a1fc7fe37bd5673981307b68 Reviewed-by: Milian Wolff --- app/main.cpp | 4 +- app/perfstdin.cpp | 100 +++++++++++++++++++++++++++++---- app/perfstdin.h | 20 +++++++ tests/auto/auto.pro | 3 +- tests/auto/auto.qbs | 2 +- tests/auto/perfstdin/perfstdin.pro | 17 ++++++ tests/auto/perfstdin/perfstdin.qbs | 11 ++++ tests/auto/perfstdin/tst_perfstdin.cpp | 87 ++++++++++++++++++++++++++++ 8 files changed, 230 insertions(+), 14 deletions(-) create mode 100644 tests/auto/perfstdin/perfstdin.pro create mode 100644 tests/auto/perfstdin/perfstdin.qbs create mode 100644 tests/auto/perfstdin/tst_perfstdin.cpp diff --git a/app/main.cpp b/app/main.cpp index 04346ff..11257ca 100644 --- a/app/main.cpp +++ b/app/main.cpp @@ -282,6 +282,7 @@ int main(int argc, char *argv[]) return; } + QObject::connect(infile.data(), &QIODevice::aboutToClose, &data, &PerfData::finishReading); QObject::connect(infile.data(), &QIODevice::readyRead, &data, &PerfData::read); if (infile->bytesAvailable() > 0) data.read(); @@ -307,7 +308,8 @@ int main(int argc, char *argv[]) } else { if (!infile->open(QIODevice::ReadOnly)) return CannotOpen; - QMetaObject::invokeMethod(&header, "read", Qt::QueuedConnection); + if (qobject_cast(infile.data())) // We don't get readyRead then ... + QTimer::singleShot(0, &header, &PerfHeader::read); } return app.exec(); diff --git a/app/perfstdin.cpp b/app/perfstdin.cpp index c5af9b4..af091eb 100644 --- a/app/perfstdin.cpp +++ b/app/perfstdin.cpp @@ -22,30 +22,59 @@ #include +#include #include #include +PerfStdin::PerfStdin(QObject *parent) : QIODevice(parent) +{ + connect(&m_timer, &QTimer::timeout, this, &PerfStdin::receiveData); +} + +PerfStdin::~PerfStdin() +{ + if (isOpen()) + close(); +} + bool PerfStdin::open(QIODevice::OpenMode mode) { if (!(mode & QIODevice::ReadOnly) || (mode & QIODevice::WriteOnly)) return false; - return QIODevice::open(mode); + if (QIODevice::open(mode)) { + m_buffer.resize(s_minBufferSize); + m_timer.start(); + return true; + } else { + return false; + } +} + +void PerfStdin::close() +{ + m_timer.stop(); + QIODevice::close(); } qint64 PerfStdin::readData(char *data, qint64 maxlen) { if (maxlen <= 0) return 0; - size_t read = fread(data, 1, static_cast(maxlen), stdin); - if (feof(stdin) || ferror(stdin)) - QTimer::singleShot(0, this, &QIODevice::close); - if (read == 0) { - return -1; - } else { - Q_ASSERT(read <= static_cast(maxlen)); - return static_cast(read); - } + + qint64 read = 0; + do { + Q_ASSERT(m_buffer.length() >= m_bufferPos); + Q_ASSERT(m_buffer.length() >= m_bufferUsed); + Q_ASSERT(m_bufferPos <= m_bufferUsed); + const size_t buffered = static_cast(qMin(bufferedAvailable(), maxlen - read)); + memcpy(data + read, m_buffer.constData() + m_bufferPos, buffered); + m_bufferPos += buffered; + read += buffered; + } while (read < maxlen && fillBuffer(maxlen) > 0); + + Q_ASSERT(read > 0 || bufferedAvailable() == 0); + return (read == 0 && stdinAtEnd()) ? -1 : read; } qint64 PerfStdin::writeData(const char *data, qint64 len) @@ -55,6 +84,55 @@ qint64 PerfStdin::writeData(const char *data, qint64 len) return -1; } +void PerfStdin::receiveData() +{ + if (fillBuffer() > 0) + emit readyRead(); + else if (stdinAtEnd()) + close(); +} + +void PerfStdin::resizeBuffer(int newSize) +{ + QByteArray newBuffer(newSize, Qt::Uninitialized); + std::memcpy(newBuffer.data(), m_buffer.data() + m_bufferPos, + static_cast(m_bufferUsed - m_bufferPos)); + qSwap(m_buffer, newBuffer); + m_bufferUsed -= m_bufferPos; + Q_ASSERT(m_buffer.length() >= m_bufferUsed); + m_bufferPos = 0; +} + +qint64 PerfStdin::fillBuffer(qint64 targetBufferSize) +{ + if (m_bufferUsed == m_bufferPos) + m_bufferPos = m_bufferUsed = 0; + + targetBufferSize = qMin(targetBufferSize, static_cast(s_maxBufferSize)); + if (targetBufferSize > m_buffer.length()) + resizeBuffer(static_cast(targetBufferSize)); + + if (m_bufferUsed == m_buffer.length()) { + if (m_bufferPos == 0) { + resizeBuffer(m_bufferUsed <= s_maxBufferSize / 2 ? m_bufferUsed * 2 + : s_maxBufferSize); + } else { + resizeBuffer(m_bufferUsed); + } + } + + const size_t read = fread(m_buffer.data() + m_bufferUsed, 1, + static_cast(m_buffer.length() - m_bufferUsed), stdin); + m_bufferUsed += read; + Q_ASSERT(m_buffer.length() >= m_bufferUsed); + return static_cast(read); +} + +bool PerfStdin::stdinAtEnd() const +{ + return feof(stdin) || ferror(stdin); +} + bool PerfStdin::isSequential() const { return true; @@ -62,5 +140,5 @@ bool PerfStdin::isSequential() const qint64 PerfStdin::bytesAvailable() const { - return isOpen() ? std::numeric_limits::max() : 0; + return bufferedAvailable() + QIODevice::bytesAvailable(); } diff --git a/app/perfstdin.h b/app/perfstdin.h index 4b4d723..afab1a1 100644 --- a/app/perfstdin.h +++ b/app/perfstdin.h @@ -21,15 +21,35 @@ #pragma once #include +#include class PerfStdin : public QIODevice { public: + PerfStdin(QObject *parent = nullptr); + ~PerfStdin(); + bool open(OpenMode mode) override; + void close() override; bool isSequential() const override; qint64 bytesAvailable() const override; protected: qint64 readData(char *data, qint64 maxlen) override; qint64 writeData(const char *data, qint64 len) override; + +private: + static const int s_minBufferSize = 1 << 10; + static const int s_maxBufferSize = 1 << 30; + + void receiveData(); + void resizeBuffer(int newSize); + qint64 fillBuffer(qint64 targetSize = -1); + qint64 bufferedAvailable() const { return m_bufferUsed - m_bufferPos; } + bool stdinAtEnd() const; + + QTimer m_timer; + QByteArray m_buffer; + int m_bufferPos = 0; + int m_bufferUsed = 0; }; diff --git a/tests/auto/auto.pro b/tests/auto/auto.pro index dea8bf5..f0ebef9 100644 --- a/tests/auto/auto.pro +++ b/tests/auto/auto.pro @@ -2,6 +2,7 @@ TEMPLATE = subdirs SUBDIRS = \ elfmap \ kallsyms \ - perfdata + perfdata \ + perfstdin OTHER_FILES += auto.qbs diff --git a/tests/auto/auto.qbs b/tests/auto/auto.qbs index 08ea2fe..e3d4026 100644 --- a/tests/auto/auto.qbs +++ b/tests/auto/auto.qbs @@ -4,6 +4,6 @@ Project { name: "PerfParserAutotests" condition: project.withAutotests references: [ - "elfmap", "kallsyms", "perfdata" + "elfmap", "kallsyms", "perfdata", "perfstdin" ] } diff --git a/tests/auto/perfstdin/perfstdin.pro b/tests/auto/perfstdin/perfstdin.pro new file mode 100644 index 0000000..e4b0074 --- /dev/null +++ b/tests/auto/perfstdin/perfstdin.pro @@ -0,0 +1,17 @@ +QT += testlib +QT -= gui + +CONFIG += testcase strict_flags warn_on + +INCLUDEPATH += ../../../app + +TARGET = tst_perfstdin + +SOURCES += \ + tst_perfstdin.cpp \ + ../../../app/perfstdin.cpp + +HEADERS += \ + ../../../app/perfstdin.h + +OTHER_FILES += perfstdin.qbs diff --git a/tests/auto/perfstdin/perfstdin.qbs b/tests/auto/perfstdin/perfstdin.qbs new file mode 100644 index 0000000..a36665c --- /dev/null +++ b/tests/auto/perfstdin/perfstdin.qbs @@ -0,0 +1,11 @@ +import qbs + +QtcAutotest { + name: "PerfStdin Autotest" + files: [ + "tst_perfstdin.cpp", + "../../../app/perfstdin.cpp", + "../../../app/perfstdin.h" + ] + cpp.includePaths: base.concat(["../../../app"]) +} diff --git a/tests/auto/perfstdin/tst_perfstdin.cpp b/tests/auto/perfstdin/tst_perfstdin.cpp new file mode 100644 index 0000000..77f7323 --- /dev/null +++ b/tests/auto/perfstdin/tst_perfstdin.cpp @@ -0,0 +1,87 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd +** Contact: http://www.qt.io/licensing/ +** +** This file is part of the Qt Enterprise Perf Profiler Add-on. +** +** GNU General Public License Usage +** This file may be used under the terms of the GNU General Public License +** version 3 as published by the Free Software Foundation and appearing in +** the file LICENSE.GPLv3 included in the packaging of this file. Please +** review the following information to ensure the GNU General Public License +** requirements will be met: https://www.gnu.org/licenses/gpl.html. +** +** If you have questions regarding the use of this file, please use +** contact form at http://www.qt.io/contact-us +** +****************************************************************************/ + +#include "perfstdin.h" + +#include +#include +#include + +#include + +class TestPerfstdin : public QObject +{ + Q_OBJECT + +private slots: + void testReadSelf(); +}; + +void TestPerfstdin::testReadSelf() +{ + std::freopen(QCoreApplication::applicationFilePath().toUtf8().constData(), "rb", stdin); + QTemporaryFile tempfile; + QVERIFY(tempfile.open()); + PerfStdin device; + int i = 0; + + auto doRead = [&](){ + QVERIFY(device.bytesAvailable() > 0); + qint64 r = device.bytesAvailable() + ((++i) % 32) - 16; + const QByteArray data = device.read(r); + qint64 pos = 0; + while (pos < data.length()) { + qint64 written = tempfile.write(data.data() + pos, data.length() - pos); + QVERIFY(written >= 0); + pos += written; + } + QCOMPARE(pos, data.length()); + }; + + QObject::connect(&device, &QIODevice::readyRead, &tempfile, doRead); + + QObject::connect(&device, &QIODevice::aboutToClose, &tempfile, [&](){ + while (device.bytesAvailable() > 0) + doRead(); + while (tempfile.bytesToWrite() > 0) + QVERIFY(tempfile.flush()); + + QFile self(QCoreApplication::applicationFilePath()); + QCOMPARE(tempfile.pos(), self.size()); + + QVERIFY(tempfile.reset()); + QVERIFY(self.open(QIODevice::ReadOnly)); + char c1, c2; + while (!self.atEnd()) { + QVERIFY(self.getChar(&c1)); + QVERIFY(tempfile.getChar(&c2)); + QCOMPARE(c1, c2); + } + QVERIFY(self.atEnd()); + QVERIFY(tempfile.atEnd()); + tempfile.close(); + }); + + QVERIFY(device.open(QIODevice::ReadOnly)); + QTRY_VERIFY(!tempfile.isOpen()); +} + +QTEST_MAIN(TestPerfstdin) + +#include "tst_perfstdin.moc" -- cgit v1.2.3