Discussion about performance tuning with details C++ technic.

[2bbb]

Discussion thread about performance tuning like shared_ptr, move semantics, r-value and etc, etc.

related: #6839 #6867

ping: @dimitre @roymacdonald @ofTheo

[roymacdonald]

What are you suggesting?

[2bbb]

@roymacdonald

sorry. I didn't explain it enough.

I thought to need some policy about performance tuning.

for example, about avoiding temporary copy of std::shared_ptr: about ofMainLoop::addWindow, I think @dimitre 's PR is better balance with tuning and keeping clean code.

if something will call too many time, then I think overloading const std::shared_ptr<T> & and std::shared_ptr<T> && is more better.

one more, I said on #6867 about ref qualifiers, and @ofTheo points out "can't see a huge reason to add this sort of complexity". and I agree to it now. but I want to discuss about critical points, then mentioned some people who are likely to be interested to this thread. so, you say "There are lots of parts where this can be implemented", I want to know what you think, and want to think better way about those together. ☺️

thanks.

[roymacdonald]

I see. Avoiding the temporary copies should be a must, although the use of const must be handled with care. move semantics worry me a bit more as these invalidate one of the objects. But mostly I think that it must be addressed on a case specific way. There might be spots where allowing for move semantics can break a lot of stuff, while having a copy can be useful. why dont you give it a try and see how it works if you implement this policies in wherever you think it is suitable?

[dimitre]

I think it is strange to use plain shared_ptr as a function parameter, so we can choose to use const & as a function parameter, or move inside the function, so we don't have multiple increment and decrement. One or the other. It doesn't change anything in function usage only streamline things better internally. I think std::move keeps the object in the same place in memory and only moves the owner, which is desirable sometimes. maybe using both std::move and const & can create problems.

And it would be great to evaluate if shared_ptr is needed in all cases. I could identify at least one case where it doesn't need to be used at all. it is the following map which is managed internally as private:

	std::map<std::shared_ptr<ofAppBaseWindow>,std::shared_ptr<ofBaseApp> > windowsApps;

[2bbb]

@roymacdonald

I completely agree what you say. Maybe we can't decide rules definitely, and we need to try to implement some cases and watch that's result. (so, I thought is better what if we can find some rules for some common cases with discussion before implement.)

@dimitre

Sorry if my understanding about what you say and what you understanding are wrong.

I think it is strange to use plain shared_ptr as a function parameter, that's right.

of course, if we move argument that typed as const & then it make problem. (rather than, it make compile error) and I agree argument like void f(std::shared_ptr<T> v) is not good absolutely.

but I think what overloading both type of const & and && for argument don't create problem.

e.g.

to change from:

void f(const std::vector<int> &v) {
    this->v = v;
}

to:

// const &
void f(const std::vector<int>&v) {
    v_ = v;
}

// &&
void f(std::vector<int> &&v) {
   v_ = std::move(v); // or more simply std::swap(v_, v);
}

makes sense in below situations.

auto v = std::make_shared<T>();
f(v); // calls const &

f(std::move(v)); // calls &&

---

when I writing this reply, I notice unclarified issue about thread safeness of move shared_ptr... I need to more study... so, this thread is meaningful for me! thanks!!

sorry if the my English is difficult to read... please read my opinion from codes...

[roymacdonald]

Hi, @dimitre @2bbb Why dont we give it a try and see how it works. or at least having a list of places where this could be applied can help telling if it is either a good idea or not. I am not an expert at all in move semantics, but from what I understand, the && override only gets called when the function is invoked using std::move(...) as argument. If it is the case then the coder using it knows what are the consequences of it, thus it should not be a problem having such overide.

@dimitre

it is the following map which is managed internally as private:

std::map<std::shared_ptr<ofAppBaseWindow>,std::shared_ptr<ofBaseApp> > windowsApps;

I think that there is a function where you can get the current window, which returns a shared_ptr, thus storing these as shared_ptrs makes sense. (I am just pulling this from the top of my head, not 100% sure)

@2bbb Your english is perfectly fine. I understand you well, and I english is not my native language either. :D

[dimitre]

There is a function in OF (important one, key to any project) that mixes raw pointers, references and shared_ptr. Can it be simplified in any way? does it need to be shared_ptr? if yes does it need to use new?

https://github.com/openframeworks/openFrameworks/blob/0251a370236abb30ca013158b2ee5143ba578975/libs/openFrameworks/app/ofAppRunner.cpp#L57-L60

[ofTheo]

@dimitre as far as I can tell this works this same.

    shared_ptr<ofMainLoop> & mainLoop(){
        static shared_ptr <ofMainLoop> mainLoop = make_shared<ofMainLoop>();
        return mainLoop;
    }

I generally use make_shared instead of new for shared_ptrs, not sure it makes much difference, but it does look a little cleaner.

[dimitre]

Which style do you prefer and why? In the end they do the same, just each one of them do the copy in different lines

void ofSoundPlayer::setPlayer(shared_ptr<ofBaseSoundPlayer> newPlayer){
	player = std::move(newPlayer);
}

void ofSoundPlayer::setPlayer(const shared_ptr<ofBaseSoundPlayer> & newPlayer){
	player = newPlayer;
}

[dimitre]

one idea of performance optimization is substitute std::map to std::unordered_map whenever possible. if you don't need this ordered (we almost never need an ordered map sequence) it is a drop-in replacement.

[2bbb]

@dimitre

I think style like

void method(type newValue){
	value = std::move(newValue);
}

is better.

because both style are same count of copy. but user can use like method(std::move(value)); if value is not use after calling method, and then no copy is occurred.

but, if size of type is bigger (like std::vector) then what we have to provide two method like:

void method(const std::vector<float> &newValue){
	value = newValue;
}

void method(std::vector<float> &&newValue){
	value = std::move(newValue);
}

is better, I think.