From dcf41b5f69d0e199d0a6a8faaad981f73e1c9aa0 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Tue, 6 Nov 2018 13:35:56 +0100 Subject: [PATCH 1/5] Yarr: Fix developer build with MinGW 64 Use the MinGW/Windows runtime specific printf formats for size_t, formats, fixing: 3rdparty\masm\yarr\YarrInterpreter.cpp: In lambda function: 3rdparty\masm\yarr\YarrInterpreter.cpp:2128:24: error: unknown conversion type character 'z' in format [-Werror=format=] out.printf("%4zu", index); ^~~~~~ 3rdparty\masm\yarr\YarrInterpreter.cpp:2128:24: error: too many arguments for format [-Werror=format-extra-args] cc1plus.exe: all warnings being treated as errors Change-Id: I9d39c51b907fc7834aaf353dd0ce8095852f9b3e Reviewed-by: Lars Knoll --- src/3rdparty/masm/yarr/YarrInterpreter.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/3rdparty/masm/yarr/YarrInterpreter.cpp b/src/3rdparty/masm/yarr/YarrInterpreter.cpp index 6eb6750dc4..4d3652fcbc 100644 --- a/src/3rdparty/masm/yarr/YarrInterpreter.cpp +++ b/src/3rdparty/masm/yarr/YarrInterpreter.cpp @@ -2125,7 +2125,15 @@ public: auto outputTermIndexAndNest = [&](size_t index, unsigned termNesting) { for (unsigned nestingDepth = 0; nestingDepth < termIndexNest; nestingDepth++) out.print(" "); +#if defined(WIN32) && defined(__MINGW32__) +# if __SIZEOF_POINTER__ == 8 + out.printf("%4I64u", index); +# else + out.printf("%4I32u", index); +# endif +#else out.printf("%4zu", index); +#endif for (unsigned nestingDepth = 0; nestingDepth < termNesting; nestingDepth++) out.print(" "); }; From 8af9d69554065924f9c92b29dd09d08ba6650328 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 1 Nov 2018 11:08:26 +0100 Subject: [PATCH 2/5] QML Tooling: Fix ordering of memory events in V4 profiler adapter We should not send memory events that are chronologically after the next call event, even if the time threshold given by the profiler service would allow us to do so. When the remaining call events are sent, the chronological order would otherwise be violated. Fixes: QTBUG-71515 Change-Id: Iee27304f836a899b2b35133316cecd3d34f128c6 Reviewed-by: Michael Brasser --- .../qmldbg_profiler/qv4profileradapter.cpp | 7 +++++-- .../data/batchOverflow.qml | 20 +++++++++++++++++++ .../qqmlprofilerservice.pro | 3 ++- .../tst_qqmlprofilerservice.cpp | 10 ++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 tests/auto/qml/debugger/qqmlprofilerservice/data/batchOverflow.qml diff --git a/src/plugins/qmltooling/qmldbg_profiler/qv4profileradapter.cpp b/src/plugins/qmltooling/qmldbg_profiler/qv4profileradapter.cpp index e4f2f556fc..12c36f3dd6 100644 --- a/src/plugins/qmltooling/qmldbg_profiler/qv4profileradapter.cpp +++ b/src/plugins/qmltooling/qmldbg_profiler/qv4profileradapter.cpp @@ -87,14 +87,17 @@ qint64 QV4ProfilerAdapter::appendMemoryEvents(qint64 until, QList &m qint64 QV4ProfilerAdapter::finalizeMessages(qint64 until, QList &messages, qint64 callNext, QQmlDebugPacket &d) { + qint64 memoryNext = -1; + if (callNext == -1) { m_functionLocations.clear(); m_functionCallData.clear(); m_functionCallPos = 0; + memoryNext = appendMemoryEvents(until, messages, d); + } else { + memoryNext = appendMemoryEvents(qMin(callNext, until), messages, d); } - qint64 memoryNext = appendMemoryEvents(until, messages, d); - if (memoryNext == -1) { m_memoryData.clear(); m_memoryPos = 0; diff --git a/tests/auto/qml/debugger/qqmlprofilerservice/data/batchOverflow.qml b/tests/auto/qml/debugger/qqmlprofilerservice/data/batchOverflow.qml new file mode 100644 index 0000000000..dde1def947 --- /dev/null +++ b/tests/auto/qml/debugger/qqmlprofilerservice/data/batchOverflow.qml @@ -0,0 +1,20 @@ +import QtQml 2.2 + +QtObject { + function iterate(dictionaryTable, j) { + var word = "a" + j.toString() + dictionaryTable[word] = null; + } + + Component.onCompleted: { + var dictionaryTable = {}; + for (var j = 0; j < 256; ++j) + iterate(dictionaryTable, j); + } + + property Timer timer: Timer { + interval: 1 + running: true; + onTriggered: Qt.quit(); + } +} diff --git a/tests/auto/qml/debugger/qqmlprofilerservice/qqmlprofilerservice.pro b/tests/auto/qml/debugger/qqmlprofilerservice/qqmlprofilerservice.pro index 2a685ed877..0cd4b331f2 100644 --- a/tests/auto/qml/debugger/qqmlprofilerservice/qqmlprofilerservice.pro +++ b/tests/auto/qml/debugger/qqmlprofilerservice/qqmlprofilerservice.pro @@ -21,4 +21,5 @@ OTHER_FILES += \ data/javascript.qml \ data/timer.qml \ data/qstr.qml \ - data/memory.qml + data/memory.qml \ + data/batchOverflow.qml diff --git a/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp b/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp index eb0b0c2fe2..7fc43671c2 100644 --- a/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp +++ b/tests/auto/qml/debugger/qqmlprofilerservice/tst_qqmlprofilerservice.cpp @@ -225,6 +225,7 @@ private slots: void memory(); void compile(); void multiEngine(); + void batchOverflow(); private: bool m_recordFromStart = true; @@ -826,6 +827,15 @@ void tst_QQmlProfilerService::multiEngine() QCOMPARE(spy.count(), 1); } +void tst_QQmlProfilerService::batchOverflow() +{ + // The trace client checks that the events are received in order. + QCOMPARE(connect(true, "batchOverflow.qml"), ConnectSuccess); + checkProcessTerminated(); + checkTraceReceived(); + checkJsHeap(); +} + QTEST_MAIN(tst_QQmlProfilerService) #include "tst_qqmlprofilerservice.moc" From 1613e32042a8e710c3105b926422827ccaad1c5c Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 14 Nov 2018 09:55:19 +0100 Subject: [PATCH 3/5] qmlpreview: Fix typo Change-Id: I5b8a674e77f770e4987e03ccf02b5a27c4987e24 Reviewed-by: Alessandro Portale --- tools/qmlpreview/qmlpreviewapplication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/qmlpreview/qmlpreviewapplication.cpp b/tools/qmlpreview/qmlpreviewapplication.cpp index 618769502c..031381a948 100644 --- a/tools/qmlpreview/qmlpreviewapplication.cpp +++ b/tools/qmlpreview/qmlpreviewapplication.cpp @@ -105,7 +105,7 @@ void QmlPreviewApplication::parseArguments() parser.addVersionOption(); parser.addPositionalArgument(QLatin1String("program"), - tr("The program to be started and profiled."), + tr("The program to be started and previewed."), QLatin1String("[program]")); parser.addPositionalArgument(QLatin1String("parameters"), tr("Parameters for the program to be started."), From 80d26c15080c92012f0a51f7675d3c70ad56a8bd Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 13 Nov 2018 14:22:59 +0100 Subject: [PATCH 4/5] qmlpreview: Use a better file system watcher We need to keep watching files even if they are removed and re-added as editors frequently do when manipulating text files. QFileSystemWatcher cannot do this by itself, and our simplistic timeout-based attempt didn't work work properly either. Fixes: QTBUG-71768 Change-Id: I21e914138179ad8adf07f0196fec8ddcda2cbfca Reviewed-by: Alessandro Portale --- tools/qmlpreview/qmlpreview.pro | 6 +- tools/qmlpreview/qmlpreviewapplication.cpp | 28 +--- tools/qmlpreview/qmlpreviewapplication.h | 7 +- .../qmlpreviewfilesystemwatcher.cpp | 150 ++++++++++++++++++ .../qmlpreview/qmlpreviewfilesystemwatcher.h | 70 ++++++++ 5 files changed, 231 insertions(+), 30 deletions(-) create mode 100644 tools/qmlpreview/qmlpreviewfilesystemwatcher.cpp create mode 100644 tools/qmlpreview/qmlpreviewfilesystemwatcher.h diff --git a/tools/qmlpreview/qmlpreview.pro b/tools/qmlpreview/qmlpreview.pro index 7d443b5f6c..d42f2cbe07 100644 --- a/tools/qmlpreview/qmlpreview.pro +++ b/tools/qmlpreview/qmlpreview.pro @@ -3,10 +3,12 @@ CONFIG += no_import_scan SOURCES += \ main.cpp \ - qmlpreviewapplication.cpp + qmlpreviewapplication.cpp \ + qmlpreviewfilesystemwatcher.cpp HEADERS += \ - qmlpreviewapplication.h + qmlpreviewapplication.h \ + qmlpreviewfilesystemwatcher.h QMAKE_TARGET_DESCRIPTION = QML Preview diff --git a/tools/qmlpreview/qmlpreviewapplication.cpp b/tools/qmlpreview/qmlpreviewapplication.cpp index 031381a948..02f10831ec 100644 --- a/tools/qmlpreview/qmlpreviewapplication.cpp +++ b/tools/qmlpreview/qmlpreviewapplication.cpp @@ -63,9 +63,9 @@ QmlPreviewApplication::QmlPreviewApplication(int &argc, char **argv) : connect(m_qmlPreviewClient.data(), &QQmlPreviewClient::request, this, &QmlPreviewApplication::serveRequest); - connect(&m_watcher, &QFileSystemWatcher::fileChanged, + connect(&m_watcher, &QmlPreviewFileSystemWatcher::fileChanged, this, &QmlPreviewApplication::sendFile); - connect(&m_watcher, &QFileSystemWatcher::directoryChanged, + connect(&m_watcher, &QmlPreviewFileSystemWatcher::directoryChanged, this, &QmlPreviewApplication::sendDirectory); } @@ -212,17 +212,12 @@ void QmlPreviewApplication::serveRequest(const QString &path) if (info.isDir()) { m_qmlPreviewClient->sendDirectory(path, QDir(path).entryList()); - m_watcher.addPath(path); + m_watcher.addDirectory(path); } else { QFile file(path); if (file.open(QIODevice::ReadOnly)) { m_qmlPreviewClient->sendFile(path, file.readAll()); - m_watcher.addPath(path); - - // Also watch the directory, because editors will rather replace a file than change it. - // Therefore when the file changes, we can't read it, but when the file is re-added we can - // see that from the directory changing. - m_watcher.addPath(info.absolutePath()); + m_watcher.addFile(path); } else { logStatus(QString("Could not open file %1 for reading: %2").arg(path) .arg(file.errorString())); @@ -236,13 +231,10 @@ bool QmlPreviewApplication::sendFile(const QString &path) QFile file(path); if (file.open(QIODevice::ReadOnly)) { m_qmlPreviewClient->sendFile(path, file.readAll()); - m_pendingFiles.removeAll(path); // Defer the Load, because files tend to change multiple times in a row. m_loadTimer.start(); return true; } - if (!m_pendingFiles.contains(path)) - m_pendingFiles.append(path); logStatus(QString("Could not open file %1 for reading: %2").arg(path).arg(file.errorString())); return false; } @@ -250,17 +242,5 @@ bool QmlPreviewApplication::sendFile(const QString &path) void QmlPreviewApplication::sendDirectory(const QString &path) { m_qmlPreviewClient->sendDirectory(path, QDir(path).entryList()); - for (auto it = m_pendingFiles.begin(); it != m_pendingFiles.end();) { - const QString filePath = *it; - QFile file(filePath); - if (file.open(QIODevice::ReadOnly)) { - logStatus(QString("Sending replaced file %1.").arg(filePath)); - m_qmlPreviewClient->sendFile(filePath, file.readAll()); - m_watcher.addPath(filePath); - it = m_pendingFiles.erase(it); - } else { - ++it; - } - } m_loadTimer.start(); } diff --git a/tools/qmlpreview/qmlpreviewapplication.h b/tools/qmlpreview/qmlpreviewapplication.h index eb363b0eb6..7da4a9ab5c 100644 --- a/tools/qmlpreview/qmlpreviewapplication.h +++ b/tools/qmlpreview/qmlpreviewapplication.h @@ -29,13 +29,14 @@ #ifndef QMLPREVIEWAPPLICATION_H #define QMLPREVIEWAPPLICATION_H +#include "qmlpreviewfilesystemwatcher.h" + #include #include #include #include #include -#include #include @@ -71,13 +72,11 @@ private: QScopedPointer m_connection; QScopedPointer m_qmlPreviewClient; - QFileSystemWatcher m_watcher; + QmlPreviewFileSystemWatcher m_watcher; QTimer m_loadTimer; QTimer m_connectTimer; uint m_connectionAttempts; - - QStringList m_pendingFiles; }; #endif // QMLPREVIEWAPPLICATION_H diff --git a/tools/qmlpreview/qmlpreviewfilesystemwatcher.cpp b/tools/qmlpreview/qmlpreviewfilesystemwatcher.cpp new file mode 100644 index 0000000000..78a6ccead3 --- /dev/null +++ b/tools/qmlpreview/qmlpreviewfilesystemwatcher.cpp @@ -0,0 +1,150 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtQml module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:GPL-EXCEPT$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** 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-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include "qmlpreviewfilesystemwatcher.h" + +#include +#include +#include + +QmlPreviewFileSystemWatcher::QmlPreviewFileSystemWatcher(QObject *parent) : + QObject(parent), m_watcher(new QFileSystemWatcher(this)) +{ + connect(m_watcher, &QFileSystemWatcher::fileChanged, + this, &QmlPreviewFileSystemWatcher::fileChanged); + connect(m_watcher, &QFileSystemWatcher::directoryChanged, + this, &QmlPreviewFileSystemWatcher::onDirectoryChanged); +} + +bool QmlPreviewFileSystemWatcher::watchesFile(const QString &file) const +{ + return m_files.contains(file); +} + +void QmlPreviewFileSystemWatcher::addFile(const QString &file) +{ + if (watchesFile(file)) { + qWarning() << "FileSystemWatcher: File" << file << "is already being watched."; + return; + } + + QStringList toAdd(file); + m_files.insert(file); + + const QString directory = QFileInfo(file).path(); + const int dirCount = ++m_directoryCount[directory]; + Q_ASSERT(dirCount > 0); + + if (dirCount == 1) + toAdd.append(directory); + + m_watcher->addPaths(toAdd); +} + +void QmlPreviewFileSystemWatcher::removeFile(const QString &file) +{ + WatchEntrySetIterator it = m_files.find(file); + if (it == m_files.end()) { + qWarning() << "FileSystemWatcher: File" << file << "is not watched."; + return; + } + + QStringList toRemove(file); + m_files.erase(it); + m_watcher->removePath(file); + + const QString directory = QFileInfo(file).path(); + const int dirCount = --m_directoryCount[directory]; + Q_ASSERT(dirCount >= 0); + + if (!dirCount) + toRemove.append(directory); + + m_watcher->removePaths(toRemove); +} + +bool QmlPreviewFileSystemWatcher::watchesDirectory(const QString &directory) const +{ + return m_directories.contains(directory); +} + +void QmlPreviewFileSystemWatcher::addDirectory(const QString &directory) +{ + if (watchesDirectory(directory)) { + qWarning() << "FileSystemWatcher: Directory" << directory << "is already being watched."; + return; + } + + m_directories.insert(directory); + const int count = ++m_directoryCount[directory]; + Q_ASSERT(count > 0); + + if (count == 1) + m_watcher->addPath(directory); +} + +void QmlPreviewFileSystemWatcher::removeDirectory(const QString &directory) +{ + WatchEntrySetIterator it = m_directories.find(directory); + if (it == m_directories.end()) { + qWarning() << "FileSystemWatcher: Directory" << directory << "is not watched."; + return; + } + + m_directories.erase(it); + + const int count = --m_directoryCount[directory]; + Q_ASSERT(count >= 0); + + if (!count) + m_watcher->removePath(directory); +} + +void QmlPreviewFileSystemWatcher::onDirectoryChanged(const QString &path) +{ + if (m_directories.contains(path)) + emit directoryChanged(path); + + QStringList toReadd; + const QDir dir(path); + for (const QFileInfo &entry : dir.entryInfoList(QDir::Files)) { + const QString file = entry.filePath(); + if (m_files.contains(file)) + toReadd.append(file); + } + + if (!toReadd.isEmpty()) { + const QStringList remove = m_watcher->addPaths(toReadd); + for (const QString &rejected : remove) + toReadd.removeOne(rejected); + + // If we've successfully added the file, that means it was deleted and replaced. + for (const QString &reAdded : qAsConst(toReadd)) + emit fileChanged(reAdded); + } +} diff --git a/tools/qmlpreview/qmlpreviewfilesystemwatcher.h b/tools/qmlpreview/qmlpreviewfilesystemwatcher.h new file mode 100644 index 0000000000..a7ead641d8 --- /dev/null +++ b/tools/qmlpreview/qmlpreviewfilesystemwatcher.h @@ -0,0 +1,70 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtQml module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:GPL-EXCEPT$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 3 as published by the Free Software +** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +** 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-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef QMLPREVIEWFILESYSTEMWATCHER_H +#define QMLPREVIEWFILESYSTEMWATCHER_H + +#include +#include +#include + +class QmlPreviewFileSystemWatcher : public QObject +{ + Q_OBJECT + +public: + explicit QmlPreviewFileSystemWatcher(QObject *parent = nullptr); + + void addFile(const QString &file); + void removeFile(const QString &file); + bool watchesFile(const QString &file) const; + + void addDirectory(const QString &file); + void removeDirectory(const QString &file); + bool watchesDirectory(const QString &file) const; + +signals: + void fileChanged(const QString &path); + void directoryChanged(const QString &path); + +private: + using WatchEntrySet = QSet; + using WatchEntrySetIterator = WatchEntrySet::iterator; + + void onDirectoryChanged(const QString &path); + + WatchEntrySet m_files; + WatchEntrySet m_directories; + + // Directories watched either explicitly or implicitly through files contained in them. + QHash m_directoryCount; + + QFileSystemWatcher *m_watcher = nullptr; +}; + +#endif // QMLPREVIEWFILESYSTEMWATCHER_H From 89a1d4ff3f829635d80a90112f6b2d44cc274b1b Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 12 Nov 2018 09:27:33 +0100 Subject: [PATCH 5/5] masm: Don't call fclose(nullptr) and initialize statics Otherwise our disassembler crashes on QV4_SHOW_ASM. Change-Id: I63b20c0932452fe852773f91ebecaa7f31dd040d Reviewed-by: Erik Verbruggen --- src/3rdparty/masm/stubs/WTFStubs.cpp | 2 +- src/3rdparty/masm/wtf/FilePrintStream.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/3rdparty/masm/stubs/WTFStubs.cpp b/src/3rdparty/masm/stubs/WTFStubs.cpp index b26d10b3ab..f408b355f5 100644 --- a/src/3rdparty/masm/stubs/WTFStubs.cpp +++ b/src/3rdparty/masm/stubs/WTFStubs.cpp @@ -66,7 +66,7 @@ uint32_t cryptographicallyRandomNumber() return 0; } -static FilePrintStream* s_dataFile; +static FilePrintStream* s_dataFile = nullptr; void setDataFile(FilePrintStream *ps) { diff --git a/src/3rdparty/masm/wtf/FilePrintStream.cpp b/src/3rdparty/masm/wtf/FilePrintStream.cpp index a56b36526e..8ddf8487bd 100644 --- a/src/3rdparty/masm/wtf/FilePrintStream.cpp +++ b/src/3rdparty/masm/wtf/FilePrintStream.cpp @@ -39,7 +39,8 @@ FilePrintStream::~FilePrintStream() { if (m_adoptionMode == Borrow) return; - fclose(m_file); + if (m_file) + fclose(m_file); } std::unique_ptr FilePrintStream::open(const char* filename, const char* mode)