Cache ControlObject lookups in MidiController::processInputMapping

[mixxxbot]

Reported by: daschuer
Date: 2013-05-27T12:07:31Z
Status: Confirmed
Importance: Low
Launchpad Issue: lp1184581
Tags: controllers, midi

---

    // If no control is bound to this MIDI message, return
    if (!m_preset.mappings.contains(mappingKey.key)) {    <- first Lockup 
        return;
    }

    QPair<MixxxControl, MidiOptions> controlOptions = m_preset.mappings.value(mappingKey.key);  <- second lookup 

... snip

    ControlObject* p = mc.getControlObject(); <- third lookup

[mixxxbot]

Commented by: daschuer
Date: 2013-10-13T18:43:29Z

---

We should do a complete refactor the CO hashtables:

I have some made some measurements on a Ubuntu 32 bit system and here are the results:
(The absolute values are only for reference, they are rather un-precise and varies from run to run, but the tendency and magnitude is allays the same.)

ControlObject::get(ConfigKey());  // 2065 ns  on a hashtable filled with all controls 
getControlObjectThread(ConfigKey())->get();  // 1930ns on a private hashtable filled with 6 controls 
pCOT->get();  // 695ns  and will be much faster on a 64 bit system because double is atomic there 

Conclusion:

• Size of QHash dos not put significant time on the get call.
• Saving a local pCOT speeds up get() by 3 @32bit

https://bugs.launchpad.net/mixxx/+bug/1166016 depends on this as well.

[mixxxbot]

Commented by: daschuer
Date: 2013-10-13T20:25:42Z

---

Results from a 64 bit machine:
ControlObject::get 1903ns
6er hash 1702ns
pCOT->get() 221ns

speed up is ~8 here.

It is fine to see that atomic-co improves get by 2,5 :-)

[mixxxbot]

Commented by: daschuer
Date: 2013-10-14T15:10:46Z

---

Just found http://woboq.com/blog/qmap_qhash_benchmark.html (please read discussion as well)

Conclusion:
Use QMap for small Lists.
Break even depends on the key length. It is at 10 .. 512 Elements for 1 ... 200 Elements
For our typical ConfigKey() keys it will near ~50 Elements.

[mixxxbot]

Commented by: rryan
Date: 2013-10-14T15:49:38Z

---

I've read it before. It's worth reading every article on woboq.com :) --
tons of great Qt internals posts.

On Mon, Oct 14, 2013 at 11:10 AM, Daniel Schürmann <
<email address hidden>> wrote:

Just found http://woboq.com/blog/qmap_qhash_benchmark.html (please read
discussion as well)

Conclusion:
Use QMap for small Lists.
Break even depends on the key length. It is at 10 .. 512 Elements for 1
... 200 Elements
For our typical ConfigKey() keys it will near ~50 Elements.

--
You received this bug notification because you are a member of Mixxx
Development Team, which is subscribed to Mixxx.
https://bugs.launchpad.net/bugs/1184581

Title:
Fix triple hash table lookup in midicontroller.cpp

To manage notifications about this bug go to:
https://bugs.launchpad.net/mixxx/+bug/1184581/+subscriptions

[mixxxbot]

Commented by: rryan
Date: 2014-03-29T21:29:33Z

---

I got rid of the double lookup. Now we just need to cache the CO in the MidiInutMapping..

[mixxxbot]

Commented by: kain88-de
Date: 2014-09-12T16:53:31Z

---

Did we also refactor the controller code with the controller wizard?

[mixxxbot]

Commented by: rryan
Date: 2014-11-18T07:17:24Z

---

The situation is improved but not perfect.

[mixxxbot]

Commented by: rryan
Date: 2014-11-24T16:16:25Z

---

There are 2 lookups now (1 is unavoidable).

The second one is calling CO::getControl() for each input mapping. We could cache the CO in the MIDI input mapping to eliminate this after the first lookup. The problem is that this would assume the ControlObject* for a ConfigKey never changes and that is not necessarily true (for example, skin-generated controls).

Given that we have no profiling data showing this inefficiency actually causes any problems -- moving out of 1.12.0 and marking low.