From 51ed77637a6001f66805e8091ce8616d8f2530db Mon Sep 17 00:00:00 2001 From: mread Date: Thu, 6 Oct 2011 15:07:29 +0100 Subject: Removing accidental use of double instead of qreal from QLine QLine contained a number of places where conversions and calculations were done as double where qreal should have been used instead. This is caused by the use of raw floating point literals. The fix is to construct the literals as qreal. The angle conversions are simplified to reduce the risk of a growing loss of precision from this change. On Symbian, where qreal is float, this gives a 5-10% performance improvement to the affected functions, as tested by a new benchmark test. The auto tests pass, these contain precision tests which indicates that there is no significant loss of precision with this change. On Windows, where qreal is double, this has no significant effect on performance. Task-number: QTBUG-4894 Reviewed-by: Shane Kearns --- src/corelib/tools/qline.cpp | 21 ++-- tests/benchmarks/corelib/tools/qline/main.cpp | 157 +++++++++++++++++++++++++ tests/benchmarks/corelib/tools/qline/qline.pro | 12 ++ tests/benchmarks/corelib/tools/tools.pro | 1 + 4 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 tests/benchmarks/corelib/tools/qline/main.cpp create mode 100644 tests/benchmarks/corelib/tools/qline/qline.pro diff --git a/src/corelib/tools/qline.cpp b/src/corelib/tools/qline.cpp index 0f67652..9c7c243 100644 --- a/src/corelib/tools/qline.cpp +++ b/src/corelib/tools/qline.cpp @@ -302,10 +302,15 @@ QDataStream &operator>>(QDataStream &stream, QLine &line) #endif // QT_NO_DATASTREAM +inline static qreal q_deg2rad(qreal x) +{ + return x * qreal(0.01745329251994329576923690768489); /* pi/180 */ +} -#ifndef M_2PI -#define M_2PI 6.28318530717958647692528676655900576 -#endif +inline static qreal q_rad2deg(qreal x) +{ + return x * qreal(57.295779513082320876798154814105); /* 180/pi */ +} /*! \class QLineF @@ -575,7 +580,7 @@ qreal QLineF::angle() const const qreal dx = pt2.x() - pt1.x(); const qreal dy = pt2.y() - pt1.y(); - const qreal theta = qAtan2(-dy, dx) * 360.0 / M_2PI; + const qreal theta = q_rad2deg(qAtan2(-dy, dx)); const qreal theta_normalized = theta < 0 ? theta + 360 : theta; @@ -599,7 +604,7 @@ qreal QLineF::angle() const */ void QLineF::setAngle(qreal angle) { - const qreal angleR = angle * M_2PI / 360.0; + const qreal angleR = q_deg2rad(angle); const qreal l = length(); const qreal dx = qCos(angleR) * l; @@ -621,7 +626,7 @@ void QLineF::setAngle(qreal angle) */ QLineF QLineF::fromPolar(qreal length, qreal angle) { - const qreal angleR = angle * M_2PI / 360.0; + const qreal angleR = q_deg2rad(angle); return QLineF(0, 0, qCos(angleR) * length, -qSin(angleR) * length); } @@ -815,8 +820,8 @@ qreal QLineF::angle(const QLineF &l) const qreal cos_line = (dx()*l.dx() + dy()*l.dy()) / (length()*l.length()); qreal rad = 0; // only accept cos_line in the range [-1,1], if it is outside, use 0 (we return 0 rather than PI for those cases) - if (cos_line >= -1.0 && cos_line <= 1.0) rad = qAcos( cos_line ); - return rad * 360 / M_2PI; + if (cos_line >= qreal(-1.0) && cos_line <= qreal(1.0)) rad = qAcos( cos_line ); + return q_rad2deg(rad); } diff --git a/tests/benchmarks/corelib/tools/qline/main.cpp b/tests/benchmarks/corelib/tools/qline/main.cpp new file mode 100644 index 0000000..d7f93ba --- /dev/null +++ b/tests/benchmarks/corelib/tools/qline/main.cpp @@ -0,0 +1,157 @@ +/**************************************************************************** +** +** Copyright (C) 2011 Nokia Corporation and/or its subsidiary(-ies). +** All rights reserved. +** Contact: Nokia Corporation (qt-info@nokia.com) +** +** This file is part of the test suite of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** GNU Lesser General Public License Usage +** This file may be used under the terms of the GNU Lesser General Public +** License version 2.1 as published by the Free Software Foundation and +** appearing in the file LICENSE.LGPL included in the packaging of this +** file. Please review the following information to ensure the GNU Lesser +** General Public License version 2.1 requirements will be met: +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU General +** Public License version 3.0 as published by the Free Software Foundation +** and appearing in the file LICENSE.GPL included in the packaging of this +** file. Please review the following information to ensure the GNU General +** Public License version 3.0 requirements will be met: +** http://www.gnu.org/copyleft/gpl.html. +** +** Other Usage +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ +// This file contains benchmarks for QLineF functions. + +#include +#include +#include + +class tst_qline : public QObject +{ + Q_OBJECT +private slots: + void fromPolar(); + void intersect_data(); + void intersect(); + void length(); + void setLength(); + void angle(); + void setAngle(); + void angleTo(); + void unitVector(); + void normalVector(); + void angle2(); +}; + +void tst_qline::fromPolar() +{ + QBENCHMARK { + QLineF::fromPolar(10, 2); + } +} + +void tst_qline::intersect_data() +{ + QTest::addColumn("l1"); + QTest::addColumn("l2"); + QTest::newRow("cross") << QLineF(-1,-1,1,1) << QLineF(-1,1,1,-1); + QTest::newRow("miss") << QLineF(1,1,2,2) << QLineF(1,11,2,12); +} + +void tst_qline::intersect() +{ + QFETCH(QLineF, l1); + QFETCH(QLineF, l2); + QPointF intersection; + QBENCHMARK { + l1.intersect(l2, &intersection); + } +} + +void tst_qline::length() +{ + QLineF line(1,2,3,4); + QBENCHMARK { + line.length(); + } +} + +void tst_qline::setLength() +{ + QLineF line(1,2,3,4); + QBENCHMARK { + line.setLength(5); + } +} + +void tst_qline::angle() +{ + QLineF line(1,2,3,4); + QBENCHMARK { + line.angle(); + } +} + +void tst_qline::setAngle() +{ + QLineF line(1,2,3,4); + QBENCHMARK { + line.setAngle(1); + } +} + +void tst_qline::angleTo() +{ + QLineF line1(1,2,3,4); + QLineF line2(8,7,6,5); + QBENCHMARK { + line1.angleTo(line2); + } +} + +void tst_qline::unitVector() +{ + QLineF line(1,2,3,4); + QBENCHMARK { + line.unitVector(); + } +} + +void tst_qline::normalVector() +{ + QLineF line(1,2,3,4); + QBENCHMARK { + line.normalVector(); + } +} + +void tst_qline::angle2() +{ + QLineF line1(1,2,3,4); + QLineF line2(8,7,6,5); + QBENCHMARK { + line1.angle(line2); + } +} + +QTEST_MAIN(tst_qline) + +#include "main.moc" diff --git a/tests/benchmarks/corelib/tools/qline/qline.pro b/tests/benchmarks/corelib/tools/qline/qline.pro new file mode 100644 index 0000000..ae6bf5e --- /dev/null +++ b/tests/benchmarks/corelib/tools/qline/qline.pro @@ -0,0 +1,12 @@ +load(qttest_p4) +TEMPLATE = app +TARGET = tst_bench_qline +DEPENDPATH += . +INCLUDEPATH += . + +QT -= gui + +CONFIG += release + +# Input +SOURCES += main.cpp diff --git a/tests/benchmarks/corelib/tools/tools.pro b/tests/benchmarks/corelib/tools/tools.pro index 44e8973..83a9411 100644 --- a/tests/benchmarks/corelib/tools/tools.pro +++ b/tests/benchmarks/corelib/tools/tools.pro @@ -3,6 +3,7 @@ SUBDIRS = \ containers-associative \ containers-sequential \ qbytearray \ + qline \ qlist \ qrect \ qregexp \ -- cgit v0.12 From 7e662f3727e7c3dd3c41c29ed49bc41d2b66c744 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Thu, 6 Oct 2011 15:40:34 +0100 Subject: Fix construction races in QtNetwork When two threads construct a QNetworkAccessManager at exactly the same time on an SMP system, there are construction races for some Q_GLOBAL_STATIC data. This is normal and expected - the losing thread deletes its instance as part of the Q_GLOBAL_STATIC macro. However, for two of the classes, destruction of the loser had side effects. For QNetworkConfigurationMangerPrivate, there was a crash because of uninitialised variable on the losing side. For QNetworkAccessBackendFactoryData, a guard mechanism intended to prevent the data being reconstructed by destructors of other global static classes was being set by the loser. To fix this, the bool is changed to a QAtomicInt. In the normal case, it will have value 0->1 on startup and 1->0 on shutdown. In the race case, it will have values 0->1->2->1 on startup and 1->0 on shutdown. Task-Number: QTBUG-20343 Reviewed-By: mread --- src/network/access/qnetworkaccessbackend.cpp | 15 ++++++++++----- src/network/bearer/qnetworkconfigmanager_p.cpp | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/network/access/qnetworkaccessbackend.cpp b/src/network/access/qnetworkaccessbackend.cpp index 2fae7d6..88c45d1 100644 --- a/src/network/access/qnetworkaccessbackend.cpp +++ b/src/network/access/qnetworkaccessbackend.cpp @@ -57,20 +57,25 @@ QT_BEGIN_NAMESPACE -static bool factoryDataShutdown = false; class QNetworkAccessBackendFactoryData: public QList { public: - QNetworkAccessBackendFactoryData() : mutex(QMutex::Recursive) { } + QNetworkAccessBackendFactoryData() : mutex(QMutex::Recursive) + { + valid.ref(); + } ~QNetworkAccessBackendFactoryData() { QMutexLocker locker(&mutex); // why do we need to lock? - factoryDataShutdown = true; + valid.deref(); } QMutex mutex; + //this is used to avoid (re)constructing factory data from destructors of other global classes + static QAtomicInt valid; }; Q_GLOBAL_STATIC(QNetworkAccessBackendFactoryData, factoryData) +QAtomicInt QNetworkAccessBackendFactoryData::valid; QNetworkAccessBackendFactory::QNetworkAccessBackendFactory() { @@ -80,7 +85,7 @@ QNetworkAccessBackendFactory::QNetworkAccessBackendFactory() QNetworkAccessBackendFactory::~QNetworkAccessBackendFactory() { - if (!factoryDataShutdown) { + if (QNetworkAccessBackendFactoryData::valid) { QMutexLocker locker(&factoryData()->mutex); factoryData()->removeAll(this); } @@ -89,7 +94,7 @@ QNetworkAccessBackendFactory::~QNetworkAccessBackendFactory() QNetworkAccessBackend *QNetworkAccessManagerPrivate::findBackend(QNetworkAccessManager::Operation op, const QNetworkRequest &request) { - if (!factoryDataShutdown) { + if (QNetworkAccessBackendFactoryData::valid) { QMutexLocker locker(&factoryData()->mutex); QNetworkAccessBackendFactoryData::ConstIterator it = factoryData()->constBegin(), end = factoryData()->constEnd(); diff --git a/src/network/bearer/qnetworkconfigmanager_p.cpp b/src/network/bearer/qnetworkconfigmanager_p.cpp index 18e29af..96a534d 100644 --- a/src/network/bearer/qnetworkconfigmanager_p.cpp +++ b/src/network/bearer/qnetworkconfigmanager_p.cpp @@ -60,7 +60,7 @@ Q_GLOBAL_STATIC_WITH_ARGS(QFactoryLoader, loader, #endif QNetworkConfigurationManagerPrivate::QNetworkConfigurationManagerPrivate() - : QObject(), pollTimer(0), mutex(QMutex::Recursive), forcedPolling(0), firstUpdate(true) + : QObject(), pollTimer(0), bearerThread(0), mutex(QMutex::Recursive), forcedPolling(0), firstUpdate(true) { qRegisterMetaType("QNetworkConfiguration"); qRegisterMetaType("QNetworkConfigurationPrivatePointer"); -- cgit v0.12