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

[master < T981] Reduce all shortest paths memory usage #981

Merged
merged 10 commits into from Jul 30, 2023

Conversation

antoniofilipovic
Copy link
Collaborator

@antoniofilipovic antoniofilipovic commented Jun 13, 2023

TLDR: Based on the mgbench results, memory usage dropped from gigabytes to a few megabytes, while performance on all shortest paths dropped ~7% -> the decision is to merge because of the significant gain in memory usage while keeping performance high.

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

To keep docs changelog up to date, one more thing to do:

  • Write a release note here
  • Tag someone from docs team in the comments

@antoniofilipovic antoniofilipovic self-assigned this Jun 13, 2023
@gitbuda
Copy link
Member

gitbuda commented Jun 22, 2023

@antoniofilipovic can we merge this to master before v2.9 is released (include in the v2.9 release)?

@gitbuda gitbuda added this to the mg-v2.10.0 milestone Jun 22, 2023
@antoniofilipovic antoniofilipovic marked this pull request as ready for review July 24, 2023 11:42
@Josipmrden Josipmrden changed the title [master < T] Reduce all shortest paths memory usage [master < T981] Reduce all shortest paths memory usage Jul 28, 2023
@Josipmrden
Copy link
Contributor

please provide text for the PR message

src/query/cypher_query_interpreter.cpp Outdated Show resolved Hide resolved
src/query/interpret/eval.hpp Show resolved Hide resolved
src/query/interpreter.cpp Outdated Show resolved Hide resolved
src/query/interpreter.cpp Outdated Show resolved Hide resolved
src/query/interpreter.cpp Outdated Show resolved Hide resolved
@Josipmrden
Copy link
Contributor

small comments, overall GJ

src/query/interpret/eval.hpp Show resolved Hide resolved
@antoniofilipovic
Copy link
Collaborator Author

@vpavicic This PR changes the way memory is used in all shortest-path queries. From now on users should see lower memory usage with query performance almost staying the same.

@gitbuda gitbuda added benchmarking mgBench and benchmarking Memgraph feature feature labels Jul 30, 2023
@gitbuda gitbuda merged commit 285b409 into master Jul 30, 2023
6 checks passed
@gitbuda gitbuda deleted the reduce-all-shortest-memory-usage branch July 30, 2023 10:58
as51340 pushed a commit that referenced this pull request Nov 10, 2023
* Change allocator to PoolResource
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarking mgBench and benchmarking Memgraph feature feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants