Skip to content

Commit 472a788

Browse files
committed
implement bimap for internal_external state map
Get hashing for inverted lookups, but incur structural overhead. Whether this is worth it depends on the number of mapped interface states and the number of pruning/reactivation requests.
1 parent 0e3ec0b commit 472a788

File tree

2 files changed

+23
-16
lines changed

2 files changed

+23
-16
lines changed

core/include/moveit/task_constructor/container_p.h

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
#include <moveit/macros/class_forward.h>
4343
#include "stage_p.h"
4444

45+
#include <boost/bimap.hpp>
46+
#include <boost/bimap/unordered_set_of.hpp>
47+
#include <boost/bimap/unordered_multiset_of.hpp>
48+
4549
#include <map>
4650
#include <climits>
4751

@@ -118,7 +122,8 @@ class ContainerBasePrivate : public StagePrivate
118122
InterfacePtr pendingForward() const { return pending_forward_; }
119123

120124
// map InterfaceStates from children to external InterfaceStates of the container
121-
const auto& internalToExternalMap() const { return internal_to_external_; }
125+
const auto& internalToExternalMap() const { return internal_external_.left; }
126+
const auto& externalToInternalMap() const { return internal_external_.right; }
122127

123128
protected:
124129
ContainerBasePrivate(ContainerBase* me, const std::string& name);
@@ -135,14 +140,16 @@ class ContainerBasePrivate : public StagePrivate
135140
child->setNextStarts(allowed ? pending_forward_ : InterfacePtr());
136141
}
137142

138-
/// copy external_state to a child's interface and remember the link in internal_to map
143+
/// copy external_state to a child's interface and remember the link in internal_external map
139144
template <Interface::Direction>
140145
void copyState(Interface::iterator external, const InterfacePtr& target, bool updated);
141146
/// lift solution from internal to external level
142147
void liftSolution(const SolutionBasePtr& solution, const InterfaceState* internal_from,
143148
const InterfaceState* internal_to);
144149

145-
auto& internalToExternalMap() { return internal_to_external_; }
150+
/// protected writable overloads
151+
inline auto& internalToExternalMap() { return internal_external_.left; }
152+
inline auto& ExternalToInternalMap() { return internal_external_.right; }
146153

147154
// set in resolveInterface()
148155
InterfaceFlags required_interface_;
@@ -151,7 +158,9 @@ class ContainerBasePrivate : public StagePrivate
151158
container_type children_;
152159

153160
// map start/end states of children (internal) to corresponding states in our external interfaces
154-
std::map<const InterfaceState*, InterfaceState*> internal_to_external_;
161+
boost::bimap<boost::bimaps::unordered_set_of<const InterfaceState*>,
162+
boost::bimaps::unordered_multiset_of<const InterfaceState*>>
163+
internal_external_;
155164

156165
/* TODO: these interfaces don't need to be priority-sorted.
157166
* Introduce base class UnsortedInterface (which is a plain list) for this use case. */

core/src/container.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,20 +109,18 @@ void ContainerBasePrivate::compute() {
109109
template <Interface::Direction dir>
110110
void ContainerBasePrivate::copyState(Interface::iterator external, const InterfacePtr& target, bool updated) {
111111
if (updated) {
112-
// TODO(v4hn): This is inefficient, consider storing inverse mapping for each start/end pair in children
113-
// for parallel containers there will be multiple internal states
114-
std::for_each(internal_to_external_.begin(), internal_to_external_.end(), [&external](auto& p) {
115-
if (p.second == &*external)
116-
setStatus<dir>(p.first, external->priority().status());
117-
});
112+
auto internals{ externalToInternalMap().equal_range(&*external) };
113+
for (auto& i = internals.first; i != internals.second; ++i) {
114+
setStatus<dir>(i->second, external->priority().status());
115+
}
118116
return;
119117
}
120118

121119
// create a clone of external state within target interface (child's starts() or ends())
122120
auto internal = states_.insert(states_.end(), InterfaceState(*external));
123121
target->add(*internal);
124122
// and remember the mapping between them
125-
internal_to_external_.insert(std::make_pair(&*internal, &*external));
123+
internalToExternalMap().insert(std::make_pair(&*internal, &*external));
126124
}
127125

128126
void ContainerBasePrivate::liftSolution(const SolutionBasePtr& solution, const InterfaceState* internal_from,
@@ -131,12 +129,12 @@ void ContainerBasePrivate::liftSolution(const SolutionBasePtr& solution, const I
131129

132130
// map internal to external states
133131
auto find_or_create_external = [this](const InterfaceState* internal, bool& created) -> InterfaceState* {
134-
auto it = internal_to_external_.find(internal);
135-
if (it != internal_to_external_.end())
136-
return it->second;
132+
auto it = internalToExternalMap().find(internal);
133+
if (it != internalToExternalMap().end())
134+
return const_cast<InterfaceState*>(it->second);
137135

138136
InterfaceState* external = &*states_.insert(states_.end(), InterfaceState(*internal));
139-
internal_to_external_.insert(std::make_pair(internal, external));
137+
internalToExternalMap().insert(std::make_pair(internal, external));
140138
created = true;
141139
return external;
142140
};
@@ -240,7 +238,7 @@ void ContainerBase::reset() {
240238
impl->pending_backward_->clear();
241239
impl->pending_forward_->clear();
242240
// ... and state mapping
243-
impl->internal_to_external_.clear();
241+
impl->internalToExternalMap().clear();
244242

245243
// interfaces depend on children which might change
246244
impl->required_interface_ = UNKNOWN;

0 commit comments

Comments
 (0)