Re: [Development] QVector reserve counterproductive?

[Martins , Sérgio]

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

[Development] QVector reserve counterproductive?

[Christian Ehrlicher]

Hi,

while digging through the bugreports I found 
https://bugreports.qt.io/browse/QTBUG-66677 which seems to show a 
downside of QVector::reserve().
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. But it looks like reserve() allocates 
*exactly* the amount of elements given. Therefore calling addPolygon() a 
second time will do a new reallocation and so on.
The documentation for reserve() only states that memory for 'at least 
size elements' is allocated so my naive assumption would be that calling 
reserve a second time with a slightly bigger value than before does 
nothing. But this doesn't seem to be the case here.


For the bug: removing the reserve() inside addPolygon() fixes the speed 
issues mentioned in the bug report. Don't know if the bug report is a 
valid use case though.


Thx,
Christian
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] QVector rvalue overloads for convenience functions?

[Mandeep Sandhu]

>
>
> inline QVector +=(T &)
> { append(std::move(t)); return *this; }
>

Ah, yes! This makes sense.

inline QVector << (T &)
> { append(std::move(t)); return *this; }
>
> Note they might be missing from qvarlengtharray too.
>

Thanks for the clarification.

-mandeep


>
> 'Allan
>
>
>
>
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] QVector rvalue overloads for convenience functions?

[Allan Sandfeld Jensen]

On Samstag, 3. März 2018 21:11:18 CET Mandeep Sandhu wrote:
> On Sat, Mar 3, 2018 at 11:46 AM, Christian Ehrlicher 
> 
> wrote:
> > Hi,
> > 
> > recently rvalue overloads for QVector::append(T), push_back(T) and others
> > were added to QVector. But not for the convenience functions like
> > operator<<(T) or operator +=(T). Is this an oversight
> 
> Why would an rvalue overload (by that I assume you mean move semantics)
> apply to the += operator? You're not discarding the existing object, just
> adding values from whats pointed to by the other  reference.
> 
> As for the << operator, it _might_ be an oversight, I'm not sure. Someone
> else can chime in.
> 
Both of them could make sense assuming we are talking about the single value 
variants. I guess it is an oversight, feel free to add them.

inline QVector +=(T &)
{ append(std::move(t)); return *this; }
inline QVector << (T &)
{ append(std::move(t)); return *this; }

Note they might be missing from qvarlengtharray too.

'Allan

  


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] QVector rvalue overloads for convenience functions?

[Mandeep Sandhu]

On Sat, Mar 3, 2018 at 11:46 AM, Christian Ehrlicher 
wrote:

> Hi,
>
> recently rvalue overloads for QVector::append(T), push_back(T) and others
> were added to QVector. But not for the convenience functions like
> operator<<(T) or operator +=(T). Is this an oversight


Why would an rvalue overload (by that I assume you mean move semantics)
apply to the += operator? You're not discarding the existing object, just
adding values from whats pointed to by the other  reference.

As for the << operator, it _might_ be an oversight, I'm not sure. Someone
else can chime in.

CMIIW.

-mandeep



> or by intention?
>
>
> Thx,
> Christian
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

[Development] QVector rvalue overloads for convenience functions?

[Christian Ehrlicher]

Hi,

recently rvalue overloads for QVector::append(T), push_back(T) and 
others were added to QVector. But not for the convenience functions like 
operator<<(T) or operator +=(T). Is this an oversight or by intention?



Thx,
Christian
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development