Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions prdoc/pr_10448.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: 'wasm-builder: Only overwrite wasm files if they changed'
doc:
- audience: Runtime Dev
description: |-
When running two different `cargo` commands, they may both compile the same wasm files. When the second `cargo` command produces the same wasm files, we are now not gonna overwrite it. This has the advantage that we can run the first command again without it trying to recompile the project. Right now it would lead to the wasm files always getting recreated, which is wasting a lot of time :)
crates:
- name: substrate-wasm-builder
bump: patch
17 changes: 8 additions & 9 deletions substrate/utils/wasm-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,7 @@
//! --toolchain nightly-2024-12-26`.

use prerequisites::DummyCrate;
use std::{
env, fs,
io::BufRead,
path::{Path, PathBuf},
process::Command,
};
use std::{env, fs, io::BufRead, path::Path, process::Command};
use version::Version;

mod builder;
Expand Down Expand Up @@ -188,14 +183,18 @@ fn write_file_if_changed(file: impl AsRef<Path>, content: impl AsRef<str>) {
}

/// Copy `src` to `dst` if the `dst` does not exist or is different.
fn copy_file_if_changed(src: PathBuf, dst: PathBuf) {
let src_file = fs::read_to_string(&src).ok();
let dst_file = fs::read_to_string(&dst).ok();
fn copy_file_if_changed(src: &Path, dst: &Path) -> bool {
let src_file = fs::read(src).ok();
let dst_file = fs::read(dst).ok();

if src_file != dst_file {
fs::copy(&src, &dst).unwrap_or_else(|_| {
panic!("Copying `{}` to `{}` can not fail; qed", src.display(), dst.display())
});

true
} else {
false
}
}

Expand Down
141 changes: 96 additions & 45 deletions substrate/utils/wasm-builder/src/wasm_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@

#[cfg(feature = "metadata-hash")]
use crate::builder::MetadataExtraInfo;
use crate::{write_file_if_changed, CargoCommandVersioned, RuntimeTarget, OFFLINE};
use crate::{
copy_file_if_changed, write_file_if_changed, CargoCommandVersioned, RuntimeTarget, OFFLINE,
};

use build_helper::rerun_if_changed;
use cargo_metadata::{DependencyKind, Metadata, MetadataCommand};
Expand Down Expand Up @@ -79,6 +81,40 @@ impl WasmBinary {
}
}

/// Helper struct for managing blob file paths.
struct BlobPaths {
/// The base name of the blob (without extension).
blob_name: String,
/// The project directory where blobs are stored.
project: PathBuf,
}

impl BlobPaths {
fn new(blob_name: String, project: PathBuf) -> Self {
Self { blob_name, project }
}

/// Returns the path to the bloaty wasm file.
fn bloaty(&self) -> PathBuf {
self.project.join(format!("{}.wasm", self.blob_name))
}

/// Returns the path to the compact wasm file.
fn compact(&self) -> PathBuf {
self.project.join(format!("{}.compact.wasm", self.blob_name))
}

/// Returns the path to the compact compressed wasm file.
fn compact_compressed(&self) -> PathBuf {
self.project.join(format!("{}.compact.compressed.wasm", self.blob_name))
}

/// Returns the blob name.
fn name(&self) -> &str {
&self.blob_name
}
}

fn crate_metadata(cargo_manifest: &Path) -> Metadata {
let mut cargo_lock = cargo_manifest.to_path_buf();
cargo_lock.set_file_name("Cargo.lock");
Expand Down Expand Up @@ -199,25 +235,27 @@ pub(crate) fn create_and_compile(

let blob_name =
blob_out_name_override.unwrap_or_else(|| get_blob_name(target, &wasm_project_cargo_toml));
let blob_paths = BlobPaths::new(blob_name, project.clone());

let (final_blob_binary, bloaty_blob_binary) = match target {
let (final_blob_binary, bloaty_blob_binary, any_changed) = match target {
RuntimeTarget::Wasm => {
let out_path = project.join(format!("{blob_name}.wasm"));
fs::copy(raw_blob_path, &out_path).expect("copying the runtime blob should never fail");
let out_path = blob_paths.bloaty();
let bloaty_changed = copy_file_if_changed(&raw_blob_path, &out_path);

maybe_compact_and_compress_wasm(
let (final_binary, bloaty_binary, did_compact) = maybe_compact_and_compress_wasm(
&wasm_project_cargo_toml,
&project,
WasmBinaryBloaty(out_path),
&blob_name,
&blob_paths,
check_for_runtime_version_section,
&build_config,
)
bloaty_changed,
);
(final_binary, bloaty_binary, bloaty_changed || did_compact)
},
RuntimeTarget::Riscv => {
let out_path = project.join(format!("{blob_name}.polkavm"));
fs::copy(raw_blob_path, &out_path).expect("copying the runtime blob should never fail");
(None, WasmBinaryBloaty(out_path))
let out_path = project.join(format!("{}.polkavm", blob_paths.name()));
let changed = copy_file_if_changed(&raw_blob_path, &out_path);
(None, WasmBinaryBloaty(out_path), changed)
},
};

Expand All @@ -229,42 +267,61 @@ pub(crate) fn create_and_compile(
&bloaty_blob_binary,
);

if let Err(err) = adjust_mtime(&bloaty_blob_binary, final_blob_binary.as_ref()) {
build_helper::warning!("Error while adjusting the mtime of the blob binaries: {}", err)
if any_changed {
if let Err(err) = adjust_mtime(&bloaty_blob_binary, final_blob_binary.as_ref()) {
build_helper::warning!("Error while adjusting the mtime of the blob binaries: {}", err)
}
}

(final_blob_binary, bloaty_blob_binary)
}

fn maybe_compact_and_compress_wasm(
wasm_project_cargo_toml: &Path,
project: &Path,
bloaty_blob_binary: WasmBinaryBloaty,
blob_name: &str,
blob_paths: &BlobPaths,
check_for_runtime_version_section: bool,
build_config: &BuildConfiguration,
) -> (Option<WasmBinary>, WasmBinaryBloaty) {
bloaty_changed: bool,
) -> (Option<WasmBinary>, WasmBinaryBloaty, bool) {
let needs_compact = build_config.outer_build_profile.wants_compact();
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm can the following happen?

  1. Build bloaty blob A + compressed + compact
  2. Build only bloaty blob B
  3. Built bloaty blob B, will be the same bloaty blob as before but compressed and compact exist from run 1 so will be wrongly returned?

Copy link
Contributor

Choose a reason for hiding this comment

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

If blob B is the same as blob A, then should it be fine to return compressed and compact from run 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. If B is the same as A, the compressed version of A should be the same as the compressed version of B.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think what @skunert meant is that we would return compressed binary from run 1 if even if needs_compact is false.
Step 2:

  • bloaty_changed = false (bloaty B is identical to A, so no copy happened)
  • needs_compact = false (debug mode)
  • compact_or_compressed_exists = true (from step 1)
    =>
  • should_regenerate = false

which results in returning:

   Some(WasmBinary(compressed_path))  // Returns compressed from step 1

Copy link
Member Author

Choose a reason for hiding this comment

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

Even if we would return the compressed one, it would be fine. The compressed one is only special cased, to speedup debug builds and not more. It doesn't do any harm.

Also we have debug/wbuild and release/wbuild. So, if needs_compact changes, we are actually in a different folder and we can not see needs_compact = true when we have needs_compact = false.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that makes sense. Maybe we could have some comment in the code pointing that out. It's quite convoluted.

Copy link
Contributor

@skunert skunert Nov 28, 2025

Choose a reason for hiding this comment

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

No, I meant that blob B is different from A.

In run 2 lets say needs_compact is false, so we would not rebuild compressed and compact wasm files. So the ones from run 1 would stay right?

In run 3 we just check that bloaty is the same, which it is. But the compact and compressed blobs are still from run 1 which was different. Then we would return the compressed blobs from run 1 in run 3.

But maybe I am missing something here 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Read what I wrote here: #10448 (comment)

TDLR: needs_compact is always the same in the folder where we output these files.

let compact_path = blob_paths.compact();
let compressed_path = blob_paths.compact_compressed();
let compact_or_compressed_exists = compact_path.exists() || compressed_path.exists();
let should_regenerate = bloaty_changed || (needs_compact && !compact_or_compressed_exists);

if !should_regenerate {
let final_blob = if compressed_path.exists() {
Some(WasmBinary(compressed_path))
} else if compact_path.exists() {
Some(WasmBinary(compact_path))
} else {
None
};

return (final_blob, bloaty_blob_binary, false);
}

// Try to compact and compress the bloaty blob, if the *outer* profile wants it.
//
// This is because, by default the inner profile will be set to `Release` even when the outer
// profile is `Debug`, because the blob built in `Debug` profile is too slow for normal
// development activities.
let (compact_blob_path, compact_compressed_blob_path) =
if build_config.outer_build_profile.wants_compact() {
let compact_blob_path = compact_wasm(&project, blob_name, &bloaty_blob_binary);
let compact_compressed_blob_path =
compact_blob_path.as_ref().and_then(|p| try_compress_blob(&p.0, blob_name));
(compact_blob_path, compact_compressed_blob_path)
} else {
// We at least want to lower the `sign-ext` code to `mvp`.
wasm_opt::OptimizationOptions::new_opt_level_0()
.add_pass(wasm_opt::Pass::SignextLowering)
.debug_info(true)
.run(bloaty_blob_binary.bloaty_path(), bloaty_blob_binary.bloaty_path())
.expect("Failed to lower sign-ext in WASM binary.");

(None, None)
};
let (compact_blob_path, compact_compressed_blob_path) = if needs_compact {
let compact_blob_path = compact_wasm(blob_paths, &bloaty_blob_binary);
let compact_compressed_blob_path =
compact_blob_path.as_ref().and_then(|p| try_compress_blob(blob_paths, p));
(compact_blob_path, compact_compressed_blob_path)
} else {
// We at least want to lower the `sign-ext` code to `mvp`.
wasm_opt::OptimizationOptions::new_opt_level_0()
.add_pass(wasm_opt::Pass::SignextLowering)
.debug_info(true)
.run(bloaty_blob_binary.bloaty_path(), bloaty_blob_binary.bloaty_path())
.expect("Failed to lower sign-ext in WASM binary.");

(None, None)
};

if check_for_runtime_version_section {
ensure_runtime_version_wasm_section_exists(bloaty_blob_binary.bloaty_path());
Expand All @@ -276,7 +333,7 @@ fn maybe_compact_and_compress_wasm(
.as_ref()
.map(|binary| copy_blob_to_target_directory(wasm_project_cargo_toml, binary));

(final_blob_binary, bloaty_blob_binary)
(final_blob_binary, bloaty_blob_binary, true)
}

/// Ensures that the `runtime_version` section exists in the given blob.
Expand Down Expand Up @@ -703,7 +760,7 @@ fn create_project(

if let Some(crate_lock_file) = find_cargo_lock(project_cargo_toml) {
// Use the `Cargo.lock` of the main project.
crate::copy_file_if_changed(crate_lock_file, wasm_project_folder.join("Cargo.lock"));
copy_file_if_changed(&crate_lock_file, &wasm_project_folder.join("Cargo.lock"));
}

wasm_project_folder
Expand Down Expand Up @@ -1030,12 +1087,8 @@ fn build_bloaty_blob(
}
}

fn compact_wasm(
project: &Path,
blob_name: &str,
bloaty_binary: &WasmBinaryBloaty,
) -> Option<WasmBinary> {
let wasm_compact_path = project.join(format!("{blob_name}.compact.wasm"));
fn compact_wasm(blob_paths: &BlobPaths, bloaty_binary: &WasmBinaryBloaty) -> Option<WasmBinary> {
let wasm_compact_path = blob_paths.compact();
let start = std::time::Instant::now();
wasm_opt::OptimizationOptions::new_opt_level_0()
.mvp_features_only()
Expand All @@ -1054,15 +1107,13 @@ fn compact_wasm(
Some(WasmBinary(wasm_compact_path))
}

fn try_compress_blob(compact_blob_path: &Path, out_name: &str) -> Option<WasmBinary> {
fn try_compress_blob(blob_paths: &BlobPaths, compact_blob: &WasmBinary) -> Option<WasmBinary> {
use sp_maybe_compressed_blob::CODE_BLOB_BOMB_LIMIT;

let project = compact_blob_path.parent().expect("blob path should have a parent directory");
let compact_compressed_blob_path =
project.join(format!("{}.compact.compressed.wasm", out_name));
let compact_compressed_blob_path = blob_paths.compact_compressed();

let start = std::time::Instant::now();
let data = fs::read(compact_blob_path).expect("Failed to read WASM binary");
let data = fs::read(compact_blob.wasm_binary_path()).expect("Failed to read WASM binary");
if let Some(compressed) =
sp_maybe_compressed_blob::compress_strongly(&data, CODE_BLOB_BOMB_LIMIT)
{
Expand Down
Loading