mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.45k stars 1.27k forks source link

Cache ControlObject lookups in MidiController::processInputMapping #7057

Open mixxxbot opened 2 years ago

mixxxbot commented 2 years ago

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 2 years ago

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:

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

mixxxbot commented 2 years ago

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 2 years ago

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 2 years ago

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 2 years ago

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 2 years ago

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


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

mixxxbot commented 2 years ago

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


The situation is improved but not perfect.

mixxxbot commented 2 years ago

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.