Skip to content

replay: eviction fix#8647

Open
yufeng-jump wants to merge 1 commit intomainfrom
yufeng/frontier-evict1
Open

replay: eviction fix#8647
yufeng-jump wants to merge 1 commit intomainfrom
yufeng/frontier-evict1

Conversation

@yufeng-jump
Copy link
Contributor

No description provided.

Copilot AI review requested due to automatic review settings March 3, 2026 02:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates bank eviction/pruning to also cancel replay-related fork state (txncache/accdb/progcache) when dead banks are pruned, addressing replay eviction consistency.

Changes:

  • Extends fd_banks_prune_dead_banks to accept txncache/accdb_admin/progcache_admin handles and perform fork cancellation during pruning.
  • Updates replay tile to pass the required handles and removes the prior FIXME.
  • Updates bank tests to match the new fd_banks_prune_dead_banks signature.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/flamenco/runtime/test_bank.c Updates unit tests to call the new prune API signature with NULL optional args.
src/flamenco/runtime/fd_bank.h Changes prune API signature and adds forward declarations for new parameter types.
src/flamenco/runtime/fd_bank.c Implements fork cancellation (txncache/accdb/progcache) when pruning dead replayable banks.
src/discof/replay/fd_replay_tile.c Passes replay tile state (txncache/accdb/progcache) into bank pruning.

Comment on lines 14 to +23

struct fd_txncache_private;
typedef struct fd_txncache_private fd_txncache_t;

struct fd_accdb_admin;
typedef struct fd_accdb_admin fd_accdb_admin_t;

struct fd_progcache_admin;
typedef struct fd_progcache_admin fd_progcache_admin_t;

Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The new forward typedefs for fd_txncache_t / fd_accdb_admin_t / fd_progcache_admin_t are likely to cause typedef redefinition errors in any TU that includes this header alongside the canonical headers that already typedef these names (e.g. fd_txncache.h, fd_accdb_base.h/fd_accdb_admin.h, fd_progcache_admin.h). For example, fd_bank.c now includes fd_txncache.h after fd_bank.h, which will redeclare fd_txncache_t. Prefer removing these typedefs from fd_bank.h and either (a) include the canonical header(s) that define the typedefs, or (b) use only struct ...; forward declarations and change the prune API to accept struct fd_* * parameters to avoid needing the typedefs here.

Suggested change
struct fd_txncache_private;
typedef struct fd_txncache_private fd_txncache_t;
struct fd_accdb_admin;
typedef struct fd_accdb_admin fd_accdb_admin_t;
struct fd_progcache_admin;
typedef struct fd_progcache_admin fd_progcache_admin_t;
#include "fd_txncache.h"
#include "fd_accdb_admin.h"
#include "fd_progcache_admin.h"

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Performance Measurements ⏳

Suite Baseline New Change
backtest mainnet-368528500-perf per slot 0.075066 s 0.075123 s 0.076%
backtest mainnet-368528500-perf snapshot load 4.237 s 3.098 s -26.882%
backtest mainnet-368528500-perf total elapsed 75.066127 s 75.123269 s 0.076%
firedancer mem usage with mainnet.toml 964.37 GiB 964.37 GiB 0.000%

@yufeng-jump yufeng-jump force-pushed the yufeng/frontier-evict1 branch from e7d29cc to 4770ae7 Compare March 3, 2026 04:36
@github-actions
Copy link

github-actions bot commented Mar 3, 2026

Performance Measurements ⏳

Suite Baseline New Change
backtest mainnet-368528500-perf per slot 0.061122 s 0.060813 s -0.506%
backtest mainnet-368528500-perf snapshot load 3.255 s 2.494 s -23.379%
backtest mainnet-368528500-perf total elapsed 61.12187 s 60.813455 s -0.505%
firedancer mem usage with mainnet.toml 963.38 GiB 963.38 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.

2 participants