Skip to content

build: compile docker image for x86-64-v3 target#311

Merged
MegaRedHand merged 3 commits intomainfrom
build/x86-64-v3-docker
Apr 21, 2026
Merged

build: compile docker image for x86-64-v3 target#311
MegaRedHand merged 3 commits intomainfrom
build/x86-64-v3-docker

Conversation

@MegaRedHand
Copy link
Copy Markdown
Collaborator

@MegaRedHand MegaRedHand commented Apr 21, 2026

Summary

Switch the release Docker image to the x86-64-v3 microarchitecture level so the binary uses AVX2, BMI1/2, LZCNT, and PCLMULQDQ. Any host from Haswell (2013) / Zen+ (2018) onward supports this.

On a Skylake-SP server, over a 400-slot devnet:

Build Aggregation mean vs upstream unstable
unstable (default x86-64) 2310 ms 1.00x
-Ctarget-cpu=native 859 ms 2.69x
-Ctarget-cpu=x86-64-v3 (this PR) 745 ms 3.10x

v3 beats native by ~15% on this CPU because aggressive AVX-512 use on Skylake-SP triggers core downclocking — staying on AVX2 keeps the clock higher. v3 is also portable (native binaries won't run on most non-SP Intel client CPUs or older AMD parts).

The config was copied from the ethrex repo: https://github.com/lambdaclass/ethrex/blob/1a2ec50ae6f99dd91046440f7e54e39cfcc01687/.cargo/config.toml

Two Docker plumbing fixes included

Without these, the .cargo/config.toml target.rustflags setting is silently ignored:

  1. COPY .cargo/ .cargo/ before cargo chef cook — otherwise dependency builds emit with default flags, and the final cargo build reuses the cached deps rather than rebuilding with the requested CPU features. Verified: without this fix, the resulting binary was byte-identical to the default-flag build (zero AVX2/BMI2 usage).
  2. Drop the ARG RUSTFLAGS="" / ENV RUSTFLAGS="$RUSTFLAGS" pair. An empty RUSTFLAGS env var overrides target.<triple>.rustflags from .cargo/config.toml (cargo precedence rule: env wins over config). With the old Dockerfile, any build that didn't pass --build-arg RUSTFLAGS=... would silently drop target-cpu/target-feature selections.

Instruction count diff (objdump -d | grep ymm/zmm/bzhi/lzcnt):

default v3
ymm (AVX2) 5,900 371,862
zmm (AVX-512) 0 0
bzhi (BMI2) 21 911
lzcnt 34 737
pclmulqdq n/a 354

Compile the release binary with target-cpu=x86-64-v3 plus pclmulqdq so it
uses AVX2, BMI1/2, LZCNT, and carryless-multiply on any host from Haswell
(2013) / Zen+ (2018) onward. Measured on a Skylake-SP server (the class
still common in fleet hosts) over a 400-slot devnet, this gets ~3.1x the
aggregation throughput of the default x86-64 build and ~1.15x the throughput
of -Ctarget-cpu=native -- the latter because aggressive AVX-512 use triggers
core downclocking on this generation, so staying on AVX2 keeps the clock
higher.

Two Docker plumbing fixes are needed to make the config actually take effect:

  * COPY .cargo/ before the cargo-chef cook step so dependency builds see
    target.rustflags. Otherwise chef cook emits deps with default flags and
    the final cargo build reuses them.

  * Drop the RUSTFLAGS ARG/ENV. An empty RUSTFLAGS env var overrides
    target.<triple>.rustflags from .cargo/config.toml (cargo precedence),
    so leaving ENV RUSTFLAGS="" in place silently nullifies the config.
@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

Overall Assessment: The PR correctly fixes a Docker build issue where cargo-chef cached dependencies would ignore CPU target flags. However, the aggressive x86-64-v3 baseline poses hardware compatibility risks for a consensus client.

Critical: Hardware Compatibility (.cargo/config.toml:3)
Targeting x86-64-v3 (AVX2, BMI1/2, LZCNT) excludes pre-2013 Intel (Sandy/Ivy Bridge) and pre-2015 AMD (Piledriver/Steamroller) processors. For Ethereum consensus clients, this is unusually aggressive—many validator operators run on older server hardware. This could cause SIGILL (illegal instruction) crashes on startup.

  1. Recommendation: Document this requirement prominently in README/Docker docs, or consider downgrading to x86-64-v2 for broader compatibility unless you've verified all target deployment hardware supports v3.

Minor: Redundant Target Features (.cargo/config.toml:4)
The x86-64-v3 target already implies avx2, sse2, ssse3, sse4.1, sse4.2, bmi1, bmi2, lzcnt, and fma. Your explicit list duplicates most of these. While harmless, consider:

  • Adding +bmi2 (missing from your list but available in v3) for shlx/shrx optimizations
  • Or simplify to just -Ctarget-cpu=x86-64-v3 -Ctarget-feature=+pclmulqdq since that's the only non-v3 feature listed

Correct: Cargo Chef Fix (Dockerfile:18-21, 33-36)
The COPY of .cargo/ before cargo chef cook is the correct fix. Without this, dependency crates compile with generic flags while the final crate uses v3 flags, risking ABI mismatches or missed optimizations. The removal of ARG RUSTFLAGS is also correct—environment variables override [target.*.rustflags] in Cargo, which would silently drop your optimizations.

Security/Build: Docker Context (.dockerignore:11)
Including !.cargo/ copies the entire directory. Ensure .cargo/credentials.toml (if it exists) is gitignored so Docker build contexts don't accidentally include registry authentication tokens. This is currently fine if you only have config.toml, but fragile.

Note: Platform Specificity
The [target.x86_64-unknown-linux-gnu] section only applies to Linux x86_64. macOS, Windows, and ARM64 Linux builds will use default generic targets. This is likely intentional for Linux container deployments, but ensure this asymmetry is expected.

Summary: Code changes are technically correct and well-commented, but Item 1 (compatibility) should be addressed before merge or clearly documented.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. High: .cargo/config.toml:1 hard-codes -Ctarget-cpu=x86-64-v3 plus AVX2-era features for every x86_64-unknown-linux-gnu build. That is a runtime-compatibility regression, not just a build-speed tweak: the published amd64 image will now SIGILL on older x86_64 hosts that do not support AVX2 / x86-64-v3. For a consensus client, “node does not start on part of the supported fleet” is a serious availability issue. If this optimization is desired, it should be opt-in or published under a separate “v3/avx2” tag instead of becoming the default artifact.

  2. Medium: Dockerfile:31 removes the RUSTFLAGS build-arg path entirely and makes the checked-in .cargo/config.toml the only source of compiler flags. That also removes the current escape hatch for local/docker builds that need extra flags (profiling, sanitizers, debugging, alternate CPU targets), while RELEASE.md:71 still documents RUSTFLAGS as supported. At minimum this is a breaking build-interface change that should be documented in the same PR; ideally keep a way to append user-supplied flags instead of replacing the interface.

No consensus-layer logic is touched here, so I did not find fork-choice / STF / attestation-specific issues in this PR. The main concern is release portability and operational safety of the produced binaries.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Here is the review:


PR Review: build: compile docker image for x86-64-v3 target

Summary

A focused, well-motivated build optimization with compelling benchmark numbers. The Docker plumbing fixes are the most technically interesting part — the cargo-chef cache-invalidation problem being solved here is subtle and the fix is correct.


Correctness

.cargo/config.toml applied correctly in the layer order. Placing COPY .cargo/ .cargo/ before cargo chef cook is exactly right: the cached deps need to be built with the target flags, otherwise cargo will reuse stale objects compiled without AVX2/BMI2 for the final binary. The comment in the Dockerfile explains this well.

RUSTFLAGS env-var removal is necessary, not just cleanup. Cargo's precedence rules mean that any RUSTFLAGS env var — including an empty string — fully overrides [target.*.rustflags] from config.toml. The old ARG RUSTFLAGS="" / ENV RUSTFLAGS="$RUSTFLAGS" pattern was silently defeating any config.toml-based flags. Removing it is the right call.


Minor Points

Redundant explicit feature flags. The -Ctarget-cpu=x86-64-v3 flag already implies +avx2,+sse2,+ssse3,+sse4.1,+sse4.2,+bmi1,+bmi2,+lzcnt. The explicit -Ctarget-feature=... list is additive and harmless, but the overlap with what x86-64-v3 already implies is noise. Only +pclmulqdq adds something beyond the x86-64-v3 baseline. Consider trimming to just "-Ctarget-feature=+pclmulqdq" for clarity, or keeping the full list with a comment explaining that they're explicit for documentation purposes.

+bmi2 is missing from the explicit list. The PR description highlights bzhi (a BMI2 instruction) as one of the improved instruction counts, and BMI2 is part of x86-64-v3. The explicit target-feature list includes +bmi1 but omits +bmi2. This is harmless since target-cpu=x86-64-v3 covers it, but the inconsistency is worth noting if the explicit list is meant to be self-documenting.

Local dev impact is undocumented. .cargo/config.toml applies to all local x86_64-Linux builds, not just Docker. Developers on pre-Haswell hardware (pre-2013) will get compile errors or silently broken binaries. A one-line comment in the config file noting the minimum hardware requirement would help (# Requires x86-64-v3 (Haswell 2013+ / Zen+ 2018+)).

The RUSTFLAGS build-arg escape hatch is gone. Any CI job or Makefile target that was passing --build-arg RUSTFLAGS=... would now silently stop working. Worth verifying no external tooling (e.g., lean-quickstart) relied on that ARG before merging. The Makefile's docker-build target doesn't pass RUSTFLAGS, so it looks clean locally.


Assessment

The core changes are correct and the reasoning is sound. The performance data (~3.1x over baseline) is strong justification. The two nits worth considering before merge: trimming the redundant feature flags and adding a hardware-requirement comment to the .cargo/config.toml.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 21, 2026

Greptile Summary

This PR compiles the release Docker image for the x86-64-v3 microarchitecture level, enabling AVX2/BMI1/BMI2/LZCNT/PCLMULQDQ instruction usage and delivering a reported ~3× aggregation speedup on Skylake-SP hardware. It also fixes two Docker plumbing bugs: copying .cargo/ before cargo chef cook (so dep caches are built with the correct flags) and removing the RUSTFLAGS env-var pair that would silently override config.toml settings.

Confidence Score: 5/5

Safe to merge; changes are well-reasoned and the two P2 findings are informational only.

All findings are P2: one notes that +pclmulqdq is technically outside the x86-64-v3 spec (but safe on all realistic hardware), and the other notes that removing the RUSTFLAGS build-arg removes an external customisation hook. Neither blocks correctness or production safety.

No files require special attention.

Important Files Changed

Filename Overview
.cargo/config.toml New file setting x86-64-v3 target-cpu and explicit feature flags for x86_64-unknown-linux-gnu; +pclmulqdq is redundant with the v3 level but safe in practice.
.dockerignore Adds !.cargo/ whitelist rule so the new .cargo/config.toml is included in the Docker build context.
Dockerfile Copies .cargo/ before cargo chef cook so rustflags apply to dependency builds; removes ARG/ENV RUSTFLAGS pair that would have silently overridden config.toml settings — though this removes external flag customisation via build-args.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Docker build context] -->|".dockerignore: !.cargo/"| B[includes .cargo/config.toml]

    subgraph planner [planner stage]
        C["COPY --exclude=.git --exclude=target . .\n(includes .cargo/)"]
        D[cargo chef prepare → recipe.json]
        C --> D
    end

    subgraph builder [builder stage]
        E[COPY recipe.json]
        F["COPY .cargo/ .cargo/\n(rustflags: -Ctarget-cpu=x86-64-v3)"]
        G["cargo chef cook\n(deps compiled w/ AVX2/BMI2 flags)"]
        H["COPY --exclude=target . .\n(full source)"]
        I["cargo build --locked\n(reuses v3-flagged dep cache)"]
        J[cp ethlambda → /app/ethlambda]
        E --> F --> G --> H --> I --> J
    end

    subgraph runtime [runtime stage / ubuntu]
        K[COPY ethlambda → /usr/local/bin]
        L[ENTRYPOINT ethlambda]
        K --> L
    end

    B --> planner
    planner --> builder
    builder --> runtime
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .cargo/config.toml
Line: 4

Comment:
**`pclmulqdq` not guaranteed by x86-64-v3**

`+pclmulqdq` is not part of the official x86-64-v3 microarchitecture level (which defines: avx, avx2, bmi1, bmi2, f16c, fma, lzcnt, movbe, popcnt, sse4.2, xsave). Adding it via `-Ctarget-feature` raises the effective minimum CPU requirement slightly beyond what the v3 label advertises. In practice this is safe — pclmulqdq is present on all Intel Westmere (2010+) and AMD Bulldozer (2011+) chips, so every v3-capable CPU has it — but it's worth knowing the binary is technically targeting a stricter subset than "x86-64-v3".

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: Dockerfile
Line: 23-29

Comment:
**`RUSTFLAGS` build-arg removed — external override no longer possible**

Dropping `ARG RUSTFLAGS` removes the ability to pass different compiler flags at image-build time without editing `.cargo/config.toml`. Previously a caller could do `docker build --build-arg RUSTFLAGS="-Ctarget-cpu=x86-64-v2"` to produce a more portable image for a specific deployment. If any CI job or `make` target relied on `--build-arg RUSTFLAGS=...`, it will now silently be ignored (the arg just doesn't exist). Consider documenting that `.cargo/config.toml` is the intended override point going forward.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "build: compile docker image for x86-64-v..." | Re-trigger Greptile

Comment thread .cargo/config.toml
Comment thread Dockerfile
Comment thread .dockerignore Outdated
Comment thread Dockerfile Outdated
Comment thread Dockerfile
Comment on lines -22 to -25
# Extra Cargo flags
ARG RUSTFLAGS=""
ENV RUSTFLAGS="$RUSTFLAGS"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was removed since setting an empty RUSTFLAGS would discard the cargo config

@MegaRedHand MegaRedHand merged commit 77aa0c8 into main Apr 21, 2026
2 checks passed
@MegaRedHand MegaRedHand deleted the build/x86-64-v3-docker branch April 21, 2026 18:06
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