Make Raptor transfer cost calculation done on demand only when it is accessed for caches generated per request

[miklcct]

Summary

Do not calculate all the transfers immediately when a Raptor transfer cache is not hit and created as a result of a request. Calculate them only when Raptor requires it.

Issue

This improve the problems reported in #6781 and #6312, but not in all cases.

Unit tests

Added for the classes to ensure that the pre-cached and on-demand implementations both behave as expected.

Documentation

N/A

Changelog

N/A

Bumping the serialization version id

Not needed

[codecov]

Codecov Report

❌ Patch coverage is 47.14286% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.20%. Comparing base (7d31996) to head (a52edcf).
⚠️ Report is 200 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...oradapter/transit/OnDemandRaptorTransferIndex.java	0.00%	34 Missing ⚠️
...thm/raptoradapter/transit/RaptorTransferIndex.java	75.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6792      +/-   ##
=============================================
+ Coverage      72.15%   72.20%   +0.04%     
- Complexity     19782    19917     +135     
=============================================
  Files           2151     2163      +12     
  Lines          79981    80252     +271     
  Branches        8061     8107      +46     
=============================================
+ Hits           57713    57942     +229     
- Misses         19422    19451      +29     
- Partials        2846     2859      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[miklcct]

As discussed in the dev meeting, I will add a new feature flag (disabled by default) to enable the use of the new class, otherwise we will keep the original behavior.

[t2gran]

Looks good, but fix the "keep history on the RaptorTransferIndex" so I can verify that there are no changes.

[t2gran]

Parallell generation of transfers can not be turned of any more:
Old code - missing:

    // we want to always parallelize the cache building during the startup
    // and only parallelize during runtime requests if the feature flag is on
    if (requestSource == RequestSource.SETUP || OTPFeature.ParallelRouting.isOn()) {
      stopIndices = stopIndices.parallel();
    }

[miklcct]

Thanks for your comment.

[leonardehrenfried]

I'm finding it very difficult to verify that the current behaviour has not changed but I compiled the code and ran it on one of my deployments and could not find a problem.

[t2gran]

This PR is very difficult to review:

• When extracting an interface it is important to rename the class and commit - no other changes should be in that commit.
• Mistakes, doc and unit-tests added at the end of working with a PR should be rebased into the proper place in the git history. Commits should be atomic containing all changes for a "mini-feature" - the history should not be your work history.
• Needed refactoring should be extracted into separate commits, and preferable be put first - or sometimes in a separate PR.

I am unsure what to do. I consider approving this - and be better at making the PRs easier for review in the future. We can discuss this in the meeting tomorrow.

[leonardehrenfried]

I could live with that risk.

[miklcct]

My original intention was to move the implementation to a new class and change the class to be an interface, such that the user of the class would not see any changes.

[miklcct]

I'll rewrite the history again to further clarify my intention.

[t2gran]

@miklcct Please wait until we have discussed this in tomorrows meeting.

[t2gran]

I think this need to be thread safe?

[miklcct]

In Java, object assignment is thread safe because it is an atomic operation. There can be race condition here between the check and the assignment but the assignment is always the same value. Will it cause a problem?

[miklcct]

If you think it is necessary, I can create a lock object for every stop index.

[optionsome]

We discussed this in the dev meeting. This can stay as is. @miklcct can add a comment to the code explaining that this is not fully thread-safe but not worth it to synchronize it.