naviqore / public-transit-service

Public transit schedule information and connection routing service based on GTFS data and the RAPTOR algorithm.
MIT License
6 stars 1 forks source link

Feature/nav 181 initialize hashset size of marked stops #118

Closed clukas1 closed 2 months ago

clukas1 commented 2 months ago

Created new pull request, to allow you to review the request.

Change is that boolean[] arrays are used to mask routesToScan and markedStops instead of HashSets. Performance has been improved significantly.

Original:

image

With Hashset set to size of Stops/Routes

image

With Boolean Arrays

image

munterfi commented 2 months ago

Thanks @clukas1, looks good.

It is a bit difficult to read the code and follow in the debugger due to passing around all the arrays.

In this commit https://github.com/naviqore/public-transit-service/commit/9236013c47853156a832da2bc3049393c3e5f5f9 on the branch feature/NAV-181-extract-marked-stops-class I extracted a MarkedStops class to encapsulate the logic of the stop mask. In performance there is not really a difference (some ms faster):

Before: 2024_09_16_13_39_42_raptor_results

After: 2024_09_16_18_53_47_raptor_results

Then i tried to get rid of the array copy / new allocation in https://github.com/naviqore/public-transit-service/commit/7e69a2688a4b0709bbd09abc121bf51f669567d0, by using an int array instead boolean, tracking the rounds. But somehow the logic messes up...

Should we introduce a MarkedStops (or other name, e.g. StopMask) class, to track the marked stops? And possibly omit the rest on the branch...

clukas1 commented 2 months ago

@munterfi I've moved the logic of markedStopsMasks to the Query State because they fit in there nicely. The int array will not work, because you will sometimes overwrite a to be checked mark to the new round potentially preventing finding ideal solutions.

benchmarking the both solutions (before and after refactoring) shows following results, probably just noise holding them apart.

before refactoring

before_refactoring

after refactoring

after_refactoring

munterfi commented 2 months ago

Thanks, looks good to merge.

2024_09_17_09_39_47_raptor_results