Skip to content

Conversation

@Elvis339
Copy link
Contributor

@Elvis339 Elvis339 commented Dec 25, 2025

Why this should be merged

Adds --pprof-dir <path> flag to C-chain re-execution benchmark for CPU and memory profiling.

Part of cross-stack profiling effort: ava-labs/firewood#1582

pprof shows time spent in CGO calls as black boxes we see that ffi.(*Revision).Get takes X% of CPU but not what happens inside Firewood it's still valuable because:

  • Shows Go-side hotspots (GC, allocations, serialization, AvalancheGo)
  • Identifies which Go code paths trigger FFI calls
  • Measures total time per FFI function

Combined with Rust-side profiling, we get full visibility across the boundary.

How this works

When --pprof-dir <path> is passed:

  1. Creates directory at <path> directory
  2. Starts CPU profiling
  3. Writes cpu.profile, mem.profile and lock.profile on exit

How this was tested

  1. nix develop (make sure you have AWS credentials set, c-chain-reexecute requires S3 access)
  2. Edit line 97 of vm_reexecution and set output dir then run ./scripts/run_task.sh c-chain-reexecution-firewood-101-250k
  3. go tool pprof /tmp/pprof/cpu.profile
    • top 20
    • list Revision.*Get

NOTE: --pprof-dir is intentionally not exposed in benchmark_cchain_range.sh yet the profiling API may evolve as other profiling needs arise ava-labs/firewood#1582. The flag is still available when running the test directly via go run.

ROUTINE ======================== github.com/ava-labs/firewood-go-ethhash/ffi.(*Revision).Get in /Users/elvis.sabanovic/go/pkg/mod/github.com/ava-labs/firewood-go-ethhash/[email protected]/revision.go
      20ms      6.27s (flat, cum)  1.06% of Total
         .          .     61:func (r *Revision) Get(key []byte) ([]byte, error) {
         .          .     62:   if r.handle == nil {
         .          .     63:           return nil, ErrDroppedRevision
         .          .     64:   }
         .          .     65:
         .       10ms     66:   var pinner runtime.Pinner
      10ms       10ms     67:   defer pinner.Unpin()
         .          .     68:
      10ms      110ms     69:   return getValueFromValueResult(C.fwd_get_from_revision(
         .          .     70:           r.handle,
         .          .     71:           newBorrowedBytes(key, &pinner),
         .      6.14s     72:   ))
         .          .     73:}
         .          .     74:
         .          .     75:// Iter creates an iterator starting from the provided key on revision.
         .          .     76:// pass empty slice to start from beginning
         .          .     77:// It returns ErrDroppedRevision if Drop has already been called.
ROUTINE ======================== github.com/ava-labs/firewood-go-ethhash/ffi.(*Revision).Get.func1 in /Users/elvis.sabanovic/go/pkg/mod/github.com/ava-labs/firewood-go-ethhash/[email protected]/revision.go
         0      6.14s (flat, cum)  1.03% of Total
         .          .     69:   return getValueFromValueResult(C.fwd_get_from_revision(
         .          .     70:           r.handle,
         .       30ms     71:           newBorrowedBytes(key, &pinner),
         .      6.11s     72:   ))
         .          .     73:}
         .          .     74:
         .          .     75:// Iter creates an iterator starting from the provided key on revision.
         .          .     76:// pass empty slice to start from beginning
         .          .     77:// It returns ErrDroppedRevision if Drop has already been called.

Need to be documented in RELEASES.md?

No

@Elvis339 Elvis339 requested a review from a team as a code owner December 25, 2025 10:02
Copilot AI review requested due to automatic review settings December 25, 2025 10:02
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 adds pprof profiling support to the C-chain re-execution benchmark to enable CPU and memory profiling during benchmark runs. This supports cross-stack profiling efforts by providing Go-side performance visibility that complements Rust-side profiling.

Key changes:

  • Added --pprof flag to enable profiling during benchmark execution
  • Implemented CPU and memory profile file generation in ./pprof/ directory
  • Added logging of pprof enablement status to benchmark output

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

…ld be replaced with PROFILE flag which will capture the whole profiling stack including pprof
@github-project-automation github-project-automation bot moved this to In Progress 🏗️ in avalanchego Dec 29, 2025
@Elvis339 Elvis339 requested a review from RodrigoVillar January 5, 2026 15:18
Copy link
Contributor

@RodrigoVillar RodrigoVillar left a comment

Choose a reason for hiding this comment

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

Currently, the changes this PR introduces are only invokable via go run, which is not the recommended way to running the reexecution test (with preference going to ./scripts/benchmark_cchain_range.sh or using task). Can this PR also include a way to invoke profiling both the preferred options above?

This may require coordination with #4761

@Elvis339
Copy link
Contributor Author

Elvis339 commented Jan 5, 2026

Currently, the changes this PR introduces are only invokable via go run, which is not the recommended way to running the reexecution test (with preference going to ./scripts/benchmark_cchain_range.sh or using task). Can this PR also include a way to invoke profiling both the preferred options above?

This may require coordination with #4761

See the note in the PR description the --pprof-dir is intentionally not exposed in the scripts yet.
This PR is a prerequisite for cross-stack profiling (a larger change that will consolidate how profiling is exposed across the tooling). Breaking it down for simpler, incremental review.
Currently I'm the only one experimenting with these benchmarks with debug flags and I'm compiling vm_reexecutable myself anyway so having it available via cmd arg is fine for now. Once the full profiling story lands, we can expose it properly in the scripts.

@RodrigoVillar
Copy link
Contributor

Currently, the changes this PR introduces are only invokable via go run, which is not the recommended way to running the reexecution test (with preference going to ./scripts/benchmark_cchain_range.sh or using task). Can this PR also include a way to invoke profiling both the preferred options above?
This may require coordination with #4761

See the note in the PR description the --pprof-dir is intentionally not exposed in the scripts yet. This PR is a prerequisite for cross-stack profiling (a larger change that will consolidate how profiling is exposed across the tooling). Breaking it down for simpler, incremental review. Currently I'm the only one experimenting with these benchmarks with debug flags and I'm compiling vm_reexecutable myself anyway so having it available via cmd arg is fine for now. Once the full profiling story lands, we can expose it properly in the scripts.

Ah I now see #4791

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

Labels

None yet

Projects

Status: In Progress 🏗️

Development

Successfully merging this pull request may close these issues.

3 participants