Repository navigation
Perfomance improvement for flex access/egress searches - #3661
Merged
leonardehrenfried merged 15 commits intoNov 4, 2021
Merged
Conversation
Closed
leonardehrenfried
force-pushed
the
access-egress-debug
branch
from
October 20, 2021 10:42
1e95755 to
15bf538
Compare
hannesj
previously approved these changes
Oct 20, 2021
Member
Author
|
After having identified the linestring computation as a major bottleneck during the the dev meeting I added a commit which only generates them when they are actually needed, not for the thousands of possible paths that will never be presented to the user. This leads to even more significant performance gains. Typical stats are these: |
leonardehrenfried
force-pushed
the
access-egress-debug
branch
from
October 27, 2021 08:03
7fbcf17 to
92d0f8c
Compare
hannesj
reviewed
Oct 27, 2021
Member
Author
|
I have also implemented what we talked about in the meeting: limiting the distance that a transfer can take when getting in or out of a flex vehicle. If I configure it to be 300m then I typically get the following times: |
hannesj
reviewed
Oct 29, 2021
leonardehrenfried
force-pushed
the
access-egress-debug
branch
from
October 30, 2021 05:22
34ae3e5 to
5fbf2da
Compare
hannesj
previously approved these changes
Nov 1, 2021
t2gran
requested changes
Nov 2, 2021
| *Config parameters* | ||
|
|
||
| - `maxTransferMeters`: how far should you a passenger be able to walk after getting out of a flex | ||
| vehicle and transferring to another one. |
Member
There was a problem hiding this comment.
"another one" - Another FLEX or Transit vehicle?
Comment on lines
+37
to
+40
| - `maxTransferMeters`: how far should you a passenger be able to walk after getting out of a flex | ||
| vehicle and transferring to another one. | ||
| This was mainly introduced to improve performance - a lower value means that | ||
| the routing is faster. |
Member
There was a problem hiding this comment.
Is there an upper limit? This is equivalent to the maxTransferDurationSeconds, so we should use the same unit.
… the linestring to the latest possible time
…edTrip.java Co-authored-by: Thomas Gran <t2gran@gmail.com>
leonardehrenfried
force-pushed
the
access-egress-debug
branch
from
November 3, 2021 09:13
9f755cd to
85fbfb5
Compare
t2gran
reviewed
Nov 3, 2021
t2gran
reviewed
Nov 3, 2021
t2gran
reviewed
Nov 3, 2021
leonardehrenfried
force-pushed
the
access-egress-debug
branch
from
November 3, 2021 09:41
849d6df to
da5bae8
Compare
leonardehrenfried
force-pushed
the
access-egress-debug
branch
from
November 3, 2021 09:46
eb188e7 to
3ab9c93
Compare
t2gran
previously approved these changes
Nov 3, 2021
Member
Author
|
I also took the liberty to also bring the configuration for the vector tiles in line with what @t2gran has requested for the flex one. |
Member
The long term goal here is to modularize the code a bit, and hopefully this will make it a bit simpler to infer a dependency-injection framework. |
hannesj
reviewed
Nov 3, 2021
hannesj
approved these changes
Nov 3, 2021
t2gran
approved these changes
Nov 4, 2021
t2gran
pushed a commit
that referenced
this pull request
Nov 4, 2021
leonardehrenfried
added
IBI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
We noticed a severe performance problem for flex access/egress searches which would typically be 5-10 times slower than flex direct searches. I also saw that for flex access/egress searches a large number (25000 in my simple case with a hand full of flex routes) of accesses are computed. For this reason I added their number to the debug output.
After a bit of investigation I noticed some performance improvements in #3499 which I extracted into this separate PR. The core of this solution is to stop every flex path from being traversed twice.
Results
Before:
After:
The improvement is very noticeable but in my view it went only from terrible to bad.
To achieve an even better performance I think we need to dramatically lower the number of accesses being computed.
For example, if I run the same request without flex, the timing looks like this:
Unit tests
I would love to write a unit test for flex but I think I need some guidance on how to do it.
Code style
Yes.
Documentation
I've commented the code.
Changelog
Yes