Skip to content

Comments

fix: prune program cache on each slot transition#736

Merged
bmuddha merged 1 commit intomasterfrom
bmuddha/fix/program-cache-prunning
Dec 7, 2025
Merged

fix: prune program cache on each slot transition#736
bmuddha merged 1 commit intomasterfrom
bmuddha/fix/program-cache-prunning

Conversation

@bmuddha
Copy link
Collaborator

@bmuddha bmuddha commented Dec 7, 2025

This PR introduces proper solution to the cache corruption issue, by pruning the program cache on each slot transition. This keeps the cache duplicate free on each slot, while avoiding full cache reset and ensuing program recompilation.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed slot relationship comparison logic to correctly identify ancestor, descendant, and equal relationships.
  • Performance

    • Optimized program cache handling with pruning to remove stale entries.
  • Chores

    • Updated dependencies to latest versions.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 7, 2025

📝 Walkthrough

Walkthrough

This pull request updates the solana-svm dependency revision in Cargo.toml and modifies the ForkGraph relationship implementation to return meaningful slot comparisons instead of always returning Unrelated. It also refactors the scheduler's transition_to_new_slot method to cache the program_cache write lock, prune the cache with the current slot, and use the cached reference for subsequent updates.

Possibly related PRs

Suggested reviewers

  • thlorenz
  • GabrielePicco
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bmuddha/fix/program-cache-prunning

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Dec 7, 2025

Manual Deploy Available

You can trigger a manual deploy of this PR branch to testnet:

Deploy to Testnet 🚀

Alternative: Comment /deploy on this PR to trigger deployment directly.

⚠️ Note: Manual deploy requires authorization. Only authorized users can trigger deployments.

Comment updated automatically when the PR is synchronized.

Copy link
Collaborator Author

bmuddha commented Dec 7, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@bmuddha bmuddha changed the title prune program cache on each slot transition fix: prune program cache on each slot transition Dec 7, 2025
@bmuddha bmuddha marked this pull request as ready for review December 7, 2025 08:52
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb87206 and 3a45fe6.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • Cargo.toml (2 hunks)
  • magicblock-processor/src/executor/mod.rs (1 hunks)
  • magicblock-processor/src/scheduler.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 596
File: magicblock-processor/src/scheduler.rs:1-1
Timestamp: 2025-10-28T13:15:42.706Z
Learning: In magicblock-processor, transaction indexes were always set to 0 even before the changes in PR #596. The proper transaction indexing within slots will be addressed during the planned ledger rewrite.
📚 Learning: 2025-10-21T11:00:18.396Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 578
File: magicblock-aperture/src/encoder.rs:176-187
Timestamp: 2025-10-21T11:00:18.396Z
Learning: In the magicblock validator, the current slot is always the root slot. The SlotEncoder in magicblock-aperture/src/encoder.rs correctly sets `root: slot` because there is no lag between current and root slots in this architecture.

Applied to files:

  • magicblock-processor/src/scheduler.rs
📚 Learning: 2025-11-04T10:53:50.922Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/locks.rs:110-122
Timestamp: 2025-11-04T10:53:50.922Z
Learning: In magicblock-processor, the TransactionScheduler runs in a single, dedicated thread and will always remain single-threaded. The `next_transaction_id()` function in scheduler/locks.rs uses `unsafe static mut` which is safe given this architectural guarantee.

Applied to files:

  • magicblock-processor/src/scheduler.rs
📚 Learning: 2025-11-07T13:20:13.793Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/coordinator.rs:227-238
Timestamp: 2025-11-07T13:20:13.793Z
Learning: In magicblock-processor's ExecutionCoordinator (scheduler/coordinator.rs), the `account_contention` HashMap intentionally does not call `shrink_to_fit()`. Maintaining slack capacity is beneficial for performance by avoiding frequent reallocations during high transaction throughput. As long as empty entries are removed from the map (which `clear_account_contention` does), the capacity overhead is acceptable.

Applied to files:

  • magicblock-processor/src/scheduler.rs
📚 Learning: 2025-11-13T09:38:43.804Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/locks.rs:64-102
Timestamp: 2025-11-13T09:38:43.804Z
Learning: In magicblock-processor's TransactionScheduler (scheduler/mod.rs line 59), the executor count is clamped to MAX_SVM_EXECUTORS (63) at initialization time, and executor IDs are assigned sequentially from 0 to count-1. This architectural guarantee ensures that executor IDs used in the bitmask-based AccountLock (scheduler/locks.rs) will always be within valid bounds for bit shifting operations, making runtime bounds checks unnecessary.

Applied to files:

  • magicblock-processor/src/scheduler.rs
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.

Applied to files:

  • Cargo.toml
📚 Learning: 2025-10-26T16:54:39.084Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.

Applied to files:

  • Cargo.toml
📚 Learning: 2025-11-24T08:45:11.164Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 675
File: Cargo.toml:145-145
Timestamp: 2025-11-24T08:45:11.164Z
Learning: The reqwest dependency version 0.11 is intentionally used in the magicblock-validator project to maintain compatibility with the Solana client dependencies.

Applied to files:

  • Cargo.toml
🔇 Additional comments (2)
magicblock-processor/src/executor/mod.rs (1)

237-254: Correct ForkGraph implementation for a forkless chain.

The updated relationship method now returns semantically meaningful values instead of always returning Unrelated. This is essential for the cache pruning in scheduler.rs to work correctly - the ProgramCache::prune method uses these relationships to determine which entries to evict.

The logic is correct for a linear, forkless chain:

  • a < b → slot a is an ancestor of slot b
  • a > b → slot a is a descendant of slot b
  • a == b → same slot
magicblock-processor/src/scheduler.rs (1)

143-152: Cache pruning logic is correct.

The implementation properly:

  1. Loads the current slot before acquiring the lock
  2. Prunes the cache to remove stale entries
  3. Updates latest_root_slot after pruning

The operation order is important—pruning before updating the root slot ensures the cache state remains consistent. The comments adequately explain the rationale for this cleanup step.

Copy link
Collaborator

@GabrielePicco GabrielePicco left a comment

Choose a reason for hiding this comment

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

LGTM!

@bmuddha bmuddha merged commit 8d59ef0 into master Dec 7, 2025
26 of 27 checks passed
@bmuddha bmuddha deleted the bmuddha/fix/program-cache-prunning branch December 7, 2025 10:08
thlorenz added a commit that referenced this pull request Dec 9, 2025
* master:
  chore: remove solana-sdk from workspace (#733)
  Feat: patched rocksdb to 0.23.0 (#692)
  fix: prune program cache on each slot transition (#736)
  Hotfix/disable frequency commits (#735)
  feat: remove noop dependency (#721)
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