Skip to content

Commit

Permalink
Kraken: Improve /line_reports perf
Browse files Browse the repository at this point in the history 
Apply PTRef filters only once to all before iterating on lines.
This greatly improves perfs when there is a "difficult" PTRef filter and
still a lot of lines to iterate on (like Bus mode on Paris region).

Perfs:
* measures done without any disruption loaded locally:
/line_reports with difficult filter, the gain is proportional to the number of lines in
response (gain x750 v1/physical_modes/Bus/line_reports on Paris for
1350 lines).
Little gains without filter.

* with all disruptions on Paris region after improvement:
Compared to no-disruption kraken's durations are between x1 and x2, which
is more than acceptable.

* No point in removing shortcuts in PTRef, the time to build candidates to
removal is too short.

JIRA: https://navitia.atlassian.net/browse/NAV-2150

----

Ideas not explored (by order of expected ROI):
* Less has_applicable_message() calls: prefilter subobjects on
  has_applicable_message (hard to do just once with the line being
  mandatory for rail/line-sections).
  - Probably differenciate prefiltered valid without considering
    line/rail-sections and the other which are only rail/line-sections
* Smaller contains() calls on routes: consume prefiltered routes when
  building LineReport (linked to a single line)
* No contains() call on networks: iterate on prefiltered networks to
  build LineReport (line linked to a single network).
  Lines will have to be correctly sorted after.
* Consider pagination sooner in processing to save some work
* Dig more on ideas left in #2949

Measure details:
* v1/physical_modes/Bus/route_schedules?count=25&items_per_schedule=10:
  same perf
* v1/line_reports:
  Paris 100ms > 50ms (gain x2).
* v1/networks/<main bus network>/line_reports:
  Paris 4.25s > 40ms (gain x106), Lyon 2.58s > 35ms (gain x73)
* v1/commercial_modes/Bus/line_reports:
  Paris 67.7s > 130ms (gain x520), Lyon 411ms > 12ms (gain x34)
* v1/physical_modes/Bus/line_reports:
  Paris 181.7s > 240ms (gain x750), Lyon 6.45s > 54ms (gain x119)
* Building of PTRef shortcuts between route and SA/SP:
  takes only 0.45s on Paris region, 0.35s on Lyon.
  -> Not worth removing the shortcut
  • Loading branch information
Pierre-Etienne Bougue committed Sep 11, 2023
1 parent 4d1dbcd commit eeaa64e
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 39 deletions.
100 changes: 68 additions & 32 deletions source/disruption/line_reports_api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,50 +38,64 @@ namespace nt = navitia::type;
namespace navitia {
namespace disruption {

struct PreFilteredLineReportSubObjects {
type::Indexes networks;
type::Indexes routes;
type::Indexes stop_areas;
type::Indexes stop_points;
};

struct LineReport {
const nt::Line* line;
std::vector<const nt::Network*> networks;
std::vector<const nt::Route*> routes;
std::vector<const nt::StopArea*> stop_areas;
std::vector<const nt::StopPoint*> stop_points;

// Using optional on 'pre_filtered_sub_objects' as the filters are most of the time empty.
// "none" optional means no filter is applied, and saves costly contains() calls.
LineReport(const nt::Line* line,
const std::string& filter,
const std::vector<std::string>& forbidden_uris,
const std::vector<nt::disruption::ActiveStatus>& filter_status,
const type::Data& d,
const boost::posix_time::ptime now,
const boost::posix_time::time_period& filter_period)
const boost::posix_time::time_period& filter_period,
boost::optional<PreFilteredLineReportSubObjects>& pre_filtered_sub_objects)
: line(line) {
add_objects(filter, forbidden_uris, filter_status, d, now, filter_period, networks);
add_objects(filter, forbidden_uris, filter_status, d, now, filter_period, routes);
add_objects(filter, forbidden_uris, filter_status, d, now, filter_period, stop_areas);
add_objects(filter, forbidden_uris, filter_status, d, now, filter_period, stop_points);
}

template <typename T>
void add_objects(const std::string& filter,
const std::vector<std::string>& forbidden_uris,
const std::vector<nt::disruption::ActiveStatus>& filter_status,
const type::Data& d,
const boost::posix_time::ptime now,
const boost::posix_time::time_period& filter_period,
std::vector<const T*>& objects) {
std::string new_filter = "line.uri=" + line->uri;
if (!filter.empty()) {
new_filter += " and " + filter;
const auto network = line->network;
if ((!pre_filtered_sub_objects || pre_filtered_sub_objects->networks.contains(network->idx))
&& network->has_applicable_message(now, filter_period, filter_status, line)) {
networks.push_back(network);
}

type::Indexes indices;
try {
indices = ptref::make_query(nt::get_type_e<T>(), new_filter, forbidden_uris, d);
} catch (const std::exception&) {
}
// remember already visited SA & SP to avoid useless checks
std::set<nt::idx_t> visited_sa;
std::set<nt::idx_t> visited_sp;
for (const auto route : line->route_list) {
for (const auto sa : route->stop_area_list) {
if (!navitia::contains(visited_sa, sa->idx)
&& (!pre_filtered_sub_objects || pre_filtered_sub_objects->stop_areas.contains(sa->idx))
&& sa->has_applicable_message(now, filter_period, filter_status, line)) {
stop_areas.push_back(sa);
}
visited_sa.insert(sa->idx);
}

for (const auto sp : route->stop_point_list) {
if (!navitia::contains(visited_sp, sp->idx)
&& (!pre_filtered_sub_objects || pre_filtered_sub_objects->stop_points.contains(sp->idx))
&& sp->has_applicable_message(now, filter_period, filter_status, line)) {
stop_points.push_back(sp);
}
visited_sp.insert(sp->idx);
}

for (const auto& idx : indices) {
const auto* obj = d.pt_data->collection<T>()[idx];
if (obj->has_applicable_message(now, filter_period, filter_status, line)) {
objects.push_back(obj);
if (!pre_filtered_sub_objects || pre_filtered_sub_objects->routes.contains(route->idx)) {
// /!\ One could think of processing SA/SP once this prefilter is
// applied (because PTRef is stable/propagates correctly).
// /!\ But if routes are also prefiltered on having disruptions, then
// responses would be incorrect (not exploring valid SA/SP)
if (route->has_applicable_message(now, filter_period, filter_status, line)) {
routes.push_back(route);
}
}
}
}
Expand All @@ -108,6 +122,18 @@ struct LineReport {
}
};

static type::Indexes safe_filter(const type::Type_e requested_type,
const std::string& request,
const std::vector<std::string>& forbidden_uris,
const type::Data& data) {
type::Indexes filtered_indices;
try {
filtered_indices = ptref::make_query(requested_type, request, forbidden_uris, data);
} catch (const std::exception&) {
}
return filtered_indices;
}

void line_reports(navitia::PbCreator& pb_creator,
const navitia::type::Data& d,
const size_t depth,
Expand Down Expand Up @@ -137,10 +163,20 @@ void line_reports(navitia::PbCreator& pb_creator,
pb_creator.fill_pb_error(pbnavitia::Error::bad_filter, "ptref : " + ptref_error.more);
return;
}

boost::optional<PreFilteredLineReportSubObjects> pre_filtered_sub_objects;
if (!filter.empty() || !forbidden_uris.empty()) {
pre_filtered_sub_objects = PreFilteredLineReportSubObjects();
pre_filtered_sub_objects->networks = safe_filter(type::Type_e::Network, filter, forbidden_uris, d);
pre_filtered_sub_objects->routes = safe_filter(type::Type_e::Route, filter, forbidden_uris, d);
pre_filtered_sub_objects->stop_areas = safe_filter(type::Type_e::StopArea, filter, forbidden_uris, d);
pre_filtered_sub_objects->stop_points = safe_filter(type::Type_e::StopPoint, filter, forbidden_uris, d);
}

std::vector<LineReport> line_reports;
for (auto idx : line_indices) {
auto line_report = LineReport(d.pt_data->lines[idx], filter, forbidden_uris, filter_status, d, pb_creator.now,
pb_creator.action_period);
auto line_report = LineReport(d.pt_data->lines[idx], filter_status, pb_creator.now, pb_creator.action_period,
pre_filtered_sub_objects);
if (line_report.has_disruption(pb_creator.now, pb_creator.action_period, filter_status)) {
line_reports.push_back(line_report);
}
Expand Down
4 changes: 2 additions & 2 deletions source/type/data.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ static void build_datasets(navitia::type::VehicleJourney* vj) {
*
* @param vj The vehicle journey to browse
*/
static void build_route_and_stop_point_relations(navitia::type::VehicleJourney* vj) {
static void build_route_and_stops_relations(navitia::type::VehicleJourney* vj) {
for (navitia::type::StopTime& st : vj->stop_time_list) {
if (st.stop_point) {
vj->route->stop_point_list.insert(st.stop_point);
Expand Down Expand Up @@ -443,7 +443,7 @@ void Data::build_relations() {
// physical_mode_list of line
for (auto* vj : pt_data->vehicle_journeys) {
build_datasets(vj);
build_route_and_stop_point_relations(vj);
build_route_and_stops_relations(vj);
if (!vj->physical_mode || !vj->route || !vj->route->line) {
continue;
}
Expand Down
10 changes: 5 additions & 5 deletions source/type/type_interfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ bool HasMessages::has_applicable_message(const boost::posix_time::ptime& current
if (!impact) {
continue; // pointer might still have become invalid
}
if (line && impact->is_only_line_section() && !impact->is_line_section_of(*line)) {
continue;
}
if (line && impact->is_only_rail_section() && !impact->is_rail_section_of(*line)) {
continue;
if (line) {
if ((impact->is_only_line_section() && !impact->is_line_section_of(*line))
|| (impact->is_only_rail_section() && !impact->is_rail_section_of(*line))) {
continue;
}
}
if (impact->is_valid(current_time, action_period)) {
// if filter empty == no filter
Expand Down

0 comments on commit eeaa64e

Please sign in to comment.