> On 6 Jun 2019, at 14:49, Mutz, Marc via Development > <development@qt-project.org> wrote: > > On 2019-06-06 14:04, Lars Knoll wrote: >>> On 6 Jun 2019, at 13:39, Mutz, Marc via Development >>> <development@qt-project.org> wrote: > [...] >>> You are equating Qt users and Qt implementers. You can maintain the Qt API, >>> but use more efficient data structures in the implementation. You seem to >>> be implying that these two things cannot be separated. >> The comment above was aimed at our APIs and towards our users, not >> about how we implement things internally. > > Ok, clash of context. This subthread was about the exact change in QtWayland, > though, and that's internal implementation. > >>> None of the changes where I replaced QMap changes the public API at all >>> (except adding an overload because we can). No user is affected by this in >>> any way, except that they have a few pages of memory more free than before. >>> Please explain to me how any of those changes makes _users_ of Qt have to >>> revert to the STL? >>> And please explain to me how it can possibly be worthwhile to generate 8KiB >>> of code _just_ to not have to use lower_bound? Which argument could >>> *possibly* be made against a lower_bound? Esp. seeing as many attempts to >>> write one by hand have failed. I remember a bug about shortcuts being >>> mapped to the wrong key, because the hand-rolled binary search was unstable. >> You have in general been advocating lately to remove/deprecate lots of >> Qt API in favour of STL. This is what I referred to, not >> implementation details in our code. > > Ok, noted. > >> Removing the usage of a QMap for a mapping of 5 values is the right >> thing to do. Whether we need lower_bound() for that and a sorted list, >> or simply an unsorted vector where we iterate until we find the right >> value doesn’t matter too much in this case. Performance wise both >> would probably be equivalent :) >>> I'm sorry, but we have a lot of code that is less readable than any of the >>> changes I uploaded. It just cannot be an argument to say that it's >>> unreadable because it uses an STL algorithm. This sentiment has caused so >>> very, very many quadratic loops because people get the impression that >>> std::remove_if is toxic, and in each one the solution was to use remove_if, >>> because the hand-rolled alternative would be totally unreadable. >> Nobody here said that. I also believe those changes that you proposed >> got approved to the largest degree. > > This is not the impression I get from > https://codereview.qt-project.org/c/qt/qtwayland/+/264069, or > https://codereview.qt-project.org/c/qt/qtbase/+/264129 vs. > https://codereview.qt-project.org/c/qt/qtbase/+/264161 > >> If you read what I said earlier, I also said that in most cases better >> data structures and algorithms will lead to more readable code. I am >> not opposed to using those in Qt, quite the opposite. > > Good. > >>> I'm sad to see that Qt devs think so lowly of themselves as to be unable to >>> understand a piece of code that uses an STL algorithm. Really. I'm out of >>> words. >> Nobody ever said that as far as I can tell. What people said is that >> there are cases, where a loop is just as efficient and easier to read >> (e.g. when searching for an entry). And in that case it’s fine to >> simply use that. So please stop turning the meaning of what people >> said around like that. > > I was a poignant remark, yes, but that's _exactly_ the feedback I get from > the reviews I quoted above: Dropping QMap is acceptable when the API is > unchanged (as with the array replacing the map), but totally not acceptable > (b/c/o unreadable) if it contains lower_bound or find_if. To the point that > the 'better' choice is to replace QMap with QHash - because they have the > same API. I'd ROTFL if I wasn't crying.
The discussion showed that replacing QMap with QHash gives you most of the improvement that you wanted. I can understand that maintainers don’t like if the change adds ~40 lines that can’t be shared, and complicate the code. We are spending a lot of time maintaining our code base, and simplifying our lives there is extremely important as we have limited man-power available. This is especially true if similar changes are being done in many places in our code base. With respect to that I can see why people argue for using either a QHash (where the change is a one liner), or ask you to consider providing a (maybe internal) special class that can be used as a drop in replacement. Cheers, Lars _______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
