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

Unify vertex heaps #32

Merged
merged 2 commits into from
Sep 19, 2022
Merged

Unify vertex heaps #32

merged 2 commits into from
Sep 19, 2022

Conversation

magnushiie
Copy link
Contributor

This unifies forwardHeap and backwardHeap - only one of the bidirectionalVertex's fields of queryDist or revQueryDistance was ever used simultaneously in one direction. This reduces memory usage a bit (10%), and helps further refactorings.

@LdDl LdDl self-assigned this Sep 19, 2022
@LdDl LdDl added enhancement New feature or request help wanted Extra attention is needed labels Sep 19, 2022
@LdDl
Copy link
Owner

LdDl commented Sep 19, 2022

@SimonWaldherr
Are you able to check if you have "Resolve Conflicts" button?

image
I do not see this button in my view (I guess it's role model policy by GitHub, since you are the author of PR)

From GitHub docs button should be located at right top corner:
image

@magnushiie
Copy link
Contributor Author

I can resolve the conflicts, sorry, it was an artifact of separating the changes to multiple PRs. Will resolve in 1h

@magnushiie
Copy link
Contributor Author

Should be OK now.

@LdDl
Copy link
Owner

LdDl commented Sep 19, 2022

Should be OK now.

Thanks!

As I see, basically you've got rid of redundant queryDist/revQueryDistance with theirs parent structs by defining single struct to rule them all (dist field in vertexDist struct). Initialization with Infinity value (queryDist or revQueryDistance - which is depends on either forward or backward propagation) leads to additional memory consumption (could be really big on large graphs). Did I get it right?

@magnushiie
Copy link
Contributor Author

Yes, my understanding is the infinity values were never used nor updated.

@LdDl LdDl merged commit a54bac7 into LdDl:master Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants