From 219983932aad50599d2c91620f69ab12bc23d9ac Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Wed, 4 Dec 2024 21:51:27 +0100 Subject: [PATCH] perf: don't request unnecessary output (#231) We're currently requesting all `evm.bytecode` (and `evm.deployedBytecode`) output by default. This includes `evm.bytecode.generatedSources`, introduced in solc 0.8.0, which is a list of potentially huge Yul AST objects that is completely unused in Foundry. Only request the `Compact*` bytecode fields in the defaults. This cuts down a clean `forge build`: - from 2:20m to 1:30m (-36%) on [superform-xyz/superform-core](https://github.com/superform-xyz/superform-core) - from 1:39m to 1:29m (-10%) on [sablier-labs/v2-core](https://github.com/sablier-labs/v2-core) - from 54.5s to 45.7s (-20%) on [sablier-labs/v2-core](https://github.com/sablier-labs/v2-core) (FOUNDRY_PROFILE=lite which is `optimizer = false`) It may have a larger impact when compiling with >=0.8.28, because the cache storing this data was removed in that version (https://github.com/ethereum/solidity/commit/3c5e46b7d75172114413c7c775c412ffc41d9d38), or when optimizations are disabled as more IR will be generated and output to JSON. I verified that these inputs are accepted by solidity 0.4.14, 0.6.3, 0.8.28, and vyper 0.3.10, 0.4.0. All of these outputs are supported by all of them except for vyper which needs to be normalized. Drive-by: buffer stdin when writing to the solc subprocess. --- crates/artifacts/solc/src/output_selection.rs | 63 ++++++------------- crates/artifacts/vyper/src/input.rs | 4 +- crates/artifacts/vyper/src/settings.rs | 41 ++++++++++-- crates/compilers/src/artifact_output/mod.rs | 2 +- .../compilers/src/compilers/solc/compiler.rs | 8 ++- crates/compilers/src/compilers/vyper/input.rs | 2 +- crates/compilers/src/compilers/vyper/mod.rs | 17 +++-- crates/compilers/tests/project.rs | 43 ++++++++++--- .../multi-sample/src/interfaces/ICounter.vy | 14 ----- .../multi-sample/src/interfaces/ICounter.vyi | 11 ++++ test-data/vyper-imports/src/A.vy | 2 - test-data/vyper-imports/src/module/inner/E.vy | 1 - .../vyper-sample/src/interfaces/ICounter.vy | 14 ----- .../vyper-sample/src/interfaces/ICounter.vyi | 11 ++++ 14 files changed, 132 insertions(+), 101 deletions(-) delete mode 100644 test-data/multi-sample/src/interfaces/ICounter.vy create mode 100644 test-data/multi-sample/src/interfaces/ICounter.vyi delete mode 100644 test-data/vyper-sample/src/interfaces/ICounter.vy create mode 100644 test-data/vyper-sample/src/interfaces/ICounter.vyi diff --git a/crates/artifacts/solc/src/output_selection.rs b/crates/artifacts/solc/src/output_selection.rs index 99beb3e0..c56c59ec 100644 --- a/crates/artifacts/solc/src/output_selection.rs +++ b/crates/artifacts/solc/src/output_selection.rs @@ -51,24 +51,6 @@ pub type FileOutputSelection = BTreeMap>; /// Note that using a using `evm`, `evm.bytecode`, `ewasm`, etc. will select /// every target part of that output. Additionally, `*` can be used as a /// wildcard to request everything. -/// -/// The default output selection is -/// -/// ```json -/// { -/// "*": { -/// "*": [ -/// "abi", -/// "evm.bytecode", -/// "evm.deployedBytecode", -/// "evm.methodIdentifiers" -/// ], -/// "": [ -/// "ast" -/// ] -/// } -/// } -/// ``` #[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize)] #[serde(transparent)] pub struct OutputSelection(pub BTreeMap); @@ -88,31 +70,18 @@ impl OutputSelection { .into() } - /// Default output selection for compiler output: - /// - /// `{ "*": { "*": [ "*" ], "": [ - /// "abi","evm.bytecode","evm.deployedBytecode","evm.methodIdentifiers"] } }` - /// - /// Which enables it for all files and all their contracts ("*" wildcard) + /// Default output selection. pub fn default_output_selection() -> Self { BTreeMap::from([("*".to_string(), Self::default_file_output_selection())]).into() } - /// Default output selection for a single file: - /// - /// `{ "*": [ "*" ], "": [ - /// "abi","evm.bytecode","evm.deployedBytecode","evm.methodIdentifiers"] }` + /// Default file output selection. /// - /// Which enables it for all the contracts in the file ("*" wildcard) + /// Uses [`ContractOutputSelection::basic`]. pub fn default_file_output_selection() -> FileOutputSelection { BTreeMap::from([( "*".to_string(), - vec![ - "abi".to_string(), - "evm.bytecode".to_string(), - "evm.deployedBytecode".to_string(), - "evm.methodIdentifiers".to_string(), - ], + ContractOutputSelection::basic().iter().map(ToString::to_string).collect(), )]) } @@ -146,7 +115,7 @@ impl OutputSelection { } /// Returns true if this output selection is a subset of the other output selection. - /// TODO: correctly process wildcard keys to reduce false negatives + // TODO: correctly process wildcard keys to reduce false negatives pub fn is_subset_of(&self, other: &Self) -> bool { self.0.iter().all(|(file, selection)| { other.0.get(file).is_some_and(|other_selection| { @@ -229,16 +198,24 @@ pub enum ContractOutputSelection { impl ContractOutputSelection { /// Returns the basic set of contract level settings that should be included in the `Contract` - /// that solc emits: - /// - "abi" - /// - "evm.bytecode" - /// - "evm.deployedBytecode" - /// - "evm.methodIdentifiers" + /// that solc emits. + /// + /// These correspond to the fields in `CompactBytecode`, `CompactDeployedBytecode`, ABI, and + /// method identfiers. pub fn basic() -> Vec { + // We don't include all the `bytecode` fields because `generatedSources` is a massive JSON + // object and is not used by Foundry. vec![ Self::Abi, - BytecodeOutputSelection::All.into(), - DeployedBytecodeOutputSelection::All.into(), + // The fields in `CompactBytecode`. + BytecodeOutputSelection::Object.into(), + BytecodeOutputSelection::SourceMap.into(), + BytecodeOutputSelection::LinkReferences.into(), + // The fields in `CompactDeployedBytecode`. + DeployedBytecodeOutputSelection::Object.into(), + DeployedBytecodeOutputSelection::SourceMap.into(), + DeployedBytecodeOutputSelection::LinkReferences.into(), + DeployedBytecodeOutputSelection::ImmutableReferences.into(), EvmOutputSelection::MethodIdentifiers.into(), ] } diff --git a/crates/artifacts/vyper/src/input.rs b/crates/artifacts/vyper/src/input.rs index 9b1b17d7..85543841 100644 --- a/crates/artifacts/vyper/src/input.rs +++ b/crates/artifacts/vyper/src/input.rs @@ -17,7 +17,7 @@ pub struct VyperInput { } impl VyperInput { - pub fn new(sources: Sources, mut settings: VyperSettings) -> Self { + pub fn new(sources: Sources, mut settings: VyperSettings, version: &Version) -> Self { let mut new_sources = Sources::new(); let mut interfaces = Sources::new(); @@ -31,7 +31,7 @@ impl VyperInput { } } - settings.sanitize_output_selection(); + settings.sanitize_output_selection(version); Self { language: "Vyper".to_string(), sources: new_sources, interfaces, settings } } diff --git a/crates/artifacts/vyper/src/settings.rs b/crates/artifacts/vyper/src/settings.rs index 373f3048..81a73670 100644 --- a/crates/artifacts/vyper/src/settings.rs +++ b/crates/artifacts/vyper/src/settings.rs @@ -8,12 +8,14 @@ use std::{ path::{Path, PathBuf}, }; -pub const VYPER_SEARCH_PATHS: Version = Version::new(0, 4, 0); +pub const VYPER_SEARCH_PATHS: Version = VYPER_0_4; pub const VYPER_BERLIN: Version = Version::new(0, 3, 0); pub const VYPER_PARIS: Version = Version::new(0, 3, 7); pub const VYPER_SHANGHAI: Version = Version::new(0, 3, 8); pub const VYPER_CANCUN: Version = Version::new(0, 3, 8); +const VYPER_0_4: Version = Version::new(0, 4, 0); + #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum VyperOptimizationMode { @@ -65,15 +67,42 @@ impl VyperSettings { }); } - /// During caching we prune output selection for some of the sources, however, Vyper will reject - /// [] as an output selection, so we are adding "abi" as a default output selection which is - /// cheap to be produced. - pub fn sanitize_output_selection(&mut self) { + /// Sanitize the output selection. + #[allow(clippy::collapsible_if)] + pub fn sanitize_output_selection(&mut self, version: &Version) { self.output_selection.0.values_mut().for_each(|selection| { selection.values_mut().for_each(|selection| { + // During caching we prune output selection for some of the sources, however, Vyper + // will reject `[]` as an output selection, so we are adding "abi" as a default + // output selection which is cheap to be produced. if selection.is_empty() { selection.push("abi".to_string()) } + + // Unsupported selections. + #[rustfmt::skip] + selection.retain(|selection| { + if *version <= VYPER_0_4 { + if matches!( + selection.as_str(), + | "evm.bytecode.sourceMap" | "evm.deployedBytecode.sourceMap" + ) { + return false; + } + } + + if matches!( + selection.as_str(), + | "evm.bytecode.sourceMap" | "evm.deployedBytecode.sourceMap" + // https://github.com/vyperlang/vyper/issues/4389 + | "evm.bytecode.linkReferences" | "evm.deployedBytecode.linkReferences" + | "evm.deployedBytecode.immutableReferences" + ) { + return false; + } + + true + }); }) }); } @@ -84,7 +113,7 @@ impl VyperSettings { self.search_paths = None; } - self.sanitize_output_selection(); + self.sanitize_output_selection(version); self.normalize_evm_version(version); } diff --git a/crates/compilers/src/artifact_output/mod.rs b/crates/compilers/src/artifact_output/mod.rs index d111d07c..04b3f9c0 100644 --- a/crates/compilers/src/artifact_output/mod.rs +++ b/crates/compilers/src/artifact_output/mod.rs @@ -868,7 +868,7 @@ pub trait ArtifactOutput { let mut files = contracts.keys().collect::>(); // Iterate starting with top-most files to ensure that they get the shortest paths. - files.sort_by(|file1, file2| { + files.sort_by(|&file1, &file2| { (file1.components().count(), file1).cmp(&(file2.components().count(), file2)) }); for file in files { diff --git a/crates/compilers/src/compilers/solc/compiler.rs b/crates/compilers/src/compilers/solc/compiler.rs index 86e7fb51..e99a09e8 100644 --- a/crates/compilers/src/compilers/solc/compiler.rs +++ b/crates/compilers/src/compilers/solc/compiler.rs @@ -9,6 +9,7 @@ use semver::{Version, VersionReq}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use std::{ collections::BTreeSet, + io::{self, Write}, path::{Path, PathBuf}, process::{Command, Output, Stdio}, str::FromStr, @@ -428,8 +429,11 @@ impl Solc { let mut child = cmd.spawn().map_err(self.map_io_err())?; debug!("spawned"); - let stdin = child.stdin.as_mut().unwrap(); - serde_json::to_writer(stdin, input)?; + { + let mut stdin = io::BufWriter::new(child.stdin.take().unwrap()); + serde_json::to_writer(&mut stdin, input)?; + stdin.flush().map_err(self.map_io_err())?; + } debug!("wrote JSON input to stdin"); let output = child.wait_with_output().map_err(self.map_io_err())?; diff --git a/crates/compilers/src/compilers/vyper/input.rs b/crates/compilers/src/compilers/vyper/input.rs index 0401e6a1..0f0f0a1d 100644 --- a/crates/compilers/src/compilers/vyper/input.rs +++ b/crates/compilers/src/compilers/vyper/input.rs @@ -26,7 +26,7 @@ impl CompilerInput for VyperVersionedInput { _language: Self::Language, version: Version, ) -> Self { - Self { input: VyperInput::new(sources, settings), version } + Self { input: VyperInput::new(sources, settings, &version), version } } fn compiler_name(&self) -> Cow<'static, str> { diff --git a/crates/compilers/src/compilers/vyper/mod.rs b/crates/compilers/src/compilers/vyper/mod.rs index a9cc7f3b..8304b767 100644 --- a/crates/compilers/src/compilers/vyper/mod.rs +++ b/crates/compilers/src/compilers/vyper/mod.rs @@ -7,6 +7,7 @@ use foundry_compilers_core::error::{Result, SolcError}; use semver::Version; use serde::{de::DeserializeOwned, Serialize}; use std::{ + io::{self, Write}, path::{Path, PathBuf}, process::{Command, Stdio}, str::FromStr, @@ -79,8 +80,11 @@ impl Vyper { /// Convenience function for compiling all sources under the given path pub fn compile_source(&self, path: &Path) -> Result { - let input = - VyperInput::new(Source::read_all_from(path, VYPER_EXTENSIONS)?, Default::default()); + let input = VyperInput::new( + Source::read_all_from(path, VYPER_EXTENSIONS)?, + Default::default(), + &self.version, + ); self.compile(&input) } @@ -114,7 +118,7 @@ impl Vyper { /// let vyper = Vyper::new("vyper")?; /// let path = Path::new("path/to/sources"); /// let sources = Source::read_all_from(path, &["vy", "vyi"])?; - /// let input = VyperInput::new(sources, VyperSettings::default()); + /// let input = VyperInput::new(sources, VyperSettings::default(), &vyper.version); /// let output = vyper.compile(&input)?; /// # Ok::<_, Box>(()) /// ``` @@ -149,8 +153,11 @@ impl Vyper { let mut child = cmd.spawn().map_err(self.map_io_err())?; debug!("spawned"); - let stdin = child.stdin.as_mut().unwrap(); - serde_json::to_writer(stdin, input)?; + { + let mut stdin = io::BufWriter::new(child.stdin.take().unwrap()); + serde_json::to_writer(&mut stdin, input)?; + stdin.flush().map_err(self.map_io_err())?; + } debug!("wrote JSON input to stdin"); let output = child.wait_with_output().map_err(self.map_io_err())?; diff --git a/crates/compilers/tests/project.rs b/crates/compilers/tests/project.rs index 4742cba5..90ac3a68 100644 --- a/crates/compilers/tests/project.rs +++ b/crates/compilers/tests/project.rs @@ -47,6 +47,10 @@ pub static VYPER: LazyLock = LazyLock::new(|| { #[cfg(target_family = "unix")] use std::{fs::Permissions, os::unix::fs::PermissionsExt}; + if let Ok(vyper) = Vyper::new("vyper") { + return vyper; + } + take_solc_installer_lock!(_lock); let path = std::env::temp_dir().join("vyper"); @@ -54,16 +58,32 @@ pub static VYPER: LazyLock = LazyLock::new(|| { return Vyper::new(&path).unwrap(); } - let url = match platform() { - Platform::MacOsAarch64 => "https://github.com/vyperlang/vyper/releases/download/v0.3.10/vyper.0.3.10+commit.91361694.darwin", - Platform::LinuxAmd64 => "https://github.com/vyperlang/vyper/releases/download/v0.3.10/vyper.0.3.10+commit.91361694.linux", - Platform::WindowsAmd64 => "https://github.com/vyperlang/vyper/releases/download/v0.3.10/vyper.0.3.10+commit.91361694.windows.exe", - _ => panic!("unsupported") - }; - - let res = reqwest::Client::builder().build().unwrap().get(url).send().await.unwrap(); + let base = "https://github.com/vyperlang/vyper/releases/download/v0.4.0/vyper.0.4.0+commit.e9db8d9f"; + let url = format!( + "{base}.{}", + match platform() { + Platform::MacOsAarch64 => "darwin", + Platform::LinuxAmd64 => "linux", + Platform::WindowsAmd64 => "windows.exe", + platform => panic!("unsupported platform: {platform:?}"), + } + ); - assert!(res.status().is_success()); + let mut retry = 3; + let mut res = None; + while retry > 0 { + match reqwest::get(&url).await.unwrap().error_for_status() { + Ok(res2) => { + res = Some(res2); + break; + } + Err(e) => { + eprintln!("{e}"); + retry -= 1; + } + } + } + let res = res.expect("failed to get vyper binary"); let bytes = res.bytes().await.unwrap(); @@ -3889,17 +3909,20 @@ fn can_compile_vyper_with_cache() { .unwrap(); let compiled = project.compile().unwrap(); + compiled.assert_success(); assert!(compiled.find_first("Counter").is_some()); compiled.assert_success(); // cache is used when nothing to compile let compiled = project.compile().unwrap(); + compiled.assert_success(); assert!(compiled.find_first("Counter").is_some()); assert!(compiled.is_unchanged()); // deleted artifacts cause recompile even with cache std::fs::remove_dir_all(project.artifacts_path()).unwrap(); let compiled = project.compile().unwrap(); + compiled.assert_success(); assert!(compiled.find_first("Counter").is_some()); assert!(!compiled.is_unchanged()); } @@ -3975,9 +3998,9 @@ fn test_can_compile_multi() { .unwrap(); let compiled = project.compile().unwrap(); + compiled.assert_success(); assert!(compiled.find(&root.join("src/Counter.sol"), "Counter").is_some()); assert!(compiled.find(&root.join("src/Counter.vy"), "Counter").is_some()); - compiled.assert_success(); } // This is a reproduction of https://github.com/foundry-rs/compilers/issues/47 diff --git a/test-data/multi-sample/src/interfaces/ICounter.vy b/test-data/multi-sample/src/interfaces/ICounter.vy deleted file mode 100644 index 6923a6a1..00000000 --- a/test-data/multi-sample/src/interfaces/ICounter.vy +++ /dev/null @@ -1,14 +0,0 @@ -# pragma version ^0.3.10 - -@external -@view -def number() -> uint256: - return empty(uint256) - -@external -def set_number(new_number: uint256): - pass - -@external -def increment() -> uint256: - return empty(uint256) diff --git a/test-data/multi-sample/src/interfaces/ICounter.vyi b/test-data/multi-sample/src/interfaces/ICounter.vyi new file mode 100644 index 00000000..788b8d61 --- /dev/null +++ b/test-data/multi-sample/src/interfaces/ICounter.vyi @@ -0,0 +1,11 @@ +# pragma version ^0.4.0 + +@external +@view +def number() -> uint256: ... + +@external +def set_number(new_number: uint256): ... + +@external +def increment() -> uint256: ... diff --git a/test-data/vyper-imports/src/A.vy b/test-data/vyper-imports/src/A.vy index 8adf5442..57e1633e 100644 --- a/test-data/vyper-imports/src/A.vy +++ b/test-data/vyper-imports/src/A.vy @@ -1,3 +1 @@ -from . import B -import A as AAlias from .module import C \ No newline at end of file diff --git a/test-data/vyper-imports/src/module/inner/E.vy b/test-data/vyper-imports/src/module/inner/E.vy index bc8f77bd..e69de29b 100644 --- a/test-data/vyper-imports/src/module/inner/E.vy +++ b/test-data/vyper-imports/src/module/inner/E.vy @@ -1 +0,0 @@ -from ...module.inner import E \ No newline at end of file diff --git a/test-data/vyper-sample/src/interfaces/ICounter.vy b/test-data/vyper-sample/src/interfaces/ICounter.vy deleted file mode 100644 index 6923a6a1..00000000 --- a/test-data/vyper-sample/src/interfaces/ICounter.vy +++ /dev/null @@ -1,14 +0,0 @@ -# pragma version ^0.3.10 - -@external -@view -def number() -> uint256: - return empty(uint256) - -@external -def set_number(new_number: uint256): - pass - -@external -def increment() -> uint256: - return empty(uint256) diff --git a/test-data/vyper-sample/src/interfaces/ICounter.vyi b/test-data/vyper-sample/src/interfaces/ICounter.vyi new file mode 100644 index 00000000..788b8d61 --- /dev/null +++ b/test-data/vyper-sample/src/interfaces/ICounter.vyi @@ -0,0 +1,11 @@ +# pragma version ^0.4.0 + +@external +@view +def number() -> uint256: ... + +@external +def set_number(new_number: uint256): ... + +@external +def increment() -> uint256: ...