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

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

Merged

Conversation

clukas1
Copy link
Member

@clukas1 clukas1 commented Sep 16, 2024

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

@clukas1 clukas1 requested a review from munterfi September 16, 2024 09:22
@clukas1 clukas1 self-assigned this Sep 16, 2024
@munterfi
Copy link
Member

munterfi commented Sep 16, 2024

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 9236013 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 7e69a26, 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
Copy link
Member Author

clukas1 commented Sep 16, 2024

@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
Copy link
Member

Thanks, looks good to merge.

2024_09_17_09_39_47_raptor_results

@munterfi munterfi merged commit f6b1e32 into main Sep 17, 2024
2 checks passed
@munterfi munterfi deleted the feature/NAV-181-initialize-hashset-size-of-marked-stops branch September 17, 2024 07:57
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