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

Kraken: Improve /line_reports perf #4104

Merged
merged 2 commits into from
Sep 13, 2023
Merged

Kraken: Improve /line_reports perf #4104

merged 2 commits into from
Sep 13, 2023

Conversation

pbougue
Copy link
Contributor

@pbougue pbougue commented Sep 11, 2023

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 slower, 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


Details

Ideas not explored (by order of expected ROI):

  • Less has_applicable_message() calls: prefilter sub-objects 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 PTRef: Some perf improvements for /line_reports #2949

Measures:

  • 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

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
As flat_set doesn't provide contains() in boost 1.55
@sonarcloud
Copy link

sonarcloud bot commented Sep 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

78.5% 78.5% Coverage
0.0% 0.0% Duplication

@azime azime merged commit e5e4631 into dev Sep 13, 2023
9 checks passed
@azime azime deleted the improve_line_reports_perf branch September 13, 2023 09:47
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.

4 participants