📊 Sort spikes. #2433
📊 Sort spikes. #2433
Conversation
…e' into perf/better-sorting-in-event-queue
arbor/backends/event.hpp
Outdated
| deliverable_event_data(cell_local_size_type idx, float w): | ||
| cell_local_size_type mech_index = 0; // same as target_handle::mech_index | ||
| float weight = 0; | ||
| deliverable_event_data(const cell_local_size_type idx, const float w) noexcept: |
There was a problem hiding this comment.
Sorry (sort-of!) for the drive-by comment without any other engagement on this PR, but it's an almost trivial thing: I think we shouldn't be using const in parameter declarations such as this where it has no semantic value.
The reason is that it clutters the signature and is a bit misleading; having a const there, being unidiomatic, trips up the reader, prompting them to ponder, why are these arguments marked so? Looking at this code, I might be lead to think that cell_local_size_type must be some sort of type alias for a reference type, when of course it isn't.
There was a problem hiding this comment.
Sure, that's not meant for merging, but rather me being confused about the compiler error (and behaviour)
There was a problem hiding this comment.
It turned out that newer (like installed on my machine) compilers allow emplace_back for aggregates in C++20 mode, but older ones do not. So, some extra work was needed.
Replace custom
introsortwithstd::sort.