Skip to content

Conversation

@intrigus-lgtm
Copy link
Contributor

calling acquire on a pool with no space left is UB and cannot be assumed to return NULL
Fixes all the results that #7056 finds.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Performance Measurements ⏳

Suite Baseline New Change
backtest mainnet-368528500-perf per slot 0.060885 s 0.050911 s -16.382%
backtest mainnet-368528500-perf snapshot load 1.744 s 1.414 s -18.922%
backtest mainnet-368528500-perf total elapsed 60.885075 s 50.911116 s -16.382%
firedancer mem usage with mainnet.toml 974.13 GiB 974.13 GiB 0.000%

Copy link
Contributor

@ripatel-fd ripatel-fd left a comment

Choose a reason for hiding this comment

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

Can we just introduce an acquire_safe API here instead of removing error checks?

@intrigus-lgtm
Copy link
Contributor Author

Can we just introduce an acquire_safe API here instead of removing error checks?

I don't fully understand what you are proposing to do.
This PR is basically just a continuation of #6739
All those error checks are redundant (as far as I understand the code) and will therefore NEVER trigger.

calling acquire on a pool with no space left is UB and cannot be assumed to return NULL
@intrigus-lgtm intrigus-lgtm force-pushed the intrigus/fix/more-fd_pool_acquire-ub branch from 8eed1bd to 736f56e Compare November 5, 2025 13:50
@github-actions
Copy link

github-actions bot commented Nov 5, 2025

Performance Measurements ⏳

Suite Baseline New Change
backtest mainnet-368528500-perf per slot 0.063766 s 0.063596 s -0.267%
backtest mainnet-368528500-perf snapshot load 2.162 s 2.142 s -0.925%
backtest mainnet-368528500-perf total elapsed 63.766393 s 63.595557 s -0.268%
firedancer mem usage with mainnet.toml 974.13 GiB 974.13 GiB 0.000%

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.

4 participants