Repository navigation
perf(storage): O(1) storage_tail append via newest-first batch list - #38
Merged
Merged
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
conn.storage_tail now keeps per-transaction datom batches newest-first and conses each report's tx_data, replacing the O(#batches) list append on every transaction. The forward (tx, then in-tx) order is restored with List.rev only where the flat tail is consumed: store_tail writes, the Conn.storage_tail accessor, and restore seeding — so the persisted Storage_tail payload is order-identical to before. Storage.tail_datom_count folds group lengths instead of concat |> length, and persist_transact_tail (the db-level transact path) computes the compaction count before materializing groups @ [tx_data], so a compaction-triggering tx skips the append entirely. Regression coverage in test_storage: on-disk Storage_tail group order across transacts, restore_conn, and compaction; Conn.storage_tail accessor order; tail_datom_count over grouped tails.
devin-ai-integration
Bot
force-pushed
the
devin/storage-tail-gc
branch
from
October 5, 2026 07:27
a1e7109 to
5c8f910
Compare
devin-ai-integration
Bot
changed the title
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
Conn.transact/apply_reportdidconn.storage_tail @ [report.tx_data]on every transaction — O(#batches) per tx.conn.storage_tailnow keeps per-transactiondatom listbatches newest-first and consesreport.tx_data;List.revrestores forward (tx, then in-tx) order only where the flat tail is consumed:store_tailwrites, theConn.storage_tailaccessor, andmake/restore_connseeding. The persistedStorage_tailpayload at address"1"is order-identical to before.Two count-side improvements ride along:
Storage.tail_datom_countfolds group lengths (fold_leftoverList.length) instead ofList.concat |> List.length, so the compaction check no longer allocates a flat copy of the tail.persist_transact_tail(the db-leveltransactpath) computestail_datom_count groups + length tx_databefore materializinggroups @ [tx_data], so a compaction-triggering tx skips the append entirely.No signature or on-disk format changes. The auto-GC work originally in this PR moved to #40.
Tests
All 34 native test suites pass. New regression tests in
test_storage.ml:test_conn_tail_write_order— storedStorage_tailgroups stay in tx order then in-tx order across multiple txs, arestore_conn(which exercises the reversed internal repr), a compaction, and the next tx;Conn.storage_tailreturns the same forward ordertest_tail_datom_count— grouped-tail count including an empty groupLink to Devin session: https://app.devin.ai/sessions/eb0e07b23c794857b6836d8a143d1688
Open in Devin Desktop: https://app.devin.ai/desktop/session/eb0e07b23c794857b6836d8a143d1688?variant=devin
Requested by: @tiensonqin