Skip to content

Commit e554624

Browse files
mjjbellmjjbell
committed
Reduce copying in API parameter constructors
When using non-default constructors for the API parameter classes, vector arguments like coordinates and hints are copied at least once (twice when passed as lvalue arguments). Enable perfect forwarding of BaseParameter arguments and pass-by-value in the constructor that uses the argument. This ensures we copy at most once (zero for rvalue arguments).
1 parent 58ba3fc commit e554624

File tree

5 files changed

+31
-27
lines changed

5 files changed

+31
-27
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
- REMOVED: we no longer publish Node 8 binary modules (they are still buildable from source) [#5918](https://github.com/Project-OSRM/osrm-backend/pull/5918)
77
- Routing:
88
- FIXED: Avoid copying ManyToMany table results [#5923](https://github.com/Project-OSRM/osrm-backend/pull/5923)
9+
- FIXED: Reduce copying in API parameter constructors [#5925](https://github.com/Project-OSRM/osrm-backend/pull/5925)
910
- Misc:
1011
- CHANGED: Unify `.osrm.turn_penalites_index` dump processing same with `.osrm.turn_weight_penalties` and `.osrm.turn_duration_penalties` [#5868](https://github.com/Project-OSRM/osrm-backend/pull/5868)
1112
- Profile:

‎include/engine/api/base_parameters.hpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,21 @@ struct BaseParameters
9292

9393
SnappingType snapping = SnappingType::Default;
9494

95-
BaseParameters(const std::vector<util::Coordinate> coordinates_ = {},
96-
const std::vector<boost::optional<Hint>> hints_ = {},
95+
BaseParameters(std::vector<util::Coordinate> coordinates_ = {},
96+
std::vector<boost::optional<Hint>> hints_ = {},
9797
std::vector<boost::optional<double>> radiuses_ = {},
9898
std::vector<boost::optional<Bearing>> bearings_ = {},
9999
std::vector<boost::optional<Approach>> approaches_ = {},
100100
bool generate_hints_ = true,
101101
std::vector<std::string> exclude = {},
102102
const SnappingType snapping_ = SnappingType::Default)
103-
: coordinates(coordinates_), hints(hints_), radiuses(radiuses_), bearings(bearings_),
104-
approaches(approaches_), exclude(std::move(exclude)), generate_hints(generate_hints_),
105-
snapping(snapping_)
103+
: coordinates(std::move(coordinates_)), hints(std::move(hints_)),
104+
radiuses(std::move(radiuses_)), bearings(std::move(bearings_)),
105+
approaches(std::move(approaches_)), exclude(std::move(exclude)),
106+
generate_hints(generate_hints_), snapping(snapping_)
106107
{
107108
}
108109

109-
// FIXME add validation for invalid bearing values
110110
bool IsValid() const
111111
{
112112
return (hints.empty() || hints.size() == coordinates.size()) &&
@@ -115,7 +115,7 @@ struct BaseParameters
115115
(approaches.empty() || approaches.size() == coordinates.size()) &&
116116
std::all_of(bearings.begin(),
117117
bearings.end(),
118-
[](const boost::optional<Bearing> bearing_and_range) {
118+
[](const boost::optional<Bearing> &bearing_and_range) {
119119
if (bearing_and_range)
120120
{
121121
return bearing_and_range->IsValid();

‎include/engine/api/match_parameters.hpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,20 @@ struct MatchParameters : public RouteParameters
6868
}
6969

7070
template <typename... Args>
71-
MatchParameters(std::vector<unsigned> timestamps_, GapsType gaps_, bool tidy_, Args... args_)
72-
: MatchParameters(std::move(timestamps_), gaps_, tidy_, {}, std::forward<Args>(args_)...)
71+
MatchParameters(const std::vector<unsigned> &timestamps_,
72+
GapsType gaps_,
73+
bool tidy_,
74+
Args &&... args_)
75+
: MatchParameters(timestamps_, gaps_, tidy_, {}, std::forward<Args>(args_)...)
7376
{
7477
}
7578

7679
template <typename... Args>
7780
MatchParameters(std::vector<unsigned> timestamps_,
7881
GapsType gaps_,
7982
bool tidy_,
80-
std::vector<std::size_t> waypoints_,
81-
Args... args_)
83+
const std::vector<std::size_t> &waypoints_,
84+
Args &&... args_)
8285
: RouteParameters{std::forward<Args>(args_)..., waypoints_}, timestamps{std::move(
8386
timestamps_)},
8487
gaps(gaps_), tidy(tidy_)

‎include/engine/api/route_parameters.hpp‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ struct RouteParameters : public BaseParameters
8787
const GeometriesType geometries_,
8888
const OverviewType overview_,
8989
const boost::optional<bool> continue_straight_,
90-
Args... args_)
90+
Args &&... args_)
9191
// Once we perfectly-forward `args` (see #2990) this constructor can delegate to the one
9292
// below.
9393
: BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
@@ -105,7 +105,7 @@ struct RouteParameters : public BaseParameters
105105
const GeometriesType geometries_,
106106
const OverviewType overview_,
107107
const boost::optional<bool> continue_straight_,
108-
Args... args_)
108+
Args &&... args_)
109109
: BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
110110
number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_},
111111
annotations_type{annotations_ ? AnnotationsType::All : AnnotationsType::None},
@@ -123,12 +123,12 @@ struct RouteParameters : public BaseParameters
123123
const GeometriesType geometries_,
124124
const OverviewType overview_,
125125
const boost::optional<bool> continue_straight_,
126-
Args... args_)
126+
Args &&... args_)
127127
: BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
128128
number_of_alternatives{alternatives_ ? 1u : 0u},
129-
annotations{annotations_ == AnnotationsType::None ? false : true},
130-
annotations_type{annotations_}, geometries{geometries_}, overview{overview_},
131-
continue_straight{continue_straight_}, waypoints()
129+
annotations{annotations_ != AnnotationsType::None}, annotations_type{annotations_},
130+
geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_},
131+
waypoints()
132132
{
133133
}
134134

@@ -141,12 +141,12 @@ struct RouteParameters : public BaseParameters
141141
const OverviewType overview_,
142142
const boost::optional<bool> continue_straight_,
143143
std::vector<std::size_t> waypoints_,
144-
const Args... args_)
144+
const Args &&... args_)
145145
: BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
146146
number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_},
147147
annotations_type{annotations_ ? AnnotationsType::All : AnnotationsType::None},
148148
geometries{geometries_}, overview{overview_},
149-
continue_straight{continue_straight_}, waypoints{waypoints_}
149+
continue_straight{continue_straight_}, waypoints{std::move(waypoints_)}
150150
{
151151
}
152152

@@ -159,12 +159,12 @@ struct RouteParameters : public BaseParameters
159159
const OverviewType overview_,
160160
const boost::optional<bool> continue_straight_,
161161
std::vector<std::size_t> waypoints_,
162-
Args... args_)
162+
Args &&... args_)
163163
: BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
164-
number_of_alternatives{alternatives_ ? 1u : 0u},
165-
annotations{annotations_ == AnnotationsType::None ? false : true},
164+
number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_ !=
165+
AnnotationsType::None},
166166
annotations_type{annotations_}, geometries{geometries_}, overview{overview_},
167-
continue_straight{continue_straight_}, waypoints{waypoints_}
167+
continue_straight{continue_straight_}, waypoints{std::move(waypoints_)}
168168
{
169169
}
170170

‎include/engine/api/table_parameters.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ struct TableParameters : public BaseParameters
8585
template <typename... Args>
8686
TableParameters(std::vector<std::size_t> sources_,
8787
std::vector<std::size_t> destinations_,
88-
Args... args_)
88+
Args &&... args_)
8989
: BaseParameters{std::forward<Args>(args_)...}, sources{std::move(sources_)},
9090
destinations{std::move(destinations_)}
9191
{
@@ -95,7 +95,7 @@ struct TableParameters : public BaseParameters
9595
TableParameters(std::vector<std::size_t> sources_,
9696
std::vector<std::size_t> destinations_,
9797
const AnnotationsType annotations_,
98-
Args... args_)
98+
Args &&... args_)
9999
: BaseParameters{std::forward<Args>(args_)...}, sources{std::move(sources_)},
100100
destinations{std::move(destinations_)}, annotations{annotations_}
101101
{
@@ -108,7 +108,7 @@ struct TableParameters : public BaseParameters
108108
double fallback_speed_,
109109
FallbackCoordinateType fallback_coordinate_type_,
110110
double scale_factor_,
111-
Args... args_)
111+
Args &&... args_)
112112
: BaseParameters{std::forward<Args>(args_)...}, sources{std::move(sources_)},
113113
destinations{std::move(destinations_)}, fallback_speed{fallback_speed_},
114114
fallback_coordinate_type{fallback_coordinate_type_}, annotations{annotations_},
@@ -122,7 +122,7 @@ struct TableParameters : public BaseParameters
122122
if (!BaseParameters::IsValid())
123123
return false;
124124

125-
// Distance Table makes only sense with 2+ coodinates
125+
// Distance Table makes only sense with 2+ coordinates
126126
if (coordinates.size() < 2)
127127
return false;
128128

0 commit comments

Comments
 (0)