feat: reuse compiled artifacts in rvr

[mansur20478]

Part 2 of the original ticket split into two. Part 1: link

Part 2 consist of reusing persisted artifacts whenever possible instead of recompiling. The idea is to compile in rvr-mode till getting .c files, and instead of invoking make, check whether already existing .so are fit for reuse. Artifact fingerprints must match for .so file to be reused. During compilation

Artifact fingerprint is a SHA-256 fingerprint of:

1. native_debug_info flag
2. toolchain.compiler, toolchain.linker, toolchain.make, toolchain.host_os
3. host_cpu_features: affects -march=native (unnecessary?)
4. make_args: make variables except EXT_LIBS and EXT_SRCS
5. name + content of every file under generated C project_dir in sorted order. The folder includes external static libs and external source files too.

New methods:
compile_pure_cached, compile_metered_cached, compile_metered_cost_cached that take cache_dir as an additional argument - a place on disk for storing artifacts. Invokable from sdk.

The following loading functions do not perform validation:
load_compiled_pure, load_compiled_metered, load_compiled_metered_cost methods don't have .so validation.

Modified build.rs of extensions to rebuild rvr FFI staticlibs on Cargo.lock and ffi-common source changes.

resolves INT-7843

[mansur20478]

@claude provide Code Review for this PR

[claude]

Claude finished @mansur20478's task in 3m 18s —— View job

---

Code Review: PR #2833 — Reuse compiled artifacts in RVR

• Gather context and read changed files
• Review artifact_cache.rs (new fingerprinting module)
• Review compile.rs changes (caching logic)
• Review vm.rs changes (API surface)
• Review SDK integration (lib.rs, compiled.rs, tests)
• Review build.rs changes and cross-process test
• Post review findings

---

Summary

This PR adds content-addressed caching for compiled RVR shared libraries (.so/.dylib). A SHA-256 fingerprint is computed from the generated C project files, toolchain identity, host CPU features, and make arguments. On cache hit the .so is loaded directly; on miss it's compiled and stored. The EXT_LIBS absolute-path issue from @gdmlcjs's review has been addressed. The race condition fix (write-to-temp-then-rename in save_artifact) is solid. The cross-process test with separate binaries validates fingerprint stability across processes.

The PR is already approved by @gdmlcjs. I have one actionable finding (the warn! on cache hit) and several observations for tracking. Previous review rounds have addressed the major issues well.

---

Findings

1. load_compiled_from_path warning fires on every cache hit (Medium)

crates/vm/src/arch/rvr/compile.rs:347-351 — load_compiled_from_path logs tracing::warn!("loading rvr artifact without compatibility validation"). On a cache hit (compile.rs:428), the code calls load_compiled_from_path(&cache_path), triggering this warning even though the artifact was just fingerprint-validated. In production where cache hits are the common case, this will spam warn-level logs.

Suggest extracting the unsafe { libloading::Library::new(...) } + fingerprint recovery into a crate-private load_lib(lib_path) helper without the warning, and calling it from both the cache-hit path and from load_compiled_from_path (which keeps the warning for the unvalidated public API):

fn load_lib(lib_path: &Path) -> Result<RvrCompiled, CompileError> {
    let lib = unsafe {
        libloading::Library::new(lib_path)
            .map_err(|e| CompileError::LibLoad(format!("{}: {}", lib_path.display(), e)))?
    };
    let fingerprint = lib_path.file_stem()
        .and_then(|s| s.to_str())
        .and_then(|s| s.strip_prefix("rvr-"))
        .map(str::to_owned);
    Ok(RvrCompiled { lib, lib_path: lib_path.to_path_buf(), artifact_dir: None, fingerprint })
}

pub fn load_compiled_from_path(lib_path: &Path) -> Result<RvrCompiled, CompileError> {
    tracing::warn!(/* ... */);
    load_lib(lib_path)
}

Then use load_lib(&cache_path) at line 428 instead of load_compiled_from_path.

Fix this →

2. host_cpu_features() returns empty string on aarch64 Linux (Medium — correctness for shared caches)

crates/vm/src/arch/rvr/artifact_cache.rs:74 — The function looks for a line starting with "flags", which is x86-only. On aarch64 Linux, the equivalent line starts with "Features" (capital F); on riscv64 it's "isa". This means on aarch64, host_cpu_features() falls back to empty string, and combined with toolchain.host_os (which carries no arch info), two different aarch64 machines (e.g. Graviton2 vs Graviton3) sharing a cache directory would compute the same fingerprint despite generating different -march=native binaries — leading to potential SIGILL at runtime.

The latest automated review (June 8) identified this same issue. Suggest at minimum:

• Look for both "flags" (x86) and "Features" (arm64) in /proc/cpuinfo
• Fold std::env::consts::ARCH into the hash unconditionally so the fallback path still differentiates architectures

#[cfg(target_os = "linux")]
{
    let cpuinfo = fs::read_to_string("/proc/cpuinfo").unwrap_or_default();
    cpuinfo.lines()
        .find(|l| l.starts_with("flags") || l.starts_with("Features"))
        .map(|l| l.to_string())
        .unwrap_or_else(|| std::env::consts::ARCH.to_string())
}

Fix this →

3. Full setup_project runs on every call, even cache hits (Low — Performance, tracking item)

crates/vm/src/arch/rvr/compile.rs:394 — The entire C codegen pipeline (convert_vmexe_to_ir_with_debug, build_blocks, CProject::write_all, write_extension_staticlibs) runs before the fingerprint is computed and the cache is checked (lines 407–416). On a cache hit, all that work is discarded — only the make step is skipped.

This is inherent to the current design since the fingerprint needs the generated files to hash. Worth tracking as a follow-up: a two-phase fingerprint (hash VmExe bytes + config cheaply first, skip codegen on hit) could eliminate this overhead. Currently the caching benefit is limited to skipping the make compilation step.

4. TracerMode not explicitly in fingerprint — relies on C code divergence (Low)

crates/vm/src/arch/rvr/artifact_cache.rs:26-51 — TracerMode isn't directly hashed in compute_fingerprint. It's captured indirectly because different modes produce different C code. This works today (the cross-process test validates it), but explicitly hashing the mode discriminant would be more defensive against future changes where two modes might produce identical C but different runtime behavior:

h.update([opts.tracer_mode as u8]);
h.update(b"\0");

5. No domain separator after native_debug_info flag byte (Nit)

crates/vm/src/arch/rvr/artifact_cache.rs:33 — The native_debug_info flag is hashed as a single byte (0x00 or 0x01) without a trailing \0 separator before toolchain.compiler. Since the flag is fixed-length (1 byte) there's technically no ambiguity, but adding a \0 after it would make the format consistent with all other fields.

6. No compile_metered_segment_boundary_cached variant (Nit — Completeness)

crates/vm/src/arch/rvr/compile.rs:242-261 — compile_metered_segment_boundary and metered_segment_rvr_instance (vm.rs:510-534) don't have _cached counterparts. This may be intentional if segment-boundary mode isn't used in the caching workflow, but worth noting for completeness.

---

What looks good

• Race condition fix (compile.rs:106-119): Write-to-temp-then-rename is correctly implemented. Cleanup on rename failure is handled. fs::rename is atomic on the same filesystem, guaranteed since both files are in cache_dir.
• Error propagation in hash_dir_into (artifact_cache.rs:97): Uses .collect::<Result<Vec<_>, _>>()? — properly propagates IO errors from read_dir entries.
• EXT_LIBS filtering (compile.rs:399-403): EXT_LIBS= and EXT_SRCS= are correctly filtered from the fingerprinted make args. The comment at line 396-398 accurately explains why. The staticlib contents are captured by hash_dir_into.
• Non-cached functions delegate through cached with None (compile.rs:225-270): Clean refactor — compile() calls compile_cached(…, None), avoiding code duplication in compile_impl.
• Fingerprint design (artifact_cache.rs:26-51): Covers all relevant inputs (files, toolchain, CPU features, make args, debug flag). Separator bytes between fields. Truncation to 128 bits is more than sufficient for content-addressing.
• Deterministic directory hashing (artifact_cache.rs:96-114): Entries sorted by path, directories recursed with /\0 separator — correct and deterministic.
• Validation moved early (compile.rs:374-390): Mode/suspension-policy validation happens before setup_project, failing fast before expensive codegen.
• Cross-process test (rvr_cross_process_cache.rs): Excellent test design — spawns a separate binary to validate fingerprint stability across processes. The three assertions (artifact count, mtime stability, public value equality) cover the key invariants.
• build.rs changes (all 6 extension build.rs files): Adding rerun-if-changed for Cargo.toml, Cargo.lock, and ffi-common/src ensures FFI staticlibs are rebuilt when shared dependencies change.
• SDK API renames (compile_and_execute → execute, compile → compile_pure, execute → execute_compiled): Clearer naming that distinguishes the compile step from the execute step.

Overall Assessment

The implementation is well-structured and the fingerprinting approach is sound. All major prior review findings have been addressed. The two actionable items are finding #1 (spurious warn! on every cache hit — easy fix) and finding #2 (host_cpu_features() on aarch64 Linux — correctness issue for shared caches). The remaining findings are low-priority observations and design nits.

---

[github-actions]

group	app.proof_time_ms	app.cycles	leaf.proof_time_ms
fibonacci	1,023	4,000,051	392
keccak	16,473	14,365,133	3,061
sha2_bench	8,105	11,167,961	988
regex	1,223	4,090,656	356
ecrecover	434	112,210	288
pairing	600	592,827	296
kitchen_sink	3,869	1,979,971	854

Note: cells_used metrics omitted because CUDA tracegen does not expose unpadded trace heights.

Commit: 51a16b4

Benchmark Workflow

[gdmlcjs]

• In crates/rvr/rvr-openvm/c/Makefile, there are some configs that can be set with environment variables using ?=. They are not contained in the fingerprint so we should better add them in the fingerprint or change ?= to =.
• There were some changes in the base branch: removed lto=false for macOS, changed method names from compile_pure to compile. Could you update those changes? A rebase might be helpful.
• I'm not sure, but instead of having two versions of each function (with cache_dir: Option<&Path> and without) it might be simpler to have just one version (with cache_dir: Option<&Path>). Could you look into that?