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

FC-1431: fix/unresolve nodes #195

Merged
merged 3 commits into from
Sep 2, 2022
Merged

FC-1431: fix/unresolve nodes #195

merged 3 commits into from
Sep 2, 2022

Conversation

zonotope
Copy link
Contributor

@zonotope zonotope commented Sep 1, 2022

This patch adds an index api function to remove the cached :flakes/:children attributes from resolved nodes to save memory when we're finished processing these nodes.

@zonotope zonotope requested a review from a team September 1, 2022 14:43
@@ -145,7 +145,8 @@

(defn child-entry
[{:keys [first] :as node}]
[first node])
(let [child-node (unresolve node)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to try to summarize my understanding of this to make sure I've got it correct:

child-entry takes a node and returns a tuple of the the first flake it addresses and the node itself. This change unresolves the node itself so it won't carry around the :children or :flakes references.

Is it to reduce the size of the node we need to pass around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This code is mainly used when the database is re-indexed. In that case, the entire index tree is loaded into memory, node by node, and rebalanced. Without this change, the result of the indexing process would hold the all the data from an index in memory, thus eliminating the gains from processing the index node by node instead of all at once. This change instead keeps a pointer to the top level child nodes on disk, and those child nodes keep pointers to their children, and so on. The whole benefit of this change is memory efficiency for the indexing process.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Thanks for the explanation, that makes perfect sense.

Copy link
Contributor

@dpetran dpetran left a comment

Choose a reason for hiding this comment

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

🪶

@zonotope zonotope merged commit 91a9f29 into main Sep 2, 2022
@zonotope zonotope deleted the fix/unresolve-nodes branch September 2, 2022 16:33
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