Skip to content

Extract entries by reference from LTX. - #5411

Open
dmkozh wants to merge 1 commit into
stellar:masterfrom
dmkozh:get_all_entries_refs
Open

Extract entries by reference from LTX.#5411
dmkozh wants to merge 1 commit into
stellar:masterfrom
dmkozh:get_all_entries_refs

Conversation

@dmkozh

@dmkozh dmkozh commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Description

Currently getAllEntries call seals the LTX and unnecessarily copies every entry, which adds up to a few ms if thousands of entries are involved.

With this change we just return the references to all the entries. Since this call is used just once in the 'production' path, and the results are only used in a couple of read-only calls, it shouldn't be hard to maintain the lifetime correctness for this method.

This was inspired by an AI-generated change that parallelized the copy; there is actually no need to do the copy in the first place.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

@dmkozh
dmkozh force-pushed the get_all_entries_refs branch 4 times, most recently from 6ed9eb2 to 8f40d52 Compare August 14, 2026 20:11
@dmkozh
dmkozh marked this pull request as ready for review August 14, 2026 20:15
Copilot AI balanced review requested due to automatic review settings August 14, 2026 20:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Replaces copied ledger transaction entries with borrowed references to reduce ledger-close overhead.

Changes:

  • Adds non-owning ledger entry/key reference views.
  • Propagates reference views through bucket and Soroban state paths.
  • Updates tests and fuzzing callers.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/ledger/LedgerEntryRefs.h Defines reference vectors and views.
src/ledger/LedgerTxn.h Exposes borrowing API.
src/ledger/LedgerTxnImpl.h Declares implementation API.
src/ledger/LedgerTxn.cpp Extracts references while sealing.
src/ledger/LedgerManagerImpl.h Accepts reference views.
src/ledger/LedgerManagerImpl.cpp Uses borrowed entries during finalization.
src/ledger/InMemorySorobanState.h Updates state API types.
src/ledger/InMemorySorobanState.cpp Iterates referenced entries.
src/bucket/BucketManager.h Updates live-batch API.
src/bucket/BucketManager.cpp Forwards reference views.
src/bucket/BucketListBase.cpp Updates template instantiation.
src/bucket/LiveBucket.h Adds reference-based bucket APIs.
src/bucket/LiveBucket.cpp Converts referenced entries into buckets.
src/bucket/LiveBucketList.h Adds reference-view batch overload.
src/bucket/LiveBucketList.cpp Implements batch adapters.
src/bucket/test/BucketTestUtils.h Declares test adapters.
src/bucket/test/BucketTestUtils.cpp Converts test vectors to references.
src/invariant/test/BucketListIsConsistentWithDatabaseTests.cpp Uses borrowed transaction entries.
src/ledger/test/LedgerTxnTests.cpp Updates sealing tests.
src/test/fuzz/targets/TxFuzzTarget.cpp Adapts fuzz setup extraction.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/bucket/LiveBucketList.h
Comment thread src/ledger/LedgerTxn.cpp
Currently `getAllEntries` call seals the LTX and unnecessarily copies every entry, which adds up to a few ms if thousands of entries are involved.

With this change we just return the references to all the entries. Since this call is used just once in the 'production' path, and the results are only used in a couple of read-only calls, it shouldn't be hard to maintain the lifetime correctness for this method.
@dmkozh
dmkozh force-pushed the get_all_entries_refs branch from 8f40d52 to c246a5f Compare August 14, 2026 20:41
@dmkozh
dmkozh requested a review from graydon August 14, 2026 20:49
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