Re: [Development] Nominating Ville Voutilainen as approver
+1 On 2018-02-27 09:36, Simon Hausmann wrote: Hi, I would like to nominate Ville as approver. Due to his work in the GCC project he is certainly experienced with strict code reviews. Based on my interaction with him I'm convinced that he has successfully demonstrated the same skill set during code reviews in Qt. Simon ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development -- Sérgio Martins | sergio.mart...@kdab.com | Senior Software Engineer Klarälvdalens Datakonsult AB, a KDAB Group company Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322) KDAB - The Qt, C++ and OpenGL Experts ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QVector reserve counterproductive?
On 2018-03-03 20:38, Christian Ehrlicher wrote: Hi, while digging through the bugreports I found https://bugreports.qt.io/browse/QTBUG-66677 which seems to show a downside of QVector::reserve(). Yes, reserve() shouldn't be called repeatedly inside loops. Instead use it only once, outside your outermost loop. QPainterPath::addPolygon() is calling reserve() to make sure to have enough space for the new positions. This is exactly what I would have done and what e.g. CLazy suggests. Not really, see "Example where reserve shouldn't be used" in https://github.com/KDE/clazy/blob/master/src/checks/level2/README-reserve-candidates.md and also the pitfalls section where it talkes about using the growth strategy instead But it looks like reserve() allocates *exactly* the amount of elements given. Actually that qpainterpath code is off-by-one, it should be: d_func()->elements.reserve(d_func()->elements.size() + polygon.size() - 1); which also fixes the performance problem (by luck, in that specific benchmark). Please test. Ideas welcome on how we can catch more of these cases... Regards, -- Sérgio Martins | sergio.mart...@kdab.com | Senior Software Engineer Klarälvdalens Datakonsult AB, a KDAB Group company Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322) KDAB - The Qt, C++ and OpenGL Experts ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] QVector reserve counterproductive?
On 2018-03-04 13:20, Christian Ehrlicher wrote: Am 04.03.2018 um 10:03 schrieb Christian Ehrlicher: Am 03.03.2018 um 23:22 schrieb Martins, Sérgio: On 2018-03-03 20:38, Christian Ehrlicher wrote: But it looks like reserve() allocates *exactly* the amount of elements given. Actually that qpainterpath code is off-by-one, it should be: d_func()->elements.reserve(d_func()->elements.size() + polygon.size() - 1); which also fixes the performance problem (by luck, in that specific benchmark). Please test. You're correct - this fixes the problem. Looks like the growth strategy is only applied when the current capacity == current size and not when the current capacity is slightly higher... good to know (and should maybe be documented?) The off-by-one idea was wrong. moveTo() will add another point to elements - therefore reserve(d_func()->elements.size() + polygon.size()) is correct. Maybe the best idea here would be to simply remove the reserve() calls and let QVector do what it is made for... :) If you're sure you're not pessimizing any other case, then removing reserve() is fine. But maybe there's a third option: vec.reserve(qMax(needed, what_the_capacity_would_have_been_without_reserve)) this way it's good for short containers, as growth is bootstrapped and also good for big containers as reserving becomes a no-op, as there's already capacity. Regards, -- Sérgio Martins | sergio.mart...@kdab.com | Senior Software Engineer Klarälvdalens Datakonsult AB, a KDAB Group company Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322) KDAB - The Qt, C++ and OpenGL Experts ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
