Skip to content

Switch to the new uplc_turbo to avoid memory leaks#669

Open
Eh2406 wants to merge 1 commit intomainfrom
eh2406/mem-leak
Open

Switch to the new uplc_turbo to avoid memory leaks#669
Eh2406 wants to merge 1 commit intomainfrom
eh2406/mem-leak

Conversation

@Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Feb 5, 2026

Bring in the fixes from pragma-org/uplc#36

Summary by CodeRabbit

  • Chores

    • Replaced the project's memory allocator dependency in the workspace manifest (removed prior allocator, added new allocator).
  • Refactor

    • Switched the pooled memory arena to a different arena backend; updated public API surface and initialization paths to match the new backend.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The PR removes the workspace bumpalo dependency from the root Cargo.toml, switches crates/amaru-plutus to use uplc-turbo.workspace = true, and refactors the arena pool to use uplc_turbo::arena::Arena (backed by bumpalo::Bump) instead of direct Bump usage.

Changes

Cohort / File(s) Summary
Crate manifests
Cargo.toml, crates/amaru-plutus/Cargo.toml
Removed bumpalo = { version = "3.16.0", features = ["collections"] } from workspace Cargo.toml; in crates/amaru-plutus/Cargo.toml replaced bumpalo.workspace = true with uplc-turbo.workspace = true.
Arena pool refactor
crates/amaru-plutus/src/arena_pool.rs
Replaced pool element type BumpArena (pub type BumpPool = Mutex<VecDeque<Arena>>); ArenaPool::new now constructs Arena::from_bump(Bump::with_capacity(...)); PooledArena stores ManuallyDrop<Arena>; impl AsRef and Deref updated to Arena; imports adjusted to use uplc_turbo::{arena::Arena, bumpalo::Bump}.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • KtorZ

Poem

A cheeky swap — bump tucked in an Arena cap,
Types shuffled like cards in a dungeon chap.
Imports aligned, the pool hums anew,
Merge it, mate — let those bytes debut. 🎮✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Switch to the new uplc_turbo to avoid memory leaks' directly and clearly describes the main change: migrating from bumpalo to uplc_turbo to fix memory leak issues.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 eh2406/mem-leak

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
crates/amaru-plutus/src/arena_pool.rs (1)

21-23: Grand stuff on the import swap — wee naming nit on the type alias though.

BumpPool now holds Arenas rather than Bumps, so the name's a touch misleading — like calling your espresso a flat white, y'know? Consider renaming to ArenaPool (or something that reflects the actual contents). That said, there's already a struct called ArenaPool, so maybe ArenaVec or just Pool would do the trick. Totally optional, no drama if you leave it.


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.

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

🤖 Fix all issues with AI agents
In `@crates/amaru-plutus/src/arena_pool.rs`:
- Around line 21-23: The workspace still declares bumpalo and the file imports
bumpalo::Bump even though BumpPool (the type alias) only uses
uplc_turbo::arena::Arena; remove the unused Bump import from
crates/amaru-plutus/src/arena_pool.rs (keep uplc_turbo::arena::Arena and the
BumpPool alias) and drop the bumpalo entry from the root Cargo.toml workspace
dependencies if no member crate directly depends on bumpalo (verify no other
crates need it first, otherwise leave it).

@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/amaru-plutus/src/arena_pool.rs 50.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
crates/amaru-plutus/src/arena_pool.rs 25.00% <50.00%> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Eh2406
Copy link
Contributor Author

Eh2406 commented Feb 6, 2026

I am not able to reproduce the CI failure locally. I don't know why it's not respecting the lock file.

With Fresh eyes on it I see there's another lockfile in the repo.

Signed-off-by: Jacob Finkelman <YeomanYaacov@gmail.com>
@Eh2406
Copy link
Contributor Author

Eh2406 commented Feb 9, 2026

CI failures seem also to be occurring on other recent PR's (https://github.com/pragma-org/amaru/actions/runs/21783366269/job/62850815322) and appear to be unrelated to my code changes.

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