refactored & simplified Settings.hpp and pmtv format #402
Conversation
* code and templating structure (via and in response to -ftime-trace analysis) * replaced std::ranges::find(..) by more compile-time friendly version * replaced std::sort by binary-search+merge-sort * added non-inline to methods to avoid unnecessary compile-time optimisations Signed-off-by: Ralph J. Steinhagen <r.steinhagen@gsi.de>
Signed-off-by: Ralph J. Steinhagen <r.steinhagen@gsi.de>
RalphSteinhagen
force-pushed
the
pmtFormat
branch
from
b1100e0
to
954cf3f
Compare
| const std::set<std::string>& currentAutoUpdateParams = _autoUpdateParameters.at(bestMatchSettingsCtx.value()); | ||
| // auto notAutoUpdateView = parameters.value() | std::views::filter([&](const auto& pair) { return !currentAutoUpdateParams.contains(pair.first); }); | ||
| // property_map notAutoUpdateParams(notAutoUpdateView.begin(), notAutoUpdateView.end()); | ||
|
|
||
| // the following is more compile-time friendly | ||
| property_map notAutoUpdateParams; | ||
| for (const auto& pair : parameters.value()) { | ||
| if (!currentAutoUpdateParams.contains(pair.first)) { | ||
| notAutoUpdateParams.insert(pair); | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure about the gain of compilation time, but we might still need to consider using ranges, where possible?
There was a problem hiding this comment.
re gain: see numbers above. Unfortunately quite significant and not in favour of ranges. :-|
| std::vector<std::pair<SettingsCtx, property_map>>& sortedVectorForContext = _storedParameters[ctx.context]; | ||
| // binary search and merge-sort | ||
| auto it = std::ranges::lower_bound(sortedVectorForContext, ctx.time, std::less<>{}, [](const auto& pair) { return pair.first.time; }); | ||
| sortedVectorForContext.insert(it, {ctx, newParameters}); |
There was a problem hiding this comment.
std::vector::insert can be quite costly, so using std::sort might be a better option in this case. However, without concrete benchmark measurements, it's difficult to make a definitive judgment, and I don't have a strong preference either way.
There was a problem hiding this comment.
The cost for this insert sort is lower than a regular sort because the list is already sorted. I.e. in most cases the field is added to the end. In the other it's one insert followed by a linear shift ( O = N) ... much faster than any other O = N log N sorts
Details:
std::visit(...)pattern (not yet fully eliminated.Compile-time differences (~4% ... hoped to be more, but it's a start).
before:
this PR:
ClangAnalyser profile with this PR (based on Clang's
-ftime-tracefeature) in the./builddirectory:before: gr4_all_before.txt
after: gr4_all_after.txt