Repository navigation
feat: upsert index nodes at their address on re-store (cljs parity) - #3
Merged
Merged
Conversation
Match the cljs persistent-sorted-set storage semantics: a modified stored node keeps its previous address and is rewritten in place (IStorage.store with the node's _address), instead of allocating a fresh address and orphaning the superseded row. - storage.store_node takes ?address for the in-place rewrite - Leaf/Branch nodes carry a dirty flag; restored nodes start clean - 1->1 replacement inherits the node address; split keeps it on the first output; merge keeps the left operand's address; borrow keeps each slot's own address - store() on an unmodified stored set still writes nothing Verified against cljs persistent-sorted-set 0.1.2 (jar from org.clojars.tiensonqin) running the same add/remove/store scenarios: root address stays stable, merge keeps the left address and drops the right (we orphan the row instead of deleting it), split gives the first output the old address.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
store_nodegains an optional?addressparameter so a node being re-stored can keep its original address and upsert in place, matching cljs persistent-sorted-set'sstoresemantics. Previously every store allocated a fresh address and left the old row dead, which accumulated ~2x dead kvs rows vs cljs for the same workload.PSet.store_node : ?address:string -> 'a -> 'a PSet.node— when the address is provided the node is written at that address (upsert), otherwise a new address is allocated as before.dirtyflag so the caller can distinguish already-stored nodes from new/modified ones; only dirty nodes are written on store.With the corresponding datascript-ocaml change, a 12k-block graph writes 17.6k kvs rows / 19.6MB — within ~2% of the cljs daemon's 17.3k / 19.1MB (previously 34k+ rows / 57MB).
Link to Devin session: https://app.devin.ai/sessions/f641a5e9845f45108c4a98cb82e268d3
Open in Devin Desktop: https://app.devin.ai/desktop/session/f641a5e9845f45108c4a98cb82e268d3?variant=devin
Requested by: @tiensonqin