Unity 8

Merge lp:~aacid/unity8/fast_string_concatenation into lp:unity8

  1. fast_string_concatenation
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Michael Zanetti
Approved revision: 1988
Merged at revision: 2015
Proposed branch: lp:~aacid/unity8/fast_string_concatenation
Merge into: lp:unity8
Diff against target: 117 lines (+20/-15)
7 files modified
CMakeLists.txt (+3/-0)
plugins/IntegratedLightDM/UsersModel.cpp (+7/-6)
plugins/IntegratedLightDM/liblightdm/UsersModelPrivate.cpp (+2/-1)
tests/mocks/Unity/Launcher/MockQuickListModel.cpp (+1/-1)
tests/mocks/Unity/fake_resultsmodel.cpp (+1/-1)
tests/mocks/Unity/fake_scopesoverview.cpp (+1/-1)
tests/plugins/Wizard/tst_pagelist.cpp (+5/-5)
To merge this branch: bzr merge lp:~aacid/unity8/fast_string_concatenation
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+272904@code.launchpad.net

Commit message

Enable Efficient String Construction by default

See http://blog.qt.io/blog/2011/06/13/string-concatenation-with-qstringbuilder/

Description of the change

 * Are there any related MPs required for this MP to build/function as expected?
No

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * Did you make sure that your branch does not contain spurious tags?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

 * If you changed the UI, has there been a design review?
N/A

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1988
http://jenkins.qa.ubuntu.com/job/unity8-ci/6390/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/4433
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/768
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1102
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-wily/420
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/997
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/998
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/630
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3608
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4430
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/4430/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23815
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/463
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/768
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/768/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23810

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6390/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

ok. works for me. Seems like a good idea to have such optimizations in a project of this size.

 * Did you perform an exploratory manual test run of the code change and any related functionality?

nothing really changed. tests still passing

 * Did CI run pass? If not, please explain why.

yes

 * Did you make sure that the branch does not contain spurious tags?

yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2015-09-02 08:04:41 +0000
3+++ CMakeLists.txt 2015-09-30 13:12:27 +0000
4@@ -101,6 +101,9 @@
5 # Save a few container detach and conversions
6 add_definitions(-DQT_STRICT_ITERATORS)
7
8+# Use the fast string builder
9+add_definitions(-DQT_USE_QSTRINGBUILDER)
10+
11 # Autopilot tests
12 include(autopilot)
13 declare_autopilot_test(shell unity8.shell ${CMAKE_SOURCE_DIR}/tests/autopilot/)
14
15=== modified file 'plugins/IntegratedLightDM/UsersModel.cpp'
16--- plugins/IntegratedLightDM/UsersModel.cpp 2015-04-30 09:31:51 +0000
17+++ plugins/IntegratedLightDM/UsersModel.cpp 2015-09-30 13:12:27 +0000
18@@ -43,16 +43,17 @@
19
20 QVariant MangleModel::data(const QModelIndex &index, int role) const
21 {
22- QVariant data = QSortFilterProxyModel::data(index, role);
23+ QVariant variantData = QSortFilterProxyModel::data(index, role);
24
25 // If user's real name is empty, switch to unix name
26- if (role == QLightDM::UsersModel::RealNameRole && data.toString().isEmpty()) {
27- data = QSortFilterProxyModel::data(index, QLightDM::UsersModel::NameRole);
28- } else if (role == QLightDM::UsersModel::BackgroundPathRole && data.toString().startsWith('#')) {
29- data = "data:image/svg+xml,<svg><rect width='100%' height='100%' fill='" + data.toString() + "'/></svg>";
30+ if (role == QLightDM::UsersModel::RealNameRole && variantData.toString().isEmpty()) {
31+ variantData = QSortFilterProxyModel::data(index, QLightDM::UsersModel::NameRole);
32+ } else if (role == QLightDM::UsersModel::BackgroundPathRole && variantData.toString().startsWith('#')) {
33+ const QString stringData = "data:image/svg+xml,<svg><rect width='100%' height='100%' fill='" + variantData.toString() + "'/></svg>";
34+ variantData = stringData;
35 }
36
37- return data;
38+ return variantData;
39 }
40
41 // **** Now we continue with actual UsersModel class ****
42
43=== modified file 'plugins/IntegratedLightDM/liblightdm/UsersModelPrivate.cpp'
44--- plugins/IntegratedLightDM/liblightdm/UsersModelPrivate.cpp 2015-09-14 09:11:08 +0000
45+++ plugins/IntegratedLightDM/liblightdm/UsersModelPrivate.cpp 2015-09-30 13:12:27 +0000
46@@ -34,7 +34,8 @@
47 entries.reserve(users.count());
48 Q_FOREACH(const QString &user, users)
49 {
50- QString name = settings.value(user + "/name", user[0].toUpper() + user.mid(1)).toString();
51+ QVariant defaultValue = QString(user[0].toUpper() + user.mid(1));
52+ QString name = settings.value(user + "/name", defaultValue).toString();
53 entries.append({user, name, 0, 0, false, false, 0, 0});
54 }
55 }
56
57=== modified file 'tests/mocks/Unity/Launcher/MockQuickListModel.cpp'
58--- tests/mocks/Unity/Launcher/MockQuickListModel.cpp 2013-08-19 15:10:58 +0000
59+++ tests/mocks/Unity/Launcher/MockQuickListModel.cpp 2015-09-30 13:12:27 +0000
60@@ -32,7 +32,7 @@
61 switch (role)
62 {
63 case RoleLabel:
64- return QLatin1String("test menu entry ") + QString::number(index.row());
65+ return QString(QLatin1String("test menu entry ") + QString::number(index.row()));
66 case RoleIcon:
67 return QLatin1String("copy.png");
68 case RoleClickable:
69
70=== modified file 'tests/mocks/Unity/fake_resultsmodel.cpp'
71--- tests/mocks/Unity/fake_resultsmodel.cpp 2015-09-22 10:44:21 +0000
72+++ tests/mocks/Unity/fake_resultsmodel.cpp 2015-09-30 13:12:27 +0000
73@@ -78,7 +78,7 @@
74 case RoleTitle:
75 return QString("Title.%1.%2").arg(m_categoryId).arg(index.row());
76 case RoleArt:
77- return qmlDirectory() + "/graphics/applicationIcons/dash.png";
78+ return QString(qmlDirectory() + "/graphics/applicationIcons/dash.png");
79 case RoleSubtitle:
80 return QString("Subtitle.%1.%2").arg(m_categoryId).arg(index.row());
81 case RoleMascot:
82
83=== modified file 'tests/mocks/Unity/fake_scopesoverview.cpp'
84--- tests/mocks/Unity/fake_scopesoverview.cpp 2015-09-22 10:44:21 +0000
85+++ tests/mocks/Unity/fake_scopesoverview.cpp 2015-09-30 13:12:27 +0000
86@@ -308,7 +308,7 @@
87 case RoleSubtitle:
88 return scope && scope->name() == "Videos this is long ab cd ef gh ij kl" ? "tube, movies, cinema, pictures, art, moving images, magic in a box" : QString();
89 case RoleArt:
90- return qmlDirectory() + "/graphics/applicationIcons/dash.png";
91+ return QString(qmlDirectory() + "/graphics/applicationIcons/dash.png");
92 case RoleMascot:
93 case RoleEmblem:
94 case RoleSummary:
95
96=== modified file 'tests/plugins/Wizard/tst_pagelist.cpp'
97--- tests/plugins/Wizard/tst_pagelist.cpp 2014-11-14 17:47:00 +0000
98+++ tests/plugins/Wizard/tst_pagelist.cpp 2015-09-30 13:12:27 +0000
99@@ -88,13 +88,13 @@
100
101 PageList pageList;
102 QCOMPARE(pageList.index(), -1);
103- QCOMPARE(pageList.next(), root.path() + "/a/" + PAGES_PATH + "/1.qml");
104+ QCOMPARE(pageList.next(), QString(root.path() + "/a/" + PAGES_PATH + "/1.qml"));
105 QCOMPARE(pageList.prev(), QString());
106- QCOMPARE(pageList.next(), root.path() + "/a/" + PAGES_PATH + "/2.qml");
107- QCOMPARE(pageList.prev(), root.path() + "/a/" + PAGES_PATH + "/1.qml");
108+ QCOMPARE(pageList.next(), QString(root.path() + "/a/" + PAGES_PATH + "/2.qml"));
109+ QCOMPARE(pageList.prev(), QString(root.path() + "/a/" + PAGES_PATH + "/1.qml"));
110 QCOMPARE(pageList.index(), 0);
111- QCOMPARE(pageList.next(), root.path() + "/a/" + PAGES_PATH + "/2.qml");
112- QCOMPARE(pageList.next(), root.path() + "/a/" + PAGES_PATH + "/3.qml");
113+ QCOMPARE(pageList.next(), QString(root.path() + "/a/" + PAGES_PATH + "/2.qml"));
114+ QCOMPARE(pageList.next(), QString(root.path() + "/a/" + PAGES_PATH + "/3.qml"));
115 QCOMPARE(pageList.index(), 2);
116 QCOMPARE(pageList.next(), QString());
117 QCOMPARE(pageList.index(), 2);

Subscribers

People subscribed via source and target branches