PollingControlProxy

[daschuer]

This is a light weight version of ControlProxy, not inheriting QObject.
It can be used whenever no signals are required and the value is polled.

I wan't to use it to replace ControlObject::getControl, which returns an unsave pointer that can become invalid during shut down and lead to a crash.

The new object is finally just a PIMPL smart pointer and should not be much more expensive compared to ControlObject::getControl

This is now on top of #4229

[rryan]

can ControlProxy inherit ControlProxyLt to prevent the code duplication?

[daschuer]

Yes, I have not done it to avoid double inheritance. Should I change it?

[daschuer]

@rryan can we merge this as it is?

[daschuer]

I have just tried to inherit ControlProxy from ControlProxyLt, but that does not work well, because the inherited functions are slots in ControlProxy, so we need to override them anyway. This feels like clutter.

[daschuer]

Now it is even more improved by using a default CO instead of all the null checks.

[Be-ing]

Can you elaborate on the use case for this class? What advantage does it have over ControlProxy? I think it would help demonstrate if you modified some existing code to use this new class.

[daschuer]

Yes I can do. I had already a branch that unfortunately does not merge anymore.
daschuer@368e5fc

The main use case was to document "I will not receive Value Change Signals".
The side effect is that this class will be optimized almost entirely away by the compiler.
It is useful to replace the control object pointer passing in the engine.
In theory it should also speed up Mixxx start ... negligible.

After the refactoring we will have a clear view which CO needs to receive a signal from the engine.

[Be-ing]

What is the importance of not receiving signals? The ControlDoublePrivate still must emit the valueChanged signal.

[daschuer]

Yes, it is mainly a refactoring for readability.

I am planning to remove QT's signal slot connections from the engine and therefore it is nice to know which object actually receives signals.

The main driver was to deprecate ControlObject::getControl(). Passing a control objects around is insafe and block further refactorings.
However this is difficult, because using this omits the signals from the owner. All proxies are signaled but not the owner.

Later I realized that the Lt variant allows to simplify memory management a lot and saves some CPU cycles.

[Holzhaus]

Can we use a more descriptive name? To me it's not immediately apparent what ControlProxyLt does. Maybe PollingControlProxy?

[daschuer]

Ok.

[uklotzde]

ADC'19 Recordings:
Real-time 101 - Part I: Investigating the real-time problem space
Real-time 101 - Part II: The real-time audio developer’s toolbox

[daschuer]

Thank you for the videos. I hope I find time to watch them completely at the weekend.

Is this PR Ok so far?
I am considering to swap the names to
ControlProxy and ControlWatcher, but this will add hundreds of new changed lines.

If you agree to the approach I would like to merge this with the preliminary name and do just the renaming in a separate PR.

What do you think?

[Be-ing]

I don't think "ControlWatcher" quite fits because that implies it is read only.

[daschuer]

Alternatives?

I like to emphersise that one receives updates and the other not.

Later we may create one, that is connected to the cue out of the engine thread or alternatively automatically check for the source thread and use the queue or not.

[daschuer]

PollingControlProxy works ok for me, however the other one can also poll and it is also not read only.

[Holzhaus]

Some ideas:

• PollingControlProxy
• NonReceivingControlProxy
• ControlProxyWithoutSignals
• SlotOnlyControlProxy
• ManualControlProxy

[daschuer]

Benchmarks for 1000 default constructed objects lifetime (create and destroy):
88 ms ControlProxyLt on stack
190 ms ControlProxyLt on heap
295 ms ControlProxy on stack
399 ms ControlProxy on heap manual delete
499 ms ControlProxy on heap with parent
617 ms ControlProxy on heap with parent and manual delete

I think we can learn a bit from this:

• Don't use parented QObjects if not required
• Don't manually delete parented QObjects manually
• Avoid heap construction (new())

By the way: Mixxx has 64209 different CO objects and many proxy objects for each of it.

[github-actions]

This PR is marked as stale because it has been open 90 days with no activity.

[coveralls]

Pull Request Test Coverage Report for Build 1152198391

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 9 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.01%) to 25.949%

---

Files with Coverage Reduction	New Missed Lines	%
src/engine/cachingreader/cachingreader.cpp	9	71.38%

Totals
Change from base Build 1152021735:	-0.01%
Covered Lines:	19993
Relevant Lines:	77047

---

💛 - Coveralls

[daschuer]

@Holzhaus this is ready to review/merge now. I think this can be a perfect base class for your ControlFramePosProxy()
In this case a PollingControlFramePosProxy() in #4227

[Holzhaus]

Thanks, this looks okay to me. Could you add it to some locations where no signals are needed? Then we could benchmark the startup time to check if it's worth it.

[daschuer]

I have already checked it and the is a performance benefit but it is negligible, compared to other log runners during start up.

The main driver for this was the documentation purpose. If we use that everywhere it is possible, we have a clear picture what to do to fix unsafe pointer access and the Qt locking issue when receiving updates.

[ferranpujolcamins]

LGTM. I too would like to get rid of Control::getControl.

[daschuer]

Thank you for review, and the positive responds :-)
Merge?