Add some const &
ClosedPublic

Authored by aacid on Oct 28 2019, 9:55 PM.

Details

Summary

Won't make things go much faster since everything that was
being passed by value is refcounted but still const & is a bit faster
than refcounting

For shared pointers instead of adding const & we move them into the
destination variable saving some cpu usage but at the same time making
clear the pointer is being stored by not being const &

Diff Detail

Repository
R108 KWin
Branch
arcpatch-D25022
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 18333
Build 18351: arc lint + arc unit
aacid created this revision.Oct 28 2019, 9:55 PM
Restricted Application added a project: KWin. · View Herald TranscriptOct 28 2019, 9:55 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
aacid requested review of this revision.Oct 28 2019, 9:55 PM
zzag added a subscriber: zzag.Oct 29 2019, 10:09 AM
zzag added inline comments.
libkwineffects/anidata.cpp
82–83

Heh, the good old "should I pass a shared pointer by value or const reference?"

The reasoning why these are not passed by const ref is to express that AniData will share ownership of the locks.

aacid updated this revision to Diff 69014.Oct 29 2019, 10:09 PM
aacid edited the summary of this revision. (Show Details)

some const & are now std::move

aacid added inline comments.Oct 29 2019, 10:10 PM
libkwineffects/anidata.cpp
82–83

fair enough, i'm now moving these ptrs instead of const &-ing them

zzag accepted this revision.Oct 30 2019, 9:37 AM

For shared pointers instead of adding const & we move them into the destination variable saving some cpu usage but at the same time making clear the pointer is being stored by not being const &

It's worth to point out that smart pointers in Qt are not movable. So we'll still be copying shared pointers.

This revision is now accepted and ready to land.Oct 30 2019, 9:37 AM
aacid added a comment.Oct 30 2019, 9:45 AM
In D25022#556505, @zzag wrote:

For shared pointers instead of adding const & we move them into the destination variable saving some cpu usage but at the same time making clear the pointer is being stored by not being const &

It's worth to point out that smart pointers in Qt are not movable. So we'll still be copying shared pointers.

What do you mean they are not movable?

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qshareddata.h?h=5.13#n222

has the move operator for QExplicitlySharedDataPointer which is what a KSharedConfigPtr is. It doesn't involve ref counting like the copy constructor in

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qshareddata.h?h=5.13#n185
zzag added a comment.Oct 30 2019, 9:56 AM

What do you mean they are not movable?

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qshareddata.h?h=5.13#n222

Edit: "not all smart pointers in Qt are movable." :-)

has the move operator for QExplicitlySharedDataPointer which is what a KSharedConfigPtr is. It doesn't involve ref counting like the copy constructor in

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qshareddata.h?h=5.13#n185

Yeah, but we also use QSharedPointer, which is not movable.

In D25022#556512, @zzag wrote:

Yeah, but we also use QSharedPointer, which is not movable.

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h#n332

It seems it is?

zzag added a comment.Oct 30 2019, 10:17 AM
In D25022#556512, @zzag wrote:

Yeah, but we also use QSharedPointer, which is not movable.

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qsharedpointer_impl.h#n332

It seems it is?

Oh, I didn't pay attention to https://github.com/qt/qtbase/blob/dev/src/corelib/tools/qsharedpointer.h#L47-L56. Please ignore my previous comments.

aacid closed this revision.Oct 30 2019, 6:23 PM