Skip to content

Commit 456c8d6

Browse files
laanwjlaanwj
committed
Merge #313: qt: Optimize string concatenation by default
a02c970 qt, refactor: Revert explicit including QStringBuilder (Hennadii Stepanov) 3fd3a0f qt, build: Optimize string concatenation (Hennadii Stepanov) Pull request description: From [Qt docs](https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction): > ... multiple uses of the \[`QString`\] '+' operator usually means multiple memory allocations. When concatenating n substrings, where n > 2, there can be as many as n - 1 calls to the memory allocator. With this PR > ... the '+' will automatically be performed as the `QStringBuilder` '%' everywhere. The change in the `src/Makefile.qt.include` file does not justify submitting this PR into the main repo, IMHO. ACKs for top commit: laanwj: Code review ACK a02c970 Talkless: utACK a02c970, built successfully on Debian Sid with Qt 5.15.2, but did not check if any displayed strings are "wrong" after refactoring. jarolrod: ACK a02c970 Tree-SHA512: cbb476ee96f27c3bd6e125efab74d8bf24bbdb4c30576b3feea45e203405f3bf5b497dd7d3e11361fc825fcbf4b893b152921a9efdeaf73b42d1865d85f0ae84
2 parents ecddd12 + a02c970 commit 456c8d6

File tree

9 files changed

+31
-19
lines changed

9 files changed

+31
-19
lines changed

‎src/Makefile.qt.include‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ RES_ANIMATION = $(wildcard $(srcdir)/qt/res/animation/spinner-*.png)
288288

289289
BITCOIN_RC = qt/res/bitcoin-qt-res.rc
290290

291-
BITCOIN_QT_INCLUDES = -DQT_NO_KEYWORDS
291+
BITCOIN_QT_INCLUDES = -DQT_NO_KEYWORDS -DQT_USE_QSTRINGBUILDER
292292

293293
qt_libbitcoinqt_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_INCLUDES) \
294294
$(QT_INCLUDES) $(QT_DBUS_INCLUDES) $(QR_CFLAGS)

‎src/qt/bitcoin.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
#include <QLocale>
5151
#include <QMessageBox>
5252
#include <QSettings>
53-
#include <QStringBuilder>
5453
#include <QThread>
5554
#include <QTimer>
5655
#include <QTranslator>
@@ -419,8 +418,8 @@ void BitcoinApplication::handleRunawayException(const QString &message)
419418
{
420419
QMessageBox::critical(
421420
nullptr, tr("Runaway exception"),
422-
tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) %
423-
QLatin1String("<br><br>") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT));
421+
tr("A fatal error occurred. %1 can no longer continue safely and will quit.").arg(PACKAGE_NAME) +
422+
QLatin1String("<br><br>") + GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT));
424423
::exit(EXIT_FAILURE);
425424
}
426425

@@ -430,8 +429,8 @@ void BitcoinApplication::handleNonFatalException(const QString& message)
430429
QMessageBox::warning(
431430
nullptr, tr("Internal error"),
432431
tr("An internal error occurred. %1 will attempt to continue safely. This is "
433-
"an unexpected bug which can be reported as described below.").arg(PACKAGE_NAME) %
434-
QLatin1String("<br><br>") % GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT));
432+
"an unexpected bug which can be reported as described below.").arg(PACKAGE_NAME) +
433+
QLatin1String("<br><br>") + GUIUtil::MakeHtmlLink(message, PACKAGE_BUGREPORT));
435434
}
436435

437436
WId BitcoinApplication::getMainWinId() const

‎src/qt/guiutil.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
#include <QShortcut>
5757
#include <QSize>
5858
#include <QString>
59-
#include <QStringBuilder>
6059
#include <QTextDocument> // for Qt::mightBeRichText
6160
#include <QThread>
6261
#include <QUrlQuery>
@@ -908,7 +907,7 @@ QString MakeHtmlLink(const QString& source, const QString& link)
908907
{
909908
return QString(source).replace(
910909
link,
911-
QLatin1String("<a href=\"") % link % QLatin1String("\">") % link % QLatin1String("</a>"));
910+
QLatin1String("<a href=\"") + link + QLatin1String("\">") + link + QLatin1String("</a>"));
912911
}
913912

914913
void PrintSlotException(

‎src/qt/optionsmodel.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <validation.h> // For DEFAULT_SCRIPTCHECK_THREADS
2222

2323
#include <QDebug>
24+
#include <QLatin1Char>
2425
#include <QSettings>
2526
#include <QStringList>
2627

@@ -244,7 +245,7 @@ static ProxySetting GetProxySetting(QSettings &settings, const QString &name)
244245

245246
static void SetProxySetting(QSettings &settings, const QString &name, const ProxySetting &ip_port)
246247
{
247-
settings.setValue(name, ip_port.ip + ":" + ip_port.port);
248+
settings.setValue(name, QString{ip_port.ip + QLatin1Char(':') + ip_port.port});
248249
}
249250

250251
static const QString GetDefaultProxyAddress()

‎src/qt/peertablemodel.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const
114114
return (qint64)rec->nodeStats.nodeid;
115115
case Address:
116116
// prepend to peer address down-arrow symbol for inbound connection and up-arrow for outbound connection
117-
return QString(rec->nodeStats.fInbound ? "↓ " : "↑ ") + QString::fromStdString(rec->nodeStats.addrName);
117+
return QString::fromStdString((rec->nodeStats.fInbound ? "↓ " : "↑ ") + rec->nodeStats.addrName);
118118
case ConnectionType:
119119
return GUIUtil::ConnectionTypeToQString(rec->nodeStats.m_conn_type, /* prepend_direction */ false);
120120
case Network:

‎src/qt/recentrequeststablemodel.cpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
#include <utility>
1616

17+
#include <QLatin1Char>
18+
#include <QLatin1String>
19+
1720
RecentRequestsTableModel::RecentRequestsTableModel(WalletModel *parent) :
1821
QAbstractTableModel(parent), walletModel(parent)
1922
{
@@ -124,7 +127,11 @@ void RecentRequestsTableModel::updateAmountColumnTitle()
124127
/** Gets title for amount column including current display unit if optionsModel reference available. */
125128
QString RecentRequestsTableModel::getAmountTitle()
126129
{
127-
return (this->walletModel->getOptionsModel() != nullptr) ? tr("Requested") + " ("+BitcoinUnits::shortName(this->walletModel->getOptionsModel()->getDisplayUnit()) + ")" : "";
130+
if (!walletModel->getOptionsModel()) return {};
131+
return tr("Requested") +
132+
QLatin1String(" (") +
133+
BitcoinUnits::shortName(this->walletModel->getOptionsModel()->getDisplayUnit()) +
134+
QLatin1Char(')');
128135
}
129136

130137
QModelIndex RecentRequestsTableModel::index(int row, int column, const QModelIndex &parent) const

‎src/qt/test/wallettests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ void TestGUI(interfaces::Node& node)
235235

236236
QCOMPARE(uri.count("amount=0.00000001"), 2);
237237
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_tag")->text(), QString("Amount:"));
238-
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_content")->text(), QString("0.00000001 ") + QString::fromStdString(CURRENCY_UNIT));
238+
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("amount_content")->text(), QString::fromStdString("0.00000001 " + CURRENCY_UNIT));
239239

240240
QCOMPARE(uri.count("label=TEST_LABEL_1"), 2);
241241
QCOMPARE(receiveRequestDialog->QObject::findChild<QLabel*>("label_tag")->text(), QString("Label:"));

‎src/qt/transactiondesc.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
#include <stdint.h>
2727
#include <string>
2828

29+
#include <QLatin1String>
30+
2931
QString TransactionDesc::FormatTxStatus(const interfaces::WalletTx& wtx, const interfaces::WalletTxStatus& status, bool inMempool, int numBlocks)
3032
{
3133
if (!status.is_final)
@@ -38,14 +40,16 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTx& wtx, const i
3840
else
3941
{
4042
int nDepth = status.depth_in_main_chain;
41-
if (nDepth < 0)
43+
if (nDepth < 0) {
4244
return tr("conflicted with a transaction with %1 confirmations").arg(-nDepth);
43-
else if (nDepth == 0)
44-
return tr("0/unconfirmed, %1").arg((inMempool ? tr("in memory pool") : tr("not in memory pool"))) + (status.is_abandoned ? ", "+tr("abandoned") : "");
45-
else if (nDepth < 6)
45+
} else if (nDepth == 0) {
46+
const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
47+
return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
48+
} else if (nDepth < 6) {
4649
return tr("%1/unconfirmed").arg(nDepth);
47-
else
50+
} else {
4851
return tr("%1 confirmations").arg(nDepth);
52+
}
4953
}
5054
}
5155

‎src/qt/transactiontablemodel.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#include <QDateTime>
2626
#include <QDebug>
2727
#include <QIcon>
28+
#include <QLatin1Char>
29+
#include <QLatin1String>
2830
#include <QList>
2931

3032

@@ -413,9 +415,9 @@ QVariant TransactionTableModel::txAddressDecoration(const TransactionRecord *wtx
413415
QString TransactionTableModel::formatTxToAddress(const TransactionRecord *wtx, bool tooltip) const
414416
{
415417
QString watchAddress;
416-
if (tooltip) {
418+
if (tooltip && wtx->involvesWatchAddress) {
417419
// Mark transactions involving watch-only addresses by adding " (watch-only)"
418-
watchAddress = wtx->involvesWatchAddress ? QString(" (") + tr("watch-only") + QString(")") : "";
420+
watchAddress = QLatin1String(" (") + tr("watch-only") + QLatin1Char(')');
419421
}
420422

421423
switch(wtx->type)

0 commit comments

Comments
 (0)