Discussion:
[Development] Qt6 source changes
Иван Комиссаров
2018-10-31 10:19:46 UTC
Permalink
Hello, Thiago wrote in the "buildsystem thread" that Qt 6 will be source compatible with Qt5. As long as I fully support source-compatibility with both hands, I would like to ask several questions about possible source-breaking changes.

1) QAbstractItemModel. I want to introduce change that adds QModelIndex parameter to headerData(). For now, QHeaderView ignores the rootIndex() property which prevents creating models with sub tables in each index (only tree models are supported now). The example - you need to display a struct (or, protobuf message) as a table. Each struct can have sub structs (or, arrays of structs) so the tree is not enough to display it. I wrote a working prototype back in 2015 which required subclassing QAIM and copying the whole QHeaderView into the project. In this case source compatibility can be saved by having 2 virtual headerData methods, the one without modelIndex market deprecated.

2) QVariantList/QList<QImage> and friends. I would like to to see most of QList usages in Qt eliminated in favor of QVector. I won't repeat Marc's arguments against QList. Instead, I want to ask the other thing - is breaking user code that uses "QList<T> list = someQtApiFunc();" is forbidden too? If Assumption that QVector is API-compatible with QList (which is not true, I suppose, but can be mostly fixed) that should not be a big deal to replace the usages of a QList with "auto list = someQtApiFunc();". Qt should really evolve and stucking with QList for the next 10 years is a bad idea IMO.

Иван Комиссаров
Thiago Macieira
2018-10-31 16:46:06 UTC
Permalink
Post by Иван Комиссаров
Hello, Thiago wrote in the "buildsystem thread" that Qt 6 will be source
compatible with Qt5. As long as I fully support source-compatibility with
both hands, I would like to ask several questions about possible
source-breaking changes.
Qt6 won't be fully source-compatible. The idea is that we'll break it to fix
things, but attempt to keep compatible as much as possible to avoid death by a
thousand paper cuts.
Post by Иван Комиссаров
2) QVariantList/QList<QImage> and friends. I would like to to see most of
QList usages in Qt eliminated in favor of QVector. I won't repeat Marc's
arguments against QList. Instead, I want to ask the other thing - is
breaking user code that uses "QList<T> list = someQtApiFunc();" is
forbidden too? If Assumption that QVector is API-compatible with QList
(which is not true, I suppose, but can be mostly fixed) that should not be
a big deal to replace the usages of a QList with "auto list =
someQtApiFunc();". Qt should really evolve and stucking with QList for the
next 10 years is a bad idea IMO.
We're studying what to do with QList, but the idea is that the name "QList"
will be completely ok and identical to QVector. The technical mechanism is in
flux.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Sascha Cunz
2018-11-01 10:58:17 UTC
Permalink
Post by Thiago Macieira
Qt6 won't be fully source-compatible. The idea is that we'll break it to fix
things, but attempt to keep compatible as much as possible to avoid death by a
thousand paper cuts.
Does this rule out some kind of smart pointer that would be used to hold
QObject*?

I've seen lots of commercial code bases in the past where QObject
inheriting classes are combined with QExplicitlySharedDataPointer -
while at the same time relying on having a parent in those QObjects.

Semantically:
- There's some class for which QObject benefits like Signals/Slots are
required

- They are strongly owned by some other QObject based entity

- A lot of code stores a reference to them (In terms of a QESDP), where
either:
- the reference _should_ be invalidated during destruction
- the existence of such reference (to a child) _should_ stop the
parent from destroying that child.

- The overhead of QPointer is not desirable

I know these requirements are kind of a paradox. But both variants of
this are a pattern, I find relatively often in customer's code bases.
I would really like to have an easy way to give such customers a "sane"
way forward.

Sascha
Konstantin Shegunov
2018-11-01 11:17:47 UTC
Permalink
I've seen that kind of argument before, so that's why I want put a comment
in here.
Post by Sascha Cunz
I've seen lots of commercial code bases in the past where QObject
inheriting classes are combined with QExplicitlySharedDataPointer -
while at the same time relying on having a parent in those QObjects.
QExplicitlySharedDataPointer? That would be rather odd. Do you mean
QSharedPointer?
Post by Sascha Cunz
- They are strongly owned by some other QObject based entity
Then that's their parent.

- A lot of code stores a reference to them (In terms of a QESDP), where
- the reference _should_ be invalidated during destruction
QPointer or connect directly to `QObject::destroyed`.

- the existence of such reference (to a child) _should_ stop the
Post by Sascha Cunz
parent from destroying that child.
Then they're not strongly owned by the parent!

- The overhead of QPointer is not desirable
Other smart pointers are cheap?

I know these requirements are kind of a paradox. But both variants of
Post by Sascha Cunz
this are a pattern, I find relatively often in customer's code bases.
Sounds more like antipattern.
Post by Sascha Cunz
I would really like to have an easy way to give such customers a "sane"
way forward.
That'd be to define the ownership clearly, not try to weasel out of it with
some "smart" pointer.
Self-owning QObject instances can still be hold in `QSharedPointer` if
needed, however,
then they're not owned by the parent, so you shouldn't give them a parent.
Thiago Macieira
2018-11-01 20:06:53 UTC
Permalink
Post by Sascha Cunz
Post by Thiago Macieira
Qt6 won't be fully source-compatible. The idea is that we'll break it to fix
things, but attempt to keep compatible as much as possible to avoid death by a
thousand paper cuts.
Does this rule out some kind of smart pointer that would be used to hold
QObject*?
No, it does not rule out "some kind of smart pointer".

So long as "some kind of smart pointer" is 99.9% source-compatible when not
opted into. Probably 99.99%.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Kevin Kofler
2018-11-02 02:18:11 UTC
Permalink
Post by Thiago Macieira
We're studying what to do with QList, but the idea is that the name
"QList" will be completely ok and identical to QVector. The technical
mechanism is in flux.
That means you will be pessimizing element inserts and removals from O(n) to
O(mn), where n is the length of the list and m the size of each element,
without offering a good alternative without that pessimization (sure, you
can use a QVector<T*> or QVector<SomeSmartPointer<T>>, but those have
somewhat different semantics and less convenient syntax).

It won't make a difference for implicitly-shared objects (but QList already
works like a vector for those anyway), but for large in-place objects, it
can make a big difference.

Kevin Kofler
Thiago Macieira
2018-11-02 03:45:58 UTC
Permalink
Post by Kevin Kofler
Post by Thiago Macieira
We're studying what to do with QList, but the idea is that the name
"QList" will be completely ok and identical to QVector. The technical
mechanism is in flux.
That means you will be pessimizing element inserts and removals from O(n) to
O(mn), where n is the length of the list and m the size of each element,
without offering a good alternative without that pessimization (sure, you
can use a QVector<T*> or QVector<SomeSmartPointer<T>>, but those have
somewhat different semantics and less convenient syntax).
Yes. Is that a widespread use? And will it be a perceptible change?

Don't forget that m is a constant, for any given list. It's not a scalability
problem, since no matter how many inputs the user provides, the size of the
object will not change.
Post by Kevin Kofler
It won't make a difference for implicitly-shared objects (but QList already
works like a vector for those anyway), but for large in-place objects, it
can make a big difference.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
André Pönitz
2018-11-02 08:13:37 UTC
Permalink
Post by Thiago Macieira
Post by Kevin Kofler
Post by Thiago Macieira
We're studying what to do with QList, but the idea is that the name
"QList" will be completely ok and identical to QVector. The technical
mechanism is in flux.
That means you will be pessimizing element inserts and removals from O(n) to
O(mn), where n is the length of the list and m the size of each element,
without offering a good alternative without that pessimization (sure, you
can use a QVector<T*> or QVector<SomeSmartPointer<T>>, but those have
somewhat different semantics and less convenient syntax).
Yes. Is that a widespread use? And will it be a perceptible change?
Depends on usage.

See e.g. 551efd91990e07902e5324f720cf5585865c323d

QmlProfiler: Use QList for QmlRange container when loading .qtd

As we are using this as a queue, with many calls to takeFirst(), a
QVector is prohibitively expensive here.

Andre'
Ulf Hermann
2018-11-02 08:20:39 UTC
Permalink
Post by André Pönitz
Depends on usage.
See e.g. 551efd91990e07902e5324f720cf5585865c323d
QmlProfiler: Use QList for QmlRange container when loading .qtd
As we are using this as a queue, with many calls to takeFirst(), a
QVector is prohibitively expensive here.
I should have used QQueue there, and that will hopefully still be a
linked(-ish) list in Qt6. Or it might become a ring buffer, but please
not a plain vector.

If we don't want to keep QQueue, I will still have the option to use an
equivalent std container there (+/- implicit sharing).

Ulf
Olivier Goffart
2018-11-02 09:06:57 UTC
Permalink
Post by Ulf Hermann
Post by André Pönitz
Depends on usage.
See e.g. 551efd91990e07902e5324f720cf5585865c323d
QmlProfiler: Use QList for QmlRange container when loading .qtd
As we are using this as a queue, with many calls to takeFirst(), a
QVector is prohibitively expensive here.
I should have used QQueue there, and that will hopefully still be a
linked(-ish) list in Qt6. Or it might become a ring buffer, but please
not a plain vector.
If we don't want to keep QQueue, I will still have the option to use an
equivalent std container there (+/- implicit sharing).
Right, in this case std::deque would probably be a better fit. You do not need
implicit sharing, and std::deque still tries to optimize for cache, and will
not call the move constructor on every element each time it has to "re-locate"
(We should probably deprecate QQueue right now and tell people to use std::deque)
--
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
Kevin Kofler
2018-11-03 14:44:34 UTC
Permalink
Post by Olivier Goffart
(We should probably deprecate QQueue right now and tell people to use std::deque)
Please no!

The API is just not comparable: QQueue uses nicer terminology (enqueue,
dequeue, head) and is implicitly shared, and most importantly, is consistent
with the other Qt containers.

It is only one-sided, but often that's all that's needed. But the two-sided
use case (for which plain QList is currently the best fit, it offers all the
operations std::deque does, with similar wording) must not be destroyed
either (as replacing QList with the current QVector would).

Kevin Kofler
André Pönitz
2018-11-02 09:10:22 UTC
Permalink
Post by Ulf Hermann
Post by André Pönitz
Depends on usage.
See e.g. 551efd91990e07902e5324f720cf5585865c323d
QmlProfiler: Use QList for QmlRange container when loading .qtd
As we are using this as a queue, with many calls to takeFirst(), a
QVector is prohibitively expensive here.
I should have used QQueue there, and that will hopefully still be a
linked(-ish) list in Qt6. Or it might become a ring buffer, but please
not a plain vector.
If we don't want to keep QQueue, I will still have the option to use an
equivalent std container there (+/- implicit sharing).
I am not saying that QVector or QList is the best container here,
it was just an example for a (rare...) case where a substitution
introduces a significant performance regression in real life.

One concern here is also that the kind of regression can be hard to
discover as compilation succeeds, systematic performance testing
is often not done, and any manual testing before release usually
uses only smaller-than-normal sample sizes. So the first one to
notice the problem is the user of the code.

Andre'
Nikolai Kosjar
2018-11-02 09:16:44 UTC
Permalink
Post by André Pönitz
Post by Ulf Hermann
Post by André Pönitz
Depends on usage.
See e.g. 551efd91990e07902e5324f720cf5585865c323d
QmlProfiler: Use QList for QmlRange container when loading .qtd
As we are using this as a queue, with many calls to takeFirst(), a
QVector is prohibitively expensive here.
I should have used QQueue there, and that will hopefully still be a
linked(-ish) list in Qt6. Or it might become a ring buffer, but please
not a plain vector.
If we don't want to keep QQueue, I will still have the option to use an
equivalent std container there (+/- implicit sharing).
I am not saying that QVector or QList is the best container here,
it was just an example for a (rare...) case where a substitution
introduces a significant performance regression in real life.
One concern here is also that the kind of regression can be hard to
discover as compilation succeeds, systematic performance testing
is often not done, and any manual testing before release usually
uses only smaller-than-normal sample sizes. So the first one to
notice the problem is the user of the code.
Introducing some new clazy checks specifically for catching issues for
the Qt5->Qt6 port could help here.

Of course, the developer still would need to actually run clazy...

Nikolai
Lars Knoll
2018-11-02 09:17:53 UTC
Permalink
On 2 Nov 2018, at 10:10, André Pönitz <***@t-online.de<mailto:***@t-online.de>> wrote:

On Fri, Nov 02, 2018 at 08:20:39AM +0000, Ulf Hermann wrote:
Depends on usage.

See e.g. 551efd91990e07902e5324f720cf5585865c323d

QmlProfiler: Use QList for QmlRange container when loading .qtd

As we are using this as a queue, with many calls to takeFirst(), a
QVector is prohibitively expensive here.

I should have used QQueue there, and that will hopefully still be a
linked(-ish) list in Qt6. Or it might become a ring buffer, but please
not a plain vector.

If we don't want to keep QQueue, I will still have the option to use an
equivalent std container there (+/- implicit sharing).

I am not saying that QVector or QList is the best container here,
it was just an example for a (rare...) case where a substitution
introduces a significant performance regression in real life.

Yes, I agree that those are possible. But should that stop us from doing changes that will improve things in 98% of the use cases? Hopefully not.

One concern here is also that the kind of regression can be hard to
discover as compilation succeeds, systematic performance testing
is often not done, and any manual testing before release usually
uses only smaller-than-normal sample sizes. So the first one to
notice the problem is the user of the code.

Well, maybe we can try to help here, e.g. by adding a special #define that would generate compiler warnings for takeFirst() and other calls that have a different performance characteristic in Qt 6 than in Qt 5. That would give people the option to find those cases more easily.

Cheers,
Lars
Lars Knoll
2018-11-02 08:43:56 UTC
Permalink
Post by Thiago Macieira
Post by Kevin Kofler
Post by Thiago Macieira
We're studying what to do with QList, but the idea is that the name
"QList" will be completely ok and identical to QVector. The technical
mechanism is in flux.
That means you will be pessimizing element inserts and removals from O(n) to
O(mn), where n is the length of the list and m the size of each element,
without offering a good alternative without that pessimization (sure, you
can use a QVector<T*> or QVector<SomeSmartPointer<T>>, but those have
somewhat different semantics and less convenient syntax).
Yes. Is that a widespread use? And will it be a perceptible change?
Don't forget that m is a constant, for any given list. It's not a scalability
problem, since no matter how many inputs the user provides, the size of the
object will not change.
m is basically the size of the object. So for pointer sized objects it doesn’t make a different. For large objects QList actually also has an overhead, namely the malloc() incurred for each item inserted.

I’ve tried a bit and for most use cases where the list and the object are of reasonable size (e.g. 4*sizeof(ptr)and ~100 items in the list) that overhead is actually just as large.
Post by Thiago Macieira
Post by Kevin Kofler
It won't make a difference for implicitly-shared objects (but QList already
works like a vector for those anyway), but for large in-place objects, it
can make a big difference.
There are always use cases where it can make a difference. The question is what happens for most use cases (ie. 90 or even 95% of the cases)? And based on my measurements so far, I’d say we will see a performance improvement for those, leading to an overall improvement.

Cheers,
Lars
Kevin Kofler
2018-11-03 14:47:53 UTC
Permalink
Post by Lars Knoll
I’ve tried a bit and for most use cases where the list and the object are
of reasonable size (e.g. 4*sizeof(ptr)and ~100 items in the list) that
overhead is actually just as large.
4*sizeof(ptr) (i.e., at most 32 bytes) is not a large object. (Won't even
the new QString with SSO be larger than that?) A large object is more like
100+ or even 1000+ bytes.

Kevin Kofler
Thiago Macieira
2018-11-03 18:11:48 UTC
Permalink
Post by Kevin Kofler
4*sizeof(ptr) (i.e., at most 32 bytes) is not a large object. (Won't even
the new QString with SSO be larger than that?) A large object is more like
100+ or even 1000+ bytes.
Whether we'll have SSO is not decided. But either way, it'll increase to
3*sizeof(void*).
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Kevin Kofler
2018-11-03 21:27:33 UTC
Permalink
Post by Thiago Macieira
Whether we'll have SSO is not decided.
That's good news, because I'm not convinced that that is a good idea,
either. It should be done only if it really is a win.

But that also removes one major argument for changing QList to yet another
QVector: the argument was that QStringList or QList<String> would be
inefficient with SSO QString, but that is moot if QString stays as it is.

Kevin Kofler
Thiago Macieira
2018-11-04 03:43:21 UTC
Permalink
Post by Kevin Kofler
But that also removes one major argument for changing QList to yet another
QVector: the argument was that QStringList or QList<String> would be
inefficient with SSO QString, but that is moot if QString stays as it is.
A minimal SSO QString would take exactly as long to copy in a QVector as a
non-SSO one.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Shawn Rutledge
2018-11-02 06:05:36 UTC
Permalink
Hi,
Post by Иван Комиссаров
1) QAbstractItemModel. I want to introduce change that adds QModelIndex parameter to headerData(). For now, QHeaderView ignores the rootIndex() property which prevents creating models with sub tables in each index (only tree models are supported now). The example - you need to display a struct (or, protobuf message) as a table. Each struct can have sub structs (or, arrays of structs) so the tree is not enough to display it. I wrote a working prototype back in 2015 which required subclassing QAIM and copying the whole QHeaderView into the project. In this case source compatibility can be saved by having 2 virtual headerData methods, the one without modelIndex market deprecated.
This is reasonable given the tree-of-tables that QAbstractItemModel models; there's the bigger question of whether the one-API-fits-all is a good way forward, however I don't see anyone willing to rewrite the model classes for this. (There are also another couple of ideas I have in the QAIM department, e.g. a multiData(), which is just BC because it's a new virtual).
If anyone was willing to work on it, what sort of API would you propose?
Иван Комиссаров
2018-11-02 19:13:05 UTC
Permalink
Nice idea to have such a method. But is't it a pessimization to return a vector which certainly allocates?
Don't throw rocks in me, what about passing std::function that can enumerate roles?

Иван Комиссаров
Hi,
Post by Shawn Rutledge
This is reasonable given the tree-of-tables that QAbstractItemModel models; there's the bigger question of whether the one-API-fits-all is a good way forward, however I don't see anyone willing to rewrite the model classes for this. (There are also another couple of ideas I have in the QAIM department, e.g. a multiData(), which is just BC because it's a new virtual).
If anyone was willing to work on it, what sort of API would you propose?
(Side note: please double check your email client, it seems you've got Sérgio as contact for the development mailing list :-))
For this sort of API, something really simple, along the lines of
Post by Shawn Rutledge
struct Data { Qt::ItemDataRole role; QVariant data; };
virtual vector<Data> multiData(const QModelIndex &index, vector<Data> roles) const;
(We can bikeshed on the exact signature, and if we should be using some "small map" type, QVector, std::vector, ...). Anyhow, the idea is to pass the set of roles that we want to read for a given index, and get the data for all those roles in one go. This should help stop hammering data() once per role; of course, the default implementation will simply call data() multiple times.
My 2 c,
--
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, http://www.kdab.com
KDAB - The Qt, C++ and OpenGL Experts
Иван Комиссаров
2018-11-03 02:16:38 UTC
Permalink
Sure. Sorry for std::array_view (it maybe a QSet or QVector or gsl::span whatever more convinient/faster).
The basic idea is here:

In the model:
void multipleData(QModelIndex index, std::array_view<Qt::ItemRole> roles, std::function<void(Qt::ItemRole,QVariant)> enumerator)
{
for (auto&& role: roles) {
if (role == Qt::DisplayRole) {
enumerator(role, data.at(index.row()).text);
if (role == Qt::EditRole) {
enumerator(role, data.at(index.row()).editText);
}
// 
.
}
}

In the view:
QMap<Qt::ItemRole, QVariant> map
auto enumerator = [&map](Qt::ItemRole role, QVariant data) {
map[role] = data; // actually here’s the view’s code, painting data and so on, no need in intermediate container, I use it just for example
}
model->multipleData(index, {Qt::DisplayRole, Qt::BackgroundRole}, enumerator);

Not sure if std::function does not allocate in case of lambda, btw. I 80% sure it does, so returning a vector can be better. Anyway, one can look at the assembly in both cases and run a profiler.
Mind to elaborate on the std::function idea?
Loading...