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

Make the edge storage more columnar to reduce memory usage #1670

Merged
merged 12 commits into from
Jul 2, 2024

Conversation

fabianmurariu
Copy link
Contributor

@fabianmurariu fabianmurariu commented Jun 24, 2024

What changes were proposed in this pull request?

change the layout of the edges from rows to columns

Why are the changes needed?

reduce memory usage

Does this PR introduce any user-facing change? If yes is this documented?

No

How was this patch tested?

Unit test

Issues

closes #1667

Are there any further changes required?

@fabianmurariu fabianmurariu force-pushed the mem_optim2 branch 3 times, most recently from e3b30b5 to 4cec8cc Compare June 27, 2024 13:10
@fabianmurariu fabianmurariu changed the title Mem optim2 Make the edge storage more columnar to reduce memory usage Jun 28, 2024
@fabianmurariu fabianmurariu marked this pull request as ready for review June 28, 2024 14:18
Copy link
Collaborator

@ljeub-pometry ljeub-pometry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to point the submodule at the correct place!

Other than that, only some very minor comments and good to go in

pometry-storage-private Outdated Show resolved Hide resolved
raphtory/src/core/entities/graph/edges.rs Outdated Show resolved Hide resolved
raphtory/src/core/storage/raw_edges.rs Outdated Show resolved Hide resolved
}

impl<'a, O, OUT> GenLockedIter<'a, O, OUT> {
pub fn from<'b>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this lifetime doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed it is useless yet the self_referencing macro fails without that lifetime

raphtory/src/db/api/storage/edges/edges.rs Outdated Show resolved Hide resolved
raphtory/src/db/api/storage/edges/edges.rs Outdated Show resolved Hide resolved
raphtory/src/db/api/storage/nodes/unlocked.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@ljeub-pometry ljeub-pometry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean up the comments and you can push the merge button

Comment on lines 115 to 129

// #[inline]
// pub(crate) fn entry(&self, id: EID) -> EdgeRGuard {
// self.get_edge(id)
// }

// #[inline]
// pub(crate) fn entry_mut(&self, id: EID) -> EdgeWGuard {
// self.get_edge_mut(id)
// }

// #[inline]
// pub(crate) fn entry_arc(&self, id: EID) -> EdgeArcGuard {
// self.get_edge_arc(id)
// }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you left in some comments

Comment on lines 8 to 12

use super::edge_entry::EdgeStorageEntry;
use crate::db::api::storage::variants::storage_variants3::StorageVariants;

use rayon::iter::ParallelIterator;
use std::sync::Arc;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still some blank lines which means this didn't get fully simplified

@miratepuffin miratepuffin merged commit fd400bc into master Jul 2, 2024
19 checks passed
@miratepuffin miratepuffin deleted the mem_optim2 branch July 2, 2024 12:47
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.

Improve memory footprint for static graphs
3 participants