Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Significantly speed up applying GTFS-RT updates #73

Conversation

vkrause
Copy link
Member

@vkrause vkrause commented Feb 27, 2024

With the national feeds from CH, NL and NO the time to process the updates goes down from about a minute to a second here. The cost of this was dominated by vecvec::push_back in rt_timetable::add_rt_transport.

Before:
gtfs-rt-update-before

After:
gtfs-rt-update-after

@felixguendling
Copy link
Member

To prevent bad memory accesses, I think it would make sense to call operator[](index) as a replacement for the resize calls. The code (routing, etc.) accesses the rt_timetable as const, so the const member function is called:

https://github.com/felixguendling/cista/blob/a46b7d806b6766175af2da3a45c4cc3f78a2d2c1/include/cista/containers/mutable_fws_multimap.h#L387-L397

This has an assert for debug mode but will crash in release mode. The rest of the code assumes that the vector has n_locations entries. As probably not all locations are touched during the real-time update, the update needs to call operator[](n_locations) instead of resize to prevent invalid memory accesses in routing where the non-resizing const member is used.

It probably makes sense to give mutable_fws_multimap the same interface as vecvec (so not mirroring a std::multimap but rather a std::vector<std::vector<T>>).

@@ -12,7 +12,6 @@ rt_timetable create_rt_timetable(timetable const& tt,
rtt.bitfields_ = tt.bitfields_;
rtt.base_day_ = base_day;
rtt.base_day_idx_ = tt.day_idx(rtt.base_day_);
rtt.location_rt_transports_.resize(tt.n_locations());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rtt.location_rt_transports_[tt.n_locations() - 1U];  // resize for later memory accesses

With the national feeds from CH, NL and NO the time to process the updates
goes down from about a minute to a second here. The cost of this was
dominated by vecvec::push_back in rt_timetable::add_rt_transport.
@vkrause vkrause force-pushed the work/vkrause/optimize-gtfs-rt-updates branch from 53709f2 to f208e34 Compare February 27, 2024 17:59
@felixguendling felixguendling merged commit e8b976e into motis-project:master Feb 29, 2024
12 checks passed
felixguendling added a commit to motis-project/motis that referenced this pull request Feb 29, 2024
felixguendling added a commit to motis-project/motis that referenced this pull request Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants