Skip to content

Commit

Permalink
refactor: use temporary QHash to reduce algorithmic complexity
Browse files Browse the repository at this point in the history
  • Loading branch information
Swiftb0y committed Mar 13, 2023
1 parent 1f46f97 commit d1de41a
Showing 1 changed file with 22 additions and 13 deletions.
35 changes: 22 additions & 13 deletions src/effects/presets/effectpreset.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#include "effects/presets/effectpreset.h"

#include <QHash>
#include <functional>

#include "effects/backends/effectsbackend.h"
#include "effects/effectslot.h"
#include "effects/presets/effectxmlelements.h"
Expand Down Expand Up @@ -102,19 +105,25 @@ const QDomElement EffectPreset::toXml(QDomDocument* doc) const {
void EffectPreset::updateParametersFrom(const EffectPreset& other) {
DEBUG_ASSERT(backendType() == other.backendType());

// technically algorithmically inefficient solution O(n²). May be
// optimizable by sorting first, gains depend on parameter count
for (const auto& parameterToCopy : other.m_effectParameterPresets) {
auto currentParameterIt =
std::find_if(m_effectParameterPresets.begin(),
m_effectParameterPresets.end(),
[&](const auto& ourParameter) {
return ourParameter.id() == parameterToCopy.id();
});
if (currentParameterIt == m_effectParameterPresets.end()) {
continue;
// we use a std::reference_wrapper as optimization so we don't copy and
// store the entire object when we need to copy later anyways
// TODO(XXX): Replace QHash with <std::flat_map>
QHash<QString, std::reference_wrapper<const EffectParameterPreset>> parameterPresetLookup;
// we build temporary hashtable to reduce the algorithmic complexity of the
// lookup later. A plain std::find_if (O(n)) would've resulted in O(n^n)
// parameter updating. The hashtable has O(1) lookup with O(n) updating,
// though with more constant overhead. Since 3rd-party EffectPresets could
// have hundreds of parameters, we can't afford O(n^2) lookups.
// TODO(XXX): measure overhead and possibly implement fallback to lower
// overhead O(n^2) solution for small presets.
for (const EffectParameterPreset& preset : other.m_effectParameterPresets) {
parameterPresetLookup.insert(preset.id(), std::ref(preset));
}

for (EffectParameterPreset& parameterToUpdate : m_effectParameterPresets) {
auto parameterToCopy = parameterPresetLookup.find(parameterToUpdate.id());
if (parameterToCopy != parameterPresetLookup.end()) {
parameterToUpdate = *parameterToCopy;
}
// overwrite our parameter by taking a copy of the same parameter from `other`
*currentParameterIt = parameterToCopy;
}
}

0 comments on commit d1de41a

Please sign in to comment.