Skip to content

Conversation

@Elvis339
Copy link
Contributor

@Elvis339 Elvis339 commented Dec 3, 2025

Why this should be merged

Enables Firewood to track performance over time by running C-Chain reexecution benchmarks with custom Firewood builds. This establishes the infrastructure for catching performance regressions before they reach production.

ava-labs/firewood#1494

How this works

  1. Firewood triggers the existing C-Chain reexecution workflow via GitHub API with with-dependencies parameter
  2. Workflow sets up dependencies via task setup-reexecution-deps before calling the benchmark action
  3. The task uses polyrepo to sync Firewood at the specified ref
  4. Runs C-Chain reexecution benchmark with the custom build
  5. Uploads results as artifact for Firewood to download and track separately

The root action remains unaware of external dependency configuration - setup is handled by the workflow before invocation.

The same functionality is available locally for development:

nix develop
FIREWOOD_REF=abc123 ./scripts/run_task.sh setup-reexecution-deps
./scripts/run_task.sh test-cchain-reexecution -- firewood-101-250k

Changes

  • Add setup-reexecution-deps task for dependency setup (externalized from action)
  • Update workflows to call setup task before invoking benchmark action
  • Root action no longer accepts dependency refs (workflow responsibility)
  • Always upload benchmark artifacts (enables downstream tracking)
  • Update documentation in C-Chain Re-Execution README

How this was tested

gh workflow run "C-Chain Re-Execution Benchmark w/ Container"
--ref es/enable-firewood-dev-workflow
-f test=firewood-101-250k
-f with-dependencies="firewood=v0.0.15,libevm=v1.13.15-0.20251210210615-b8e76562a300"
-f runner=avalanche-avalanchego-runner-2ti
-f timeout-minutes=60

Need to be documented in RELEASES.md?

No

@Elvis339 Elvis339 self-assigned this Dec 3, 2025
Copilot AI review requested due to automatic review settings December 3, 2025 17:43
@Elvis339 Elvis339 requested review from a team and aaronbuchwald as code owners December 3, 2025 17:43
@Elvis339 Elvis339 added the ci This focuses on changes to the CI process label Dec 3, 2025
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

This PR establishes infrastructure for tracking Firewood performance over time by enabling C-Chain reexecution benchmarks with custom Firewood builds. The workflow can be triggered from the Firewood repository with either published versions (for quick testing) or branch/commit references (for comprehensive testing with source builds).

Key changes:

  • Adds a reusable workflow that accepts Firewood version/branch/commit as input
  • Implements intelligent build strategy: uses go get for published versions, builds from source for branches/commits
  • Creates build script for compiling Firewood FFI from source using Nix

Reviewed changes

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

File Description
.github/workflows/c-chain-reexecution-benchmark-firewood.yml New workflow that orchestrates benchmark execution with custom Firewood builds and uploads results as artifacts
graft/coreth/scripts/build_firewood.sh Shell script to clone, build, and optionally integrate Firewood FFI from source
.github/workflows/c-chain-reexecution-benchmark-container.yml Removes container configuration (unrelated cleanup)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Elvis339 Elvis339 requested a review from maru-ava December 9, 2025 18:10
Elvis339 and others added 4 commits December 9, 2025 22:10
…ution benchmarks

Integrate firewood/libevm dependency overrides into existing workflows
using polyrepo, eliminating the need for a separate firewood workflow.

- Add LIBEVM_REF/FIREWOOD_REF to reexecute-cchain-range-with-copied-data
- Update composite action and workflows to pass inputs
- Remove redundant firewood workflow and build_firewood.sh script
…/avalanchego into es/enable-firewood-dev-workflow
Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

I do not think that the re-execution benchmark action itself should be responsible for coupling firewood/libevm refs when any workflow invoking this can simply update the dependencies before invoking this action as a step. Separately, the action should not support a special case of archiving data just for Firewood.

These are simple pre- and post- execution steps, which can be easily added as a step before or after the action gets invoked within a workflow.

If Firewood wants to execute this separately, why not just define its own workflow to do so? If it wants to execute it within AvalancheGo, then we should just write the data to the default location using Firewood as the config.

@Elvis339
Copy link
Contributor Author

I do not think that the re-execution benchmark action itself should be responsible for coupling firewood/libevm refs when any workflow invoking this can simply update the dependencies before invoking this action as a step. Separately, the action should not support a special case of archiving data just for Firewood.

These are simple pre- and post- execution steps, which can be easily added as a step before or after the action gets invoked within a workflow.

If Firewood wants to execute this separately, why not just define its own workflow to do so? If it wants to execute it within AvalancheGo, then we should just write the data to the default location using Firewood as the config.

Resolved offline. Root action now:

  • Has no knowledge of firewood/libevm refs (workflow handles setup via task)
  • Pushes to GitHub Action Benchmark (controlled by input)
  • Always uploads artifacts

Dependency setup externalized to task setup-reexecution-deps, called by workflow before action.

- Add LIBEVM_REF and FIREWOOD_REF env var handling to run_polyrepo.sh
- Remove setup-reexecution-deps task from Taskfile.yml
- Update CI workflows to call run_polyrepo.sh directly
- Delete setup_reexecution_deps.sh (no longer needed)
…g permission issue by redirecting Nix's temp directory to /tmp which has proper permissions on ARC
- Remove FIREWOOD_REF env var handling from run_polyrepo.sh
- LIBEVM_REF remains as env var (needs go get)
- Firewood passed as polyrepo arg: sync firewood@<ref>
- Clean up workflow steps with inline conditional
- Update README with new usage patterns
The github-action-benchmark step was running even when
push-github-action-benchmark was false, causing failures when
custom deps modify go.mod/go.sum (can't checkout gh-pages with
uncommitted changes).

Now the step only runs when comparison is actually needed.
- Comparison step conditional on push-github-action-benchmark
- Default true preserves existing behavior
- Prevents gh-pages checkout failure when go.mod/go.sum are dirty (i.e., polyrepo firewood setup)
When custom dependencies (firewood-ref or libevm-ref) are used,
go.mod/go.sum are modified. This causes github-action-benchmark
to fail when checking out gh-pages.

New input skips comparison step only when custom deps are present,
preserving summary display for normal PR runs.
push-github-action-benchmark:
description: 'Whether to push the benchmark result to GitHub.'
required: true
skip-benchmark-comparison:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When custom dependencies (firewood-ref or libevm-ref) are used,
go.mod/go.sum are modified. This causes github-action-benchmark
to fail when checking out gh-pages.

New input skips comparison step only when custom deps are present,
preserving summary display for normal PR runs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to make this skip automatic with an appropriate warning if the tree is dirty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The inputs already tell us when the tree will be dirty if firewood-ref or libevm-ref is provided, polyrepo/go get will modify go.mod. A runtime git status check would just confirm what we already know statically.

Putting the detection logic inside the action would also conflict with Aaron's constraint he was okay with the action accepting skip-benchmark-comparison as an input, but the decision of when to skip should be made by the caller, not the action itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

@aaronbuchwald Why is it desirable to have to pass in a synthesized variable instead of just detecting this condition locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding context on the design. Polyrepo is a composable layer that sits before the benchmark action, setting up deps. The base action runs benchmarks on whatever code is present, then uploads artifacts (Firewood will track performance on their own gh-pages).

Aaron's constraint as I understood it is that the action should run reexecution for the environment that's present not be responsible for in-place modification or inferring dependency state.

This matters because if the action detects dirty tree and decides to skip, it's making decisions about dependency state without knowing why the tree is dirty. The caller (workflow) knows it ran polyrepo → knows the tree is dirty → passes skip-benchmark-comparison: true.

This keeps concerns separated dep setup in workflow, benchmarking in action.

I might be wrong in how I inferred Aaron's constraint happy for him to correct me. Open to doing whatever to progress with this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The original constraint is that the custom action should not need to know about the special case of firewood or overwriting dependencies. This seems a better separation of responsibilities to me than to pass in firewood-ref, libevm-ref, etc. which imo is better handled as a separate step.

I am not sure why skip-benchmark-comparison needs to be added as opposed to simply setting push-github-action-benchmark to false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that github-action-benchmark fails entirely when the tree is dirty - it tries to checkout gh-pages and errors on the dirty working tree. Setting push-github-action-benchmark: false still runs the step, which fails. We need to skip the step entirely.

push-github-action-benchmark:
description: 'Whether to push the benchmark result to GitHub.'
required: true
skip-benchmark-comparison:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to make this skip automatic with an appropriate warning if the tree is dirty?

# ./scripts/run_polyrepo.sh [polyrepo args...]
#
# Environment variables (optional):
# LIBEVM_REF - Git ref for libevm (runs: go get && go mod tidy)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use env vars for both args so that the caller has a consistent way of specifying both values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my initial implementation :D

Re-added: a29c22c

Add comments explaining that Nix is installed before calling polyrepo
and that the c-chain-reexecution-benchmark action's Nix install will
be skipped since installation is idempotent.
prometheus-password: ${{ secrets.PROMETHEUS_PASSWORD || '' }}
push-github-action-benchmark: ${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && github.repository == 'ava-labs/avalanchego' && github.ref_name == 'master') }}
push-github-action-benchmark: ${{ (github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && github.repository == 'ava-labs/avalanchego' && github.ref_name == 'master')) && matrix.firewood-ref == '' }}
skip-benchmark-comparison: ${{ matrix.firewood-ref != '' || matrix.libevm-ref != '' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these values the trigger instead of the dirty tree state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two separate concerns:

  • push-github-action-benchmark - Controls whether results get pushed to gh-pages for historical tracking. Only enabled for scheduled runs on master with default deps.
  • skip-benchmark-comparison - Controls whether the comparison step runs at all. When custom deps are used, go.mod/go.sum are modified, causing the benchmark action to fail on gh-pages checkout (dirty tree error).

Why input values instead of dirty tree check?
We know statically that if firewood-ref or libevm-ref is provided, the tree will be dirty. Runtime check would confirm what inputs already tell us.

I'm okay with adding automatic dirty tree detection inside c-chain-reexecution-benchmark action if Aaron agrees. From my conversation with him, I inferred the action should stay focused on running benchmarks and not infer/handle dependency state the caller knows it modified deps, so the caller decides whether to skip comparison.

…enchmarks

- Use LIBEVM_REF and FIREWOOD_REF env vars for consistent interface
- Rename task from polyrepo to run-polyrepo
- Update README with new usage patterns
@Elvis339
Copy link
Contributor Author

Elvis339 commented Jan 23, 2026

Tested with custom firewood dependency:
https://github.com/ava-labs/avalanchego/actions/runs/21286107306/job/61267590660

gh workflow run "C-Chain Re-Execution Benchmark w/ Container" \
  --repo ava-labs/avalanchego \
  --ref es/enable-firewood-dev-workflow \
  -f runner="avalanche-avalanchego-runner-2ti" \
  -f config="firewood" \
  -f with-dependencies="firewood=v0.0.18" \
  -f start-block="1" \
  -f end-block="10000" \
  -f timeout-minutes="1440"

Expected:

  • Dependency setup
  • Benchmark summary skipped

Tested without custom deps and Firewood:
https://github.com/ava-labs/avalanchego/actions/runs/21286367028/job/61268454476

gh workflow run "C-Chain Re-Execution Benchmark w/ Container" \
  --repo ava-labs/avalanchego \
  --ref es/enable-firewood-dev-workflow \
  -f runner="avalanche-avalanchego-runner-2ti" \
  -f test="hashdb-101-250k" \
  -f timeout-minutes="60"

Expected:

  • Nix/dependency configuration skipped
  • Benchmark summary displayed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci This focuses on changes to the CI process

Projects

Status: In Progress 🏗️

Development

Successfully merging this pull request may close these issues.

5 participants